Skip to content

Conversation

MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Oct 17, 2024

  • modules/nixpkgs: restructure nixpkgs.pkgs.isDefined assertion
  • modules/nixpkgs: add overlays option

Follow up to #2301 and working towards #1784; this PR adds the nixpkgs.overlays option.

This change is supported by some related refactoring to the existing assertion to a) reduce future indentation changes to the module and b) ensure the assertion message is actually shown to users. Without this change, _module.args.pkgs being absent will cause other errors that get thrown before we are able to check the module assertions.

Additionally, I've added a test to ensure the new option works correctly.

I would like a better option example for nixpkgs.overlays. Currently I'm using the same example as NixOS. Perhaps we could do something using the vimPlugins = prev.vimPlugins.extend pattern? IDK, struggling to find "useful" real-world examples that are small and obvious enough to be a good example.

@MattSturgeon MattSturgeon requested a review from a team October 17, 2024 19:18
Comment on lines 73 to 109
[
(self: super: {
openssh = super.openssh.override {
hpnSupport = true;
kerberos = self.libkrb5;
};
})
]
Copy link
Member Author

Choose a reason for hiding this comment

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

Any ideas for a better vim-related example?

Copy link
Contributor

Choose a reason for hiding this comment

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

Was wondering if we could do one that adds a tree-sitter-X grammar to the treesitter package ?

Copy link
Contributor

@khaneliman khaneliman Oct 18, 2024

Choose a reason for hiding this comment

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

        (self: super: { neovim-unwrapped = inputs.neovim-nightly-overlay.packages.${system}.default; })

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a few examples. Happy to iterate further on them and/or remove some of them. Either in this PR or a follow up.

We can throw inline to ensure our error is displayed before any attempt
to use the (non-existent) `pkgs` arg results in a less helpful error.
Based on the `nixpkgs.overlays` option available in NixOS, allows users
to further customize the `pkgs` instance used when evaluating nixvim.

The standard module tests are now provided a few extra module args to
enable a test where we instantiate a custom nixpkgs instance.
Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

LGTM

@MattSturgeon
Copy link
Member Author

@mergify queue

Copy link
Contributor

mergify bot commented Oct 19, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c4ad4d0

Copy link
Contributor

mergify bot commented Oct 19, 2024

This pull request, with head sha c4ad4d0b2e7de04fa9ae0652b006807f42062080, has been successfully merged with fast-forward by Mergify.

This pull request will be automatically closed by GitHub.

As soon as GitHub detects that the sha c4ad4d0b2e7de04fa9ae0652b006807f42062080 is part of the main branch, it will mark this pull request as merged.

It is possible for this pull request to remain open if this detection does not happen, this usually happens when a force-push is done on this branch nixpkgs_overlays, this means GitHub will fail to detect the merge.

@mergify mergify bot merged commit c4ad4d0 into nix-community:main Oct 19, 2024
4 checks passed
@mergify mergify bot temporarily deployed to github-pages October 19, 2024 20:23 Inactive
@MattSturgeon MattSturgeon deleted the nixpkgs_overlays branch October 19, 2024 20:24
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.

2 participants