nixos/defaults: init #101127
nixos/defaults: init #101127
Conversation
deviant
requested review from
adisbladis,
edolstra,
infinisil,
nbp and
ttuegel
as code owners
ofborg
bot
added
6.topic: emacs
6.topic: GNOME
|
(rebased) |
|
(rebased) |
This adds a new passthru to all text editor derivations, analogous to the shellPath passthru, containing a string suitable to be used as the $EDITOR environment variable. Editors that daemonise by default are passed flags that stop them from doing so. This covers every single text editor in nixpkgs. IDEs were not given such treatment, with the rationale that they are intended to be used for editing entire projects - and often can't be configured to open single files or disable daemonisation. Similarly, task-specific text editors (such as Markdown editors) were skipped.
This adds a new passthru to all pager derivations, containing a string suitable to be used as the $PAGER environment variable.
This introduces a new set of options for configuring the default editor and pager. users.defaults.editor can be set to any derivation with the 'editorCommand' attribute, which is a string suitable for being used as the $EDITOR variable. Similarly, the option users.defaults.pager can be set to any derivation with the 'pagerCommand' attribute, and controls the $PAGER variable. Of note: programs invoking $EDITOR and $PAGER expect the respective program to not exit until the user has finished editing/viewing the file, which means that any program that daemonises by default (such as Atom or Sublime Text) needs an additional flag to be passed. Such flags are specific to the program in question, and are usually documented in manual pages and --help output. This also means that the system path no longer implicitly contains nano and less, which is probably welcome news for anyone with particularly passionate opinions about text editors ;)
Since users.defaults is intended to hold options for selecting default programs, it makes sense to deprecate users.defaultUserShell and incorporate its functionality here.
This module is made redundant by the users.defaults.editor option.
This removes duplication of functionality provided by users.defaults.editor, and sets that instead.
This removes duplication of functionality provided by users.defaults.editor, and sets that instead. It also makes defaultEditor actually do something, since it wasn't previously used.
|
(simplified how to configure different commands + rebased) |
| packageWithAttr = type: attr: package // { | ||
| name = "${type}Package"; | ||
| description = "${type} package"; | ||
| check = x: package.check x && hasAttr attr x; |
There was a problem hiding this comment.
Can we generate a meaningful error message here? I know that this will complain about the invalid type but as it is likely that someone adds a new editor without knowing about the new passthru's it might be good to tell the user what is required.
There was a problem hiding this comment.
I think the most meaningful error message we're likely to end up with is "package with the passthru ${attr}", and I'm unsure as to whether that's any more clear than the error that's already produced.
There was a problem hiding this comment.
I was thinking about something like:
The package you provided as defaults.editor does not have the required attribute `editorCommand`. Please select the correct package or add the passthru to the package as documented in the option.
NIx is often already bad enough user experience for new users so it is probably worth being helpful in error messages.
| @@ -26,6 +26,22 @@ | |||
| <listitem> | |||
| <para>GNOME desktop environment was upgraded to 3.38, see its <link xlink:href="https://help.gnome.org/misc/release-notes/3.38/">release notes</link>.</para> | |||
| </listitem> | |||
| <listitem> | |||
There was a problem hiding this comment.
In addition to the nixos release notes we should probably mention it in the stdenv docs so new contributors will be able to read up on it. We already document passthur.updateScript there so this wouldn't be too weird.
There was a problem hiding this comment.
Is there a place in the documentation that guides people through how to add a new editor/pager/shell/specific type of package, already? I don't remember seeing anything of that sort, and it'd be more easily discoverable there.
There was a problem hiding this comment.
I don't think there is how to package XYZ in the documentation but rather documentation on our build system where we already document other attributes of the passthru's. Due to a lack of a better section I'd stick it in there so it isn't undocumented. If, at some point, we have more specific documentation that could be moved.
| the editorCommand passthru of the package. For example: | ||
|
|
||
| <programlisting> | ||
| users.defaults.editor = pkgs.vim // { editorCommand = "gvim -f"; }; |
There was a problem hiding this comment.
Should we encourage users to open a bug report if that attribute is missing?
There was a problem hiding this comment.
In the options documentation?
There was a problem hiding this comment.
I am not sure! I was just thinking about that when I saw the suggested ad-hoc set merging,.
There was a problem hiding this comment.
Do you feel the description isn't clear enough about that being the way to override the attribute instead of merely adding it? I could perhaps reword that a bit if you feel it's ambiguous.
There was a problem hiding this comment.
maybe something like this:
description = ''
The package to use as the default editor.
To change the <varname>$EDITOR</varname> variable, most
editors will be configured enough that, for example:
<programlisting>
users.defaults.editor = pkgs.vim;
</programlisting>
will be enough, but if you want to override what the package provides as editorCommand:
set the editorCommand passthru of the package. For example:
<programlisting>
users.defaults.editor = pkgs.vim // { editorCommand = "gvim -f"; };
</programlisting>
'';
| pkgs.less | ||
| pkgs.libcap | ||
| pkgs.nano |
There was a problem hiding this comment.
To anyone else reviewing: This is fine since <nixpkgs/nixos/modules/config/defaults.nix> sets those as defaults for the new configuration options.
| attributes, respectively. For instance: | ||
| <programlisting> | ||
| users.defaults = { | ||
| editor = pkgs.vim // { editorCommand = "gvim -f"; }; |
There was a problem hiding this comment.
Please add a note that this example overrides the default editorCommand set in vim. That most editors will have a good default configured.
Otherwise I'll bet we'll see pkgs.vim // {editorCommand = "vim"; }; in use :)
|
|
||
| pager = mkOption { | ||
| type = types.pagerPackage; | ||
| default = pkgs.less; |
There was a problem hiding this comment.
The previous default used less -R:
PAGER = mkDefault "less -R";
The current configuration for pkgs.less is simply less.
Shouldn't the default for pkgs.less be less -R and your description instead describe how to override it to the -R-less version? (Stating as the note above, that this overrides the default value)
| @@ -21,4 +21,6 @@ stdenv.mkDerivation rec { | |||
| license = licenses.gpl3; | |||
| maintainers = with maintainers; [ eelco dtzWill ]; | |||
| }; | |||
|
|
|||
| passthru.pagerCommand = "less"; | |||
There was a problem hiding this comment.
(See other comment)
Shouldn't this be less -R?
| <para> | ||
| <option>users.defaultUserShell</option> has been renamed to | ||
| <link linkend="opt-users.defaults.shell">users.defaults.shell</link>. | ||
| </para> |
There was a problem hiding this comment.
(Unless I'm mistaken, not tested yet)
In addition, the value must now be set to a package.
The previous implementation used:
type = types.either types.path types.shellPackage;
While now it only uses types.shellPackage.
The documentation stated:
This option defines the default shell assigned to user accounts. This can be either a full system path or a shell package.
This must not be a store path, since the path is used outside the store (in particular in /etc/passwd).
There was a problem hiding this comment.
This one's intentional. I added some fairly elaborate backcompat code to take care of the most common cases, and the description explains how to change the path if that isn't enough. The reason why this was even an either in the first place was actually due to further backcompat — hopefully 4 years is enough time for it to be reasonable to remove after. Additionally, it's less of a problem restricting the type slightly here, since people will have to update their configs after this change regardless.
Avoiding mentioning $EDITOR anywhere other than in the documentation for users.defaults.editorCommand will help people find the option more easily, and decrease the chances of it being overlooked.
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
Motivation for this change
This adds a new NixOS module for configuring the default editor and
pager of a system. It also deprecates
users.defaultUserShell, optingto handle all three in one place for consistency. As with #16189, it
makes use of passthru attributes on applicable packages: editorCommand
for $EDITOR, and pagerCommand for $PAGER.
This gives every text editor in nixpkgs first-class support for being
configured as the default editor, without needing to create a new module
for each and every one of them. This avoids the need for changes such as
#100132, which are now handled totally generically. Configuration can be
done via packages themselves (or wrappers), further alleviating the need
for additional modules.
Like with #97171, this allows for removing more things from the system
path: nano is no longer a hardcoded dependency, sitting around unused
by those with different editing tastes. Since it now has a dedicated
option, it's easy to find where to configure the default text editor.
Additionally, the default editor and pager packages are now added to the
system path in the same location that their corresponding variables are
set, simplifying the surrounding logic and making it easier to follow.
As noted in nixos/modules/programs/environment.nix:
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)