dircolors: add nushell integration

[jaredmontoya]

Description

Adds nushell support to dircolors module

Checklist

• Change is backwards compatible.

• Code formatted with nix fmt or ./format.

• Code tested through nix-shell --pure tests -A run.all
  or nix build --reference-lock-file flake.lock ./tests#test-all using Flakes.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.

Maintainer CC

@justinlovinger

[justinlovinger]

Half of these changes are an unnecessary refactor, unrelated to Nushell integration. Specifically, all the lines related to replacing home.file.".dir_colors".text with home.file.".dir_colors".source. Those changes should be removed.

You appear to be adding support for running tests on Darwin. That should not be in the same commit as adding Nushell integration. I question whether its necessary at all, but I'm not sure what the precedent is for tests supporting Darwin.

I disagree with how Nushell integration is implemented here. You appear to be running a Bash script at rebuild-time, which generates a Nushell script, which sets LS_COLORS based on static values generated during rebuild-time. This is a hack. A better solution would be to use Nushell to parse the output of dircolors.

Although, now that I think about it, using Nushell to parse the output of dircolors isn't a good solution either. This module should be about dircolors. It shouldn't include complex logic for a specific shell, especially when that shell doesn't have a stable API yet. Integration was added for shells that dircolors itself supports. We shouldn't add our own logic for shells dircolors doesn't support.

In conclusion, I recommend closing this pull-request.

[jaredmontoya]

Half of these changes are an unnecessary refactor, unrelated to Nushell integration. Specifically, all the lines related to replacing home.file.".dir_colors".text with home.file.".dir_colors".source. Those changes should be removed.

No, they are not unnecessary, if I remove them I won't be able to use the /nix/store path to the dircolors config. It is guaranteed to be accessible during pkgs.runCommand

You appear to be adding support for running tests on Darwin. That should not be in the same commit as adding Nushell integration. I question whether its necessary at all, but I'm not sure what the precedent is for tests supporting Darwin.

I don't use Darwin and couldn't care less about Apple as a whole. But most nushell related tests I saw in the home-manager codebase use this kind of logic for Darwin compatibility. I can remove the if else check that implements Darwin support but why do that? It will introduce inconsistency between the way nushell tests are implemented.

I disagree with how Nushell integration is implemented here. You appear to be running a Bash script at rebuild-time, which generates a Nushell script, which sets LS_COLORS based on static values generated during rebuild-time. This is a hack. A better solution would be to use Nushell to parse the output of dircolors.

Well, it may be called a hack but it turns whatever the output of dircolors is to what nushell directly understands and does all of that at build-time. Referencing your last point, that output is probably what you want dircolors to output with a dircolors -nu flag.

If I implement a parser in nu that parses the output of dircolors -c at runtime and strips setenv LS_COLORS ' and ' from both sides of the ouput at runtime each time you start your shell. It will be worse than the current version.

In my opinion this nushell integration is technically superior to all dircolors integrations for other shells because it just adds necessary LS_COLORS definition to the nushell configuration and that's it. It doesn't need to fetch the dircolors binary from disk at runtime and then run it every time you open your shell in order to convert data that is stored in ~/.config/dir_colors in another format. The immutability of that dircolors output that is baked into nushell configuration at Nix rebuild time doesn't matter because ~/.config/dir_colors is just as baked in and will only change together with the immutable value in the nushell config.

Although, now that I think about it, using Nushell to parse the output of dircolors isn't a good solution either. This module should be about dircolors. It shouldn't include complex logic for a specific shell, especially when that shell doesn't have a stable API yet. Integration was added for shells that dircolors itself supports. We shouldn't add our own logic for shells dircolors doesn't support.

Something is telling me that getting nushell supported by dircolors (a coreutils program) is going to be a lot harder than opening a PR to home-manager.

You are right that nushell is unstable and unsupported by a lot of stuff, but in my experience when you combine Nix+Home-Manager with Nushell it can become the best shell. I never had a shell so fast and snappy yet functional before I tried Nushell+Nix. And I think nushell could start even faster and snappier if more nushell integrations were implemented in the same way as the one in this PR.
And I beleive this difference will be a lot more significant when running it on a slow sd card in a 1GB RAM raspberry pi.

There are still some things that Nix can't fix. Like for example atrocious VIM emulation in Nushell, but I am sure at some point it will be fixed and bash/zsh don't even have VIM emulation built in so it's an unfair comparison.

So what I am trying to say here is that it's true that nushell needs development, but If nobody is going to try it and stick around then there likely won't be much development. I am trying to make it as easy as possible to use nushell with home manager by upstreaming whatever I already invented in my configuration that helps this goal.

It's very easy to try things out with Nix by changing a couple of configuration options and go back if you don't like it. This is how Nix encourages experimentation. But if you change those configuration options and suddenly dircolors and 20 other integrations don't work for some reason, you will just go back to what you had before walking away disappointed. (Unless it is me)

[justinlovinger]

I don't disagree that your solution is more efficient than running dircolors at runtime, I disagree that the Home Manager module for dircolors is the right place for it. Because of the vast scope of Home Manager, individual modules need to remain simple for easy maintenance. Using Nushell code is especially problematic because the API isn't stable yet.

[jaredmontoya]

You may be right, but I don't know of a better place. Also there are already quite a few more home manager integrations that use A LOT more Nu code than export-env { $env.LS_COLORS = value } here which is unlikely to break. Also it's not like there is anything complex done in this PR.

[jaredmontoya]

@khaneliman Your opinion on this?

''
  eval "$(${lib.getExe' cfg.package "dircolors"} -b ${dircolorsConfig})"
  echo "export-env { \$env.LS_COLORS = \"$LS_COLORS\" }" >> $out
''

came from #6941 (review)

[khaneliman]

I don't disagree that your solution is more efficient than running dircolors at runtime, I disagree that the Home Manager module for dircolors is the right place for it. Because of the vast scope of Home Manager, individual modules need to remain simple for easy maintenance. Using Nushell code is especially problematic because the API isn't stable yet.

I'm not really understanding the concern here atm... it's an optional integration to an optional shell just to populate LS_COLORS with the dircolors generated palette?