Skip to content
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

treewide: refactor neovim-flake #225

Closed
wants to merge 30 commits into from
Closed

treewide: refactor neovim-flake #225

wants to merge 30 commits into from

Conversation

NotAShelf
Copy link
Owner

@NotAShelf NotAShelf commented Feb 17, 2024

Okay this has been long overdue, and I am finally pissed enough to spend what's left of my free time (and energy) on aggressively refactoring neovim-flake to have a sane and easy to grasp structure for contributors and maintainers alike.

This PR is that.

DONE

  1. Cleans up the initial root directory of the project.
    • Files that belong in their respective directories (e.g. assets) have been moved
  2. Moves home-manager module to a directory dedicated to modules, accommodating for the possibilities of NixOS and Darwin modules in the future.
    • lib/modules/default.nix -> flake/modules/hm-module.nix
  3. Reduces dependency cycles and layered imports in modules/
    • All plugins have been moved to modules/plugins/
    • Option definitions separated from configurations
    • Basic and Core have been isolated as öodules/core and modules/neovim (previously basic)`
      • core will contain wrapper and build related files
      • neovim will contain the most basic vim/neovim options that are passed to base configurations.
  4. Cleans up and re-organizes lib so that files and functions are easier to search by name.
    • Adds temporary `bool.nix for boolean operations. We will need to revisit structure later on.

In Progress

  • Make lib usage more explicit => Instead of inherit (lib) concatStringsSep, we use inherit (lib.strings) concatStringsSep to ensure clarity. This slightly clutters the beginning of each file, but it's more understandable where exactly the function comes from (especially when working with the extended lib)
    • Plugins converted:
      • assistant
      • autopairs
      • comments
      • completion
      • dashboard
        • alpha
        • dashboard-nvim
        • startify

TODO/TBD

Things I want to do:

  • Move neovim config builder/helper functions into lib, and call from lib where necessary. This has been an issue while implementinig the home manager module, and I'd like to avoid repeating the same mistake again.

Things I might want to do:

  • Get rid of inputs per-plugin. It's a nice idea, but flakes suck. Hundreds of inputs that a user will likely not even think about overriding in their whole usage of this flake is, I assume, burdening. If my fellow co-maintainers @FrothyMarrow and @horriblename agree; I'd like to fall back to either customizable package options, or a packages list that gets passed to Neovim wrapper.
  • Switch to makeNeovimConfig, or move to "unstable" neovim wrapper - which is not documented (???)
  • Other suggestions by contributors and maintainers alike

@NotAShelf NotAShelf added enhancement New feature or request help wanted Extra attention is needed TBD More discussion and research is in order labels Feb 17, 2024
@NotAShelf NotAShelf changed the base branch from main to v0.6 February 19, 2024 10:22
@horriblename
Copy link
Collaborator

Get rid of inputs per-plugin

no objections, I feel like inputs would be good for stability if we only had a small set of well tested plugins, but we're way past that point now

@NotAShelf
Copy link
Owner Author

I'd like to get rid of the input system that we have now, but I also would like to avoid having to pin/overlay plugins to get their latest versions. Nixpkgs usually lags back a month or two, and an individual override for each plugin that's behind master would be annoying. Perhaps we should reconsider inputs (but automate adding plugin inputs to the list to reduce a development step) or consider something like npins?

@NotAShelf NotAShelf added this to the 0.6 milestone Feb 19, 2024
@elijahimmer
Copy link
Contributor

elijahimmer commented Feb 20, 2024

now, but I also would like to avoid having to pin/overlay plugins to get their latest versions. Nixpkgs usually lags back a month or two, and an individual override for each plugin that's behind master would be annoying.

Why cannot we just use nixpkgs and have the user deal with what version of the packages?
They can override nixpkgs with whichever version they want to pin it to, like nixpkgs-unstable, and we can just upstream any of the packages nixpkgs doesn't have.

It would make the inputs only nixpkgs, the flake specific stuff, and anything that shouldn't be in nixpkgs.

There would be some difficulties with transitioning to it, like people on nixpkgs stable won't get the new packages for a while, but I think that may be the simplest way to do it.
(that also means that this repo won't have to vet every input update itself, it can lean on nixpkgs a little)

@NotAShelf
Copy link
Owner Author

Why cannot we just use nixpkgs and have the user deal with what version of the packages?

Leaving the responsibility of dealing with the packages is annoying for the user. There can be a new configuration option, or a configuration option removed outside user's knowledge that would cause neovim to spit a whole bunch of errors, potentially confusing the user.

As much as I dislike having a whole bunch of inputs (that's an understatement tbh), it allows us to pin package versions for our convenience. If the user wants to override (e.g. go back, or upgrade before us), it's easy as overriding an input because the plugins are built from their inputs.

@NotAShelf
Copy link
Owner Author

NotAShelf commented Feb 23, 2024

@horriblename could you rebase #228, #229 and #230 around this PR? It's creating a lot of merge conflicts and I think it's better to deal with them now than to wait for me to finish refactoring and then deal with the consequences altogether.

@NotAShelf NotAShelf closed this Feb 26, 2024
@NotAShelf NotAShelf deleted the restructure branch February 26, 2024 04:04
@NotAShelf
Copy link
Owner Author

Being continued in #231

@NotAShelf NotAShelf removed enhancement New feature or request help wanted Extra attention is needed TBD More discussion and research is in order labels Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cancelled
Development

Successfully merging this pull request may close these issues.

4 participants