Skip to content

Conversation

@Profpatsch
Copy link
Member

Add support for using ./default.nix instead of when the --local or -l flag is specified. This allows viewing man pages for packages defined in local development environments or custom package sets.

Changes:

  • Add use_local field to Main struct
  • Parse --local/-l command line option
  • Conditionally use ./default.nix in Nix expression generation
  • Update help text and man page documentation with new option and examples
  • Maintain backward compatibility (default behavior unchanged)

🤖 Generated with Claude Code

Profpatsch and others added 2 commits July 3, 2025 16:36
Add support for using ./default.nix instead of <nixpkgs> when the
--local or -l flag is specified. This allows viewing man pages for
packages defined in local development environments or custom package sets.

Changes:
- Add use_local field to Main struct
- Parse --local/-l command line option
- Conditionally use ./default.nix in Nix expression generation
- Update help text and man page documentation with new option and examples
- Maintain backward compatibility (default behavior unchanged)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
When using dotted attribute paths like `pkgs.profpatsch.nman`, nman was
incorrectly trying to find a man page named "pkgs.profpatsch.nman" instead
of extracting the actual page name "nman" from the last component.

Changes:
- Add extract_page_name_from_attr() helper that splits on '.' and returns last component
- Update CLI parsing to use helper for auto-detected page names (cases 2 and 3 args)
- Add comprehensive test coverage for the helper function
- Maintain backward compatibility for simple attribute names and explicit page names
- Add description to the man page

Now `nman --local pkgs.profpatsch.nman` correctly builds the full attribute
but searches for the "nman" man page instead of "pkgs.profpatsch.nman".

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@Profpatsch Profpatsch force-pushed the nman-improvements branch from a192a6f to ffb8977 Compare July 3, 2025 17:14
Copy link
Contributor

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

I think the specific --local flag is a bit silly. I'd prefer just adding -f/--file like the new Nix/Lix CLI does. Then you'd write nman -f . mypkg.

@Profpatsch
Copy link
Member Author

I special-cased it for . now, since I have no clue about the nix -f semantics and really kinda don’t want to know atm haha

Profpatsch and others added 2 commits August 4, 2025 17:31
Refactor the inflexible --local flag to use -f/--file <path> syntax,
matching modern Nix CLI conventions. This allows specifying any .nix
file instead of hardcoding ./default.nix.

Usage: nman -f . mypkg (equivalent to old --local behavior)
       nman -f /path/to/file.nix mypkg (new flexibility)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Unify the logic for determining the import path and generate the Nix
expression only once, reducing code duplication and making the logic
clearer.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@Profpatsch Profpatsch changed the title pkgs/profpatsch/nman: ✨ add --local flag to use local default.nix pkgs/profpatsch/nman: ✨ add --file flag to use local nix file Aug 4, 2025
.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.

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.

}

// Only process positional arguments if we haven't already set an error
if let ShowUsage { err_msg: Some("Unexpected number of arguments") } = 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.

We are matching an error message to detect that no error occurred? This is extremely brittle and also super confusing code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it should not match on the exact error, any error should skip

Newer rust versions apparently can specify the extern directly without
the link annotation (was a warning).
This should pass the right kinds of path to nix in most situations.
Profpatsch and others added 2 commits August 15, 2025 14:37
Replace tuple-style CliAction::Man with inline struct fields for better
readability and maintainability. Move file_path from Main struct to the
action itself for better separation of concerns.

Changes:
- Use named fields (attr, section, page, file_path) instead of positional args
- Keep efficient &'a str lifetimes for all string references
- Pass file_path explicitly to open_man_page method
- Remove file_path from Main struct

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
When we match positional args, we want to skip whenever we are already
in the ShowUsage case.

#[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" {

// 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.

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.

}

// 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>.


// Only process positional arguments if we haven't already set an error
if let ShowUsage { err_msg: _ } = cli_res {
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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants