Skip to content

Revert #29 - #34

Merged
vic merged 3 commits into
denful:mainfrom
jbgi:revert-29
Oct 18, 2026
Merged

vic merged 3 commits into
denful:mainfrom
jbgi:revert-29

Conversation

@jbgi

@jbgi jbgi commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

#29 was merged following a regression I noticed, but I now think it was due to me wrongly the api, hence the revert.

Indeed the idiom of returning another input as the outputs
nixpkgs = with-inputs.from.flake inputs.home-manager { } (i: i.nixpkgs) is much better written:
inherit (with-inputs.from.flake inputs.home-manager {}) nixpkgs;

So reverting #29 in order to keep the same override logic inside and outside the "outputs" function, which is much cleaner imo.

Added doc to document this.

@sini sini left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Using the same override logic inside and outside the outputs function is cleaner, and the inherit (… from.flake inputs.home-manager {}) nixpkgs idiom covers the #26 case. topLevelInputs and nonEmptyInputs were indeed unused. just ci passes (51/51), and so does this branch merged with #32 and #33 (52/52).

Nits:

  • Typo inline.
  • Worth a test for the documented inherit idiom, so the #26 case stays covered now that #29's reproducer is gone.
  • The removed "Flake backed by non-flake pins" section was inaccurate, but the mechanism it was describing still exists: indirect, built from builtins.functionArgs flake.outputs. Maybe give it a one-line mention so it isn't undocumented.

Comment thread README.md Resolved
@jbgi

jbgi commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author
  • Typo inline.

fixed.

I'll add the test in #33.

  • The removed "Flake backed by non-flake pins" section was inaccurate, but the mechanism it was describing still exists: indirect, built from builtins.functionArgs flake.outputs. Maybe give it a one-line mention so it isn't undocumented.

the AI got confused here, the obsolete removed section had nothing to do with builtins.functionArgs flake.outputs...

@jbgi
jbgi requested a review from sini October 18, 2026 08:26
@vic
vic merged commit 641d01e into denful:main Oct 18, 2026
2 checks passed
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.

3 participants