Skip to content
23 changes: 23 additions & 0 deletions pkgs/profpatsch/nman/nman.1
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
.Sh SYNOPSIS
.Nm
.Op Fl hv
.Op Fl f Ar path
.Ar ATTR
.Op Ar PAGE | SECTION Op PAGE
.Sh DESCRIPTION
Expand Down Expand Up @@ -36,6 +37,11 @@ of
and searches all its outputs for a man page named
.Ar ATTR
which may be in any section.
For dotted attribute paths like
.Ql pkgs.profpatsch.nman ,
only the last component
.Po Ql nman Pc
is used as the man page name to search for.
If multiple matches are found, the one that is alphanumerically
lower is preferred:
For example,
Expand All @@ -47,6 +53,7 @@ Like above, but
.Nm
will only look for the man page in the given section
.Ar SECTION .
For dotted attributes, the last component is used as the page name to search for.
Note that
.Ar SECTION
must be a number or
Expand Down Expand Up @@ -80,6 +87,13 @@ Print usage information.
Put
.Nm
into verbose mode, where it prints all commands it executes.
.It Fl -file Ar path | f Ar path
Use the specified .nix file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use the specified .nix file
Use the Nix expression at

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it only allows nix files? I think. We pass it to import but if we want to allow arbitrary expressions we’d have to escape them properly first

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is clear that it's a file from what follows.

.Ar path
instead of
.Ql <nixpkgs>
from the Nix search path. This allows using local package definitions
or custom package sets.
.El

.Sh EXAMPLES
Expand All @@ -97,6 +111,15 @@ nman mandoc man

# open man(7) from the same package
nman mandoc 7 man

# open lowdown(1) from the default.nix in the current directory
nman -f . lowdown

# open nman(1) from dotted attribute path (searches for "nman", not "pkgs.profpatsch.nman")
nman -f . pkgs.profpatsch.nman

# open man page from a custom package set
nman -f ./my-packages.nix myCustomPackage
.Ed
.Sh DISCLAIMER
.Nm
Expand Down
162 changes: 127 additions & 35 deletions pkgs/profpatsch/nman/nman.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@ use std::process::{Command, ExitStatus, Stdio};
use temp::TempDir;

enum CliAction<'a> {
/// attribute, section, page
Man(&'a str, Option<&'a str>, &'a str),
Man {
attr: &'a str,
section: Option<&'a str>,
page: &'a str,
file_path: Option<&'a str>,
},
}

enum CliResult<'a> {
Expand All @@ -22,37 +26,85 @@ enum CliResult<'a> {

