nix-community / home-manager

Manage a user environment using Nix [maintainer=@rycee]
https://nix-community.github.io/home-manager/
MIT License
6.78k stars 1.77k forks source link

bug: nushell module configuration overwrites default basic configuration #5527

Open nyabinary opened 3 months ago

nyabinary commented 3 months ago

Are you following the right branch?

Is there an existing issue for this?

Issue description

So the home-manager module for nushell is actually destructive, which is not an idea as it overwrites the original nushell configuration which most users probably don't want since it breaks nushell. Rather, users want to most likely append the config to add their config. image image My config for clarity is:

    programs.nushell = {
      enable = true;
      environmentVariables = {
        NIXOS_OZONE_WL = "1";
        ELECTRON_OZONE_PLATFORM_HINT = "auto";
        EDITOR = "hx";
        VISUAL = "hx";
        TERM = "rio";
      };
      shellAliases = {
        switch = "sudo nixos-rebuild switch";
        boot = "sudo nixos-rebuild boot";
      };
      extraConfig = ''
        $env.config = {
          show_banner: false,
        }
      '';
    };

Maintainer CC

@Philipp-M

System information

- system: `"x86_64-linux"`
 - host os: `Linux 6.9.3, NixOS, 24.11 (Vicuña), 24.11.20240607.051f920`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.23.0pre20240526_7de033d6`
 - nixpkgs: `/nix/store/3dr5pyja36lvvrszhzffww1jwyrx6i09-source`
Philipp-M commented 3 months ago

I do think this is intended behavior as the name of either programs.nushell.{envFile,configFile}, suggests that the whole file is written, and we cannot cleanly (side-effects) just extend the default file at that path, which may not even exist when nushell was not yet started, and the user confirmed creation of such files.

But it's a little bit unfortunate, that nushell somewhat "relies" on a default file at that path as well to provide the expected UX. Instead of using its own (internally hardcoded) defaults that can be overwritten with the user-specified file... It's also not optimal that enableNushellIntegration in a few other programs seems to set extraEnv/extraConfig which in turn leads to writing the envFile/configFile.

I think the defaults are too long to be included in this module, as I think it will be very annoying to keep this up-to-date. Maybe we can fetch the defaults from nushell directly from here, but just looking at the commit history, these files are updated somewhat often, so I'm not sure whether that's a good idea (because I'm not able to maintain this), when we do this, it should likely be an extra option useDefaultConfig (default = true) to toggle this.

So all in all I think this is a wontfix, but we can warn the user in the docs, and link them to where the defaults are, when they use the nushell module (or enableNushellIntegration in other modules) You can do something like this to support this in your config, I think that could be added to the example of that option (PR welcome).

programs.nushell.configFile.source = builtins.fetchurl {
  url = "https://raw.githubusercontent.com/nushell/nushell/f2f4b83886d0060b93eef49baac1bb3ce18d42af/crates/nu-utils/src/sample_config/default_config.nu";
  sha256 = "sha256:0mdbl66g8ivhm4yn2rasswdcxrxijmsa27a75kkblfbjbymnb46w";
};
nyabinary commented 3 months ago

I do think this is intended behavior as the name of either programs.nushell.{envFile,configFile}, suggests that the whole file is written, and we cannot cleanly (side-effects) just extend the default file at that path, which may not even exist when nushell was not yet started, and the user confirmed creation of such files.

But it's a little bit unfortunate, that nushell somewhat "relies" on a default file at that path as well to provide the expected UX. Instead of using its own (internally hardcoded) defaults that can be overwritten with the user-specified file... It's also not optimal that enableNushellIntegration in a few other programs seems to set extraEnv/extraConfig which in turn leads to writing the envFile/configFile.

I think the defaults are too long to be included in this module, as I think it will be very annoying to keep this up-to-date. Maybe we can fetch the defaults from nushell directly from here, but just looking at the commit history, these files are updated somewhat often, so I'm not sure whether that's a good idea (because I'm not able to maintain this), when we do this, it should likely be an extra option useDefaultConfig (default = true) to toggle this.

So all in all I think this is a wontfix, but we can warn the user in the docs, and link them to where the defaults are, when they use the nushell module (or enableNushellIntegration in other modules) You can do something like this to support this in your config, I think that could be added to the example of that option (PR welcome).

programs.nushell.configFile.source = builtins.fetchurl {
  url = "https://raw.githubusercontent.com/nushell/nushell/f2f4b83886d0060b93eef49baac1bb3ce18d42af/crates/nu-utils/src/sample_config/default_config.nu";
  sha256 = "sha256:0mdbl66g8ivhm4yn2rasswdcxrxijmsa27a75kkblfbjbymnb46w";
};

Something like useDefaultConfig sounds good also, maybe just an option to disable the banner? (since I only want to change that from the default config anyways)

tranquillity-codes commented 2 months ago

For future reference:

xdg.configFile."nushell/config.nu".text = builtins.readFile "${pkgs.fetchFromGitHub (with pkgs.nushell.src; {owner=owner; repo=repo; rev=rev; hash=outputHash;})}/crates/nu-utils/src/sample_config/default_config.nu" + ''
# Your custom config.nu overrides
'';
xdg.configFile."nushell/env.nu".text = builtins.readFile "${pkgs.fetchFromGitHub (with pkgs.nushell.src; {owner=owner; repo=repo; rev=rev; hash=outputHash;})}/crates/nu-utils/src/sample_config/default_env.nu" + ''
# Your custom env.nu overrides
'';

Should work.