NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.81k stars 13.91k forks source link

less error on default configuration if missing vim on path #319128

Open samhpickering opened 4 months ago

samhpickering commented 4 months ago

Describe the bug

The default NixOS configuration uses lesspipe as a preprocessor for less. lesspipe looks for various tools (bat, batcat, pygmentize, source-highlight) on the path and falls back to the included vimcolor and code2color scripts if none are found. vimcolor assumes vim is available on the PATH and outputs an error message if it is not found. This means using less on a default NixOS install without vim produces an error message (but otherwise fails gracefully).

Steps To Reproduce

Steps to reproduce the behavior:

  1. Setup a default NixOS install
  2. Do not install bat, batcat, pygmentize, source-highlight, or vim
  3. less /etc/nixos/configuration.nix

Expected behavior

Either lesspipe includes vim as a dependency, vimcolor is not enabled by default, or lesspipe is not configured by default.

Additional context

Before 2e2671126e2e1af4621e31c159f1a32b0ef8a903 (#254409) lesspipe was wrapped. lesspipe attempts to add its own directory to the path here. When wrapped, this failed as the name of the wrapped script did not match, so vimcolor was never found on the PATH. (also code2color, the other included colorizer) After 2e2671126e2e1af4621e31c159f1a32b0ef8a903 lesspipe is no longer wrapped, and successfully adds its store directory to PATH. vimcolor is found on the PATH, and is used as a colorizer if bat, batcat, pygmentize, or source-highlight are not found. vimcolor errors if vim is not found on the PATH.

Notify maintainers

@martijnvermaat is listed as maintainer but appears to be inactive


Add a :+1: reaction to issues you find important.

vancluever commented 3 months ago

I think this is also causing issues when your installed "vim" is neovim (i.e., you use programs.neovim.vimAlias = true in home-manager), as vimcolor relies on options that are not available in neovim. Viewing certain files will give:

vim returned an error code of '1'
vim wrote this error output:
vim: Unknown option argument: "-RXZ"
More info with "vim -h"
shaver commented 1 month ago

I'm willing to take a crack at it, and it seems like the Right Nix Thing here is to actually pull in all those externals to make less be hermetic. On the other hand, those were probably made fake externals for a reason (reducing the dependency graph). In the default NixOS configuration vim is installed, I believe, so making the dependency graph bigger wouldn't likely affect most people, just us weirdos who use nvim as vim, or omit vim entirely.

@edolstra @dtzWill are listed in nixpkgs, do you have a preference?

vancluever commented 1 month ago

@shaver I haven't looked deep to see if it helps make this any easier to do, but nvimpager was added as a colorizer to lesspipe fairly recently: https://github.com/wofr06/lesspipe/issues/152#issuecomment-2212551264

Just in case you hadn't come across that yet. :slightly_smiling_face:

shaver commented 1 month ago

That would make us nvimmers work better, but would still leave the whole thing wildly impure. It would solve my problem, but I'm not sure it's the right way to leave the less package set up.

But then, maybe "pull in its own copy of vim that has to be configured with the same plugins and such so that it looks right for the user" isn't the right way either!

Is this impurity any worse than running $EDITOR for a git commit? Maybe we give the less package a way to configure which editor to use for colorization, or maybe we just check $EDITOR and call the right one/none for a few known editors? Whoever wants to colorize through nano or emacs can make it more general. 😀

(Or: programs.less.colorizeWithEditor = "auto/vim/nvim/none"?)

I'm happy to do the work (once I figure out how...), I just want to make sure I'm not leaving it even worse...

vancluever commented 1 month ago

@shaver just FYI that I've put in a PR to update lesspipe as well (as per the recently mentioned PR). Looks like it hasn't been updated since 2.11 in unstable.

From here all I needed to do was add in the nvimpager package and set my LESSOPEN manually as I'm not using overlays; honestly if you use overlays, it might "just work" after adding the overlay and nvimpager.

Just based on this experience I'd almost say it'd be optimal to, yeah, just leave the packages and focus on the module where purity is much less of a concern - pulling in nvimpager or something else based on a module config setting seems like a much better experience.

Hope that helps!

shaver commented 1 month ago

How do you set LESSOPEN such that lesspipe uses nvimpager? I think we still want lesspipe's other behaviours, so I'm not sure how to wedge in nvimpager from a module. Do you mean that you set LESSCOLORIZER to nvimpager?

vancluever commented 1 month ago

@shaver so what I've done, is:

In my system config:

programs.less.lessopen = null;

And then in my home-manager config:

home = {
  packages = [ pkgs.nvimpager ];
  sessionVariables = {
    LESSOPEN = "|${lesspipe}/bin/lesspipe.sh %s";
  };
};

Note that the lesspipe referenced here is aliased to the lesspipe package in my PR.

As mentioned, the changes to lesspipe upstream pull in nvimpager along with the other preferred colorizers, see https://github.com/wofr06/lesspipe/commit/511bc528ee5ab786aef62629e8509e67e88d5152. So no extra config is necessary.