fn main() {
use CliResult::*;
let (opts, args): (Vec<String>, Vec<String>) =
std::env::args().partition(|s| s.starts_with("-"));

let mut cli_res: CliResult = match args.len() {
2 => Action(CliAction::Man(&args[1], None, &args[1])),
3 => match parse_man_section(&args[2]) {
Ok(s) => Action(CliAction::Man(&args[1], Some(s), &args[1])),
Err(_) => Action(CliAction::Man(&args[1], None, &args[2])),
},
4 => match parse_man_section(&args[2]) {
Err(err_msg) => ShowUsage {
err_msg: Some(err_msg),
},
Ok(s) => Action(CliAction::Man(&args[1], Some(s), &args[3])),
},
_ => ShowUsage {
err_msg: Some("Unexpected number of arguments"),
},
};

let all_args: Vec<String> = std::env::args().collect();

let mut is_debug: bool = false;
for opt in opts {
match &opt[..] {
"--help" | "--usage" | "-h" => cli_res = ShowUsage { err_msg: None },
"--verbose" | "-v" => is_debug = true,
_ => {
cli_res = ShowUsage {
err_msg: Some("Unknown option"),
let mut file_path: Option<&str> = None;
let mut cli_res: CliResult = ShowUsage { err_msg: Some("Unexpected number of arguments") };
let mut positional_args = Vec::new();

let mut i = 1; // Skip program name
while i < all_args.len() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit icky.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to do it better tbh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does while let Some(arg) = … entitle us to call next() inside the loop? Surely something like that must be possible.

let arg = &all_args[i];

if arg.starts_with("-") {
match &arg[..] {
"--help" | "--usage" | "-h" => {
cli_res = ShowUsage { err_msg: None };
break;
},
"--verbose" | "-v" => is_debug = true,
"-f" | "--file" => {
if i + 1 >= all_args.len() {
cli_res = ShowUsage {
err_msg: Some("Option -f/--file requires an argument"),
};
break;
}
file_path = Some(&all_args[i + 1]);
i += 1; // Skip the next argument since it's the file path
},
_ => {
cli_res = ShowUsage {
err_msg: Some("Unknown option"),
};
break;
}
}
} else {
positional_args.push(arg);
}
i += 1;
}

// Only process positional arguments if we haven't already set an error
if let ShowUsage { err_msg: _ } = cli_res {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this matches if we have set an error already!!! you've made it worse!!!!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also return a Result from either main or a dedicated function and return early instead of adding your own Resultish type. For example if you want to have different errors with different actions, you could still use Result<_, CustomErrorEnum>.

cli_res = match positional_args.len() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you match on a slice of positonal_args?

1 => Action(CliAction::Man {
attr: &positional_args[0],
section: None,
page: extract_page_name_from_attr(&positional_args[0]),
file_path,
}),
2 => match parse_man_section(&positional_args[1]) {
Ok(s) => Action(CliAction::Man {
attr: &positional_args[0],
section: Some(s),
page: extract_page_name_from_attr(&positional_args[0]),
file_path,
}),
Err(_) => Action(CliAction::Man {
attr: &positional_args[0],
section: None,
page: &positional_args[1],
file_path,
}),
},
3 => match parse_man_section(&positional_args[1]) {
Err(err_msg) => ShowUsage {
err_msg: Some(err_msg),
},
Ok(s) => Action(CliAction::Man {
attr: &positional_args[0],
section: Some(s),
page: &positional_args[2],
file_path,
}),
},
_ => ShowUsage {
err_msg: Some("Unexpected number of arguments"),
},
};
}

let main = Main { is_debug };
Expand All @@ -61,11 +113,15 @@ fn main() {
if let Some(msg) = err_msg {
eprintln!("nman: usage error: {}", msg);
}
println!("Usage: {} ATTR [PAGE | SECTION [PAGE]]", &args[0]);
println!("Usage: {} [OPTIONS] ATTR [PAGE | SECTION [PAGE]]", &all_args[0]);
println!("Options:");
println!(" -h, --help, --usage Show this help message");
println!(" -v, --verbose Enable verbose output");
println!(" -f, --file <path> Use specified .nix file instead of <nixpkgs>");
std::process::exit(NmanError::Usage.code());
}
Action(action) => match action {
CliAction::Man(attr, section, page) => match main.open_man_page(attr, section, page) {
CliAction::Man { attr, section, page, file_path } => match main.open_man_page(attr, section, page, file_path) {
Ok(_) => (),
Err(t) => {
let msg = t.msg();
Expand Down Expand Up @@ -304,13 +360,24 @@ impl Main {
attr: &'a str,
section: Option<&'a str>,
page: &'a str,
file_path: Option<&'a str>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should clarify that we treat this as a subset of Nix syntax which is passed on, hence we don't use Path.

) -> Result<(), NmanError<'a>> {
let tmpdir = TempDir::new("nman").map_err(NmanError::IO)?;
// TODO(sterni): allow selecting other base package sets,
// like <vuizvui>, /home/lukas/src/nix/nixpkgs, …
let nix_import = match file_path {
Some(path) => {
if path.starts_with('/') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also check if the path matches ~/… or <…> which are also valid Nix paths.

I guess we should also clarify in the man page that we allow this special path syntax.

Could be nice to add a check whether it is unnecessary to add the ./ prefix, but it also doesn't really matter.

// Absolute path - strip trailing slashes
path.trim_end_matches('/').to_string()
} else {
// Relative path - prepend ./
format!("./{}", path)
}
},
None => "<nixpkgs>".to_string(),
};
let expr = format!(
"with (import <nixpkgs> {{}}); builtins.map (o: {}.\"${{o}}\") {}.outputs",
attr, attr
"with (import {} {{}}); builtins.map (o: {}.\"${{o}}\") {}.outputs",
nix_import, attr, attr
);
let inst = self
.debug_log_command(
Expand Down Expand Up @@ -603,6 +670,13 @@ fn match_man_page_file(name: &str, section: &str, page: &str) -> bool {
}
}

/// Extract the page name from a dotted attribute path.
/// For example, "pkgs.profpatsch.nman" becomes "nman".
/// If there are no dots, returns the original string.
fn extract_page_name_from_attr(attr: &str) -> &str {
attr.split('.').last().unwrap_or(attr)
}

/// Check if a string describes a man section,
/// i. e. is a number or "3p" (Perl Developer's
/// manual). Used to distinguish between man pages
Expand Down Expand Up @@ -693,6 +767,24 @@ mod tests {
assert!(parse_man_section("").is_err());
}

#[test]
fn test_extract_page_name_from_attr() {
// Simple case without dots
assert_eq!(extract_page_name_from_attr("nman"), "nman");
assert_eq!(extract_page_name_from_attr("hello"), "hello");

// Dotted attribute paths
assert_eq!(extract_page_name_from_attr("pkgs.profpatsch.nman"), "nman");
assert_eq!(extract_page_name_from_attr("a.b.c.d"), "d");
assert_eq!(extract_page_name_from_attr("nixpkgs.hello"), "hello");

// Edge cases
assert_eq!(extract_page_name_from_attr(""), "");
assert_eq!(extract_page_name_from_attr("."), "");
assert_eq!(extract_page_name_from_attr("package."), "");
assert_eq!(extract_page_name_from_attr(".package"), "package");
}

#[test]
fn test_output_preference() {
// lower =^= preferred
Expand Down
3 changes: 1 addition & 2 deletions pkgs/sternenseemann/temp/temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ use std::path::Path;

// libc interaction

#[link(name = "c")]
extern {
extern "C" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to be explicit?

Suggested change
extern "C" {
#[link(name = "c")]
extern "C" {

fn mkdtemp(template: *mut u8) -> *mut u8;
fn mkstemp(template: *mut u8) -> *mut u8;
}
Expand Down