-
Notifications
You must be signed in to change notification settings - Fork 11
pkgs/profpatsch/nman: ✨ add --file flag to use local nix file #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
fecb29f
ffb8977
de9c554
76c7c94
3b1820e
0aafc7f
9b9a8a5
f7c85c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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> { | ||
|
|
@@ -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() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit icky.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure how to do it better tbh
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does |
||
| 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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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!!!!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also return a |
||
| cli_res = match positional_args.len() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 }; | ||
|
|
@@ -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(); | ||
|
|
@@ -304,13 +360,24 @@ impl Main { | |
| attr: &'a str, | ||
| section: Option<&'a str>, | ||
| page: &'a str, | ||
| file_path: Option<&'a str>, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| ) -> 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('/') { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should also check if the path matches 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 |
||
| // 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( | ||
|
|
@@ -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 { | ||
sternenseemann marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
|
|
@@ -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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,8 +32,7 @@ use std::path::Path; | |||||||
|
|
||||||||
| // libc interaction | ||||||||
|
|
||||||||
| #[link(name = "c")] | ||||||||
| extern { | ||||||||
| extern "C" { | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be better to be explicit?
Suggested change
|
||||||||
| fn mkdtemp(template: *mut u8) -> *mut u8; | ||||||||
| fn mkstemp(template: *mut u8) -> *mut u8; | ||||||||
| } | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
importbut if we want to allow arbitrary expressions we’d have to escape them properly firstThere was a problem hiding this comment.
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.