nixos/environment: make default shell aliases overridable #28072
Conversation
This fixes issue [NixOS#24560](NixOS#24560) by moving the predefined shellAliases to the `environment.shellAliases` option definition. I also used a custom merge function, such that the defaults only get overridden when the user defines an alternative.
|
@TomSmeets, thanks for your PR! By analyzing the history of the files in this pull request, we identified @shlevy, @oxij and @edolstra to be potential reviewers. |
| type = types.attrs // { | ||
| # merge aliases defined by the user and the defaults | ||
| merge = loc: defs: default // types.attrs.merge loc defs; | ||
| }; |
There was a problem hiding this comment.
I'm not sure about this, it forces the user to have these aliases. If one wants to keep the defaults while adding more, they could always do shellAliases = { gst = "git status"; } // options.environment.shellAliases.default; instead.
There was a problem hiding this comment.
yes, I was not sure what to do here.
But to me it seems a bit strange that if you just want to add a new alias, that the defaults disappear.
Maybe we should move these entirely to example? (this would affect existing systems)
Or maybe add something like a environment.useDefaultShellAliases option, which is set to true by default.
There was a problem hiding this comment.
Another option would be an extraAliases option, which has already become somewhat standard for config files (config/extraConfig).
But I ultimately don't think it's worth adding another option just for this rare use case. Just adding a section to the manual on how to refer to the default values (with this as an example) would be sufficient imo.
| ls = "ls --color=tty"; | ||
| ll = "ls -l"; | ||
| l = "ls -alh"; | ||
| }; | ||
| example = { ll = "ls -l"; }; |
There was a problem hiding this comment.
Maybe something other than a default could be used here, e.g. example = { gst = "git status"; }
I also added some documentation and a better example expression.
|
@infinisil I saw no need to add an extra section to the nixos manual. Please tell me what you think. |
| description = '' | ||
| An attribute set that maps aliases (the top level attribute names in | ||
| this option) to command strings or directly to build outputs. The | ||
| aliases are added to all users' shells. | ||
|
|
||
| <note><para> |
There was a problem hiding this comment.
You changed description, have you tried building manual?
There was a problem hiding this comment.
@danbst
Yes I have build the manual, and everything looks good.
However, I couldn't find any documentation on the available tags.
I used existing options as a reference.
| <code> | ||
| environment.shellAliases = { | ||
| gst = "git status"; | ||
| } // options.environment.shellAliases.default; |
There was a problem hiding this comment.
Nitpick: would be better to use
environment.shellAliases = options.environment.shellAliases.default // {
gst = "git status";
}because otherwise the default aliases would override your customized ones.
| ''; | ||
| type = types.attrs; # types.attrsOf types.stringOrPath; | ||
| type = types.attrs; |
There was a problem hiding this comment.
This should use types.attrsOf types.str or something slightly less restrictive if necessary. That will make merges work like expected and I think also enables using mkDefault for attributes, like this:
{
environment.shellAliases =
{ ls = mkDefault "ls --color=tty";
ll = mkDefault "ls -l";
l = mkDefault "ls -alh";
};
}
|
A similar pull request has been merged: #44441 |
Motivation for this change
This fixes issue #24560 by moving the predefined shellAliases to the
environment.shellAliasesoption definition.I also used a custom merge function, such that the defaults only get overridden when the user defines an alternative.
Things done
Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)