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

bug: nushell module configuration overwrites default basic configuration #5527

Open
2 tasks done
nyabinary opened this issue Jun 10, 2024 · 3 comments · May be fixed by #6184
Open
2 tasks done

bug: nushell module configuration overwrites default basic configuration #5527

nyabinary opened this issue Jun 10, 2024 · 3 comments · May be fixed by #6184
Assignees
Labels
bug triage Issues or feature request that have not been triaged yet

Comments

@nyabinary
Copy link

Are you following the right branch?

  • My Nixpkgs and Home Manager versions are in sync

Is there an existing issue for this?

  • I have searched the existing issues

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`
@nyabinary nyabinary added bug triage Issues or feature request that have not been triaged yet labels Jun 10, 2024
@Philipp-M
Copy link
Contributor

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
Copy link
Author

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Issues or feature request that have not been triaged yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants