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
neovim: expose the neovim-unwrapped manpage #87929
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wrapper gets more and more complicated, I wonder why doesn't it use symlinkJoin
as it would just grab everything there is in neovim-unwrapped
. But that's out of scope for this PR. Could you move your addition a bit upwards? Before optionalString (!stdenv.isDarwin)
so Darwin users will enjoy this enhancement it as well?
I had thought about that as well, I don't think I'll submit another PR to add that, but since that PR will just update what gets pushed in this one, should I just close this one and submit the other to avoid a useless commit? |
Sometimes, in my own overlays, I do something like this: octave-wrapped = super.symlinkJoin rec {
name = "octave-wrapped";
paths = [ super.octave ];
pythonEnv = super.python3.withPackages(ps: [
ps.sympy
]);
nativeBuildInputs = [ super.makeWrapper ];
# along with some fixes to the desktop file
postBuild = ''
cp -L $out/bin/octave $out/bin/octave.tmp
mv -f $out/bin/octave.tmp $out/bin/octave
wrapProgram $out/bin/octave --prefix PATH : ${pythonEnv}/bin
cp -L $out/share/applications/org.octave.Octave.desktop $out/share/applications/org.octave.Octave.desktop.tmp
mv -f $out/share/applications/org.octave.Octave.desktop.tmp $out/share/applications/org.octave.Octave.desktop
substituteInPlace $out/share/applications/org.octave.Octave.desktop \
--replace "Terminal=false" "Terminal=true" \
--replace "octave --gui" octave
'';
}; Which may seem a bit hacky but I think it's not that bad. If I were the original writer of the neovim wrapper function I'd do this. I think it'll be best for you to wait a few days and see what the neovim maintainers think about this. |
Right, that is much cleaner. I'll just wait and see what happens. |
#87929 (comment) sounds good, no need to open another PR. Thanks for doing this, it has been on my TODO for a while and was surprised no one complained about it (I sure did xD) |
That branch looks good. Maybe this is not related to the issue at hand, but if someone wants to wrap Neovim with a |
Yeah it'll be wrapped twice, but the arguments passed to the first wrap will be propagated up to the final binary. I don't really see a clean way to only wrap once without another |
I disagree that there's no clean way to do it. The problem is that the current way the wrapper function is implemented is of low quality (if you ask me). I once wrote a generic wrapper Nix function that "compiles" a string which in turn becomes a list of arguments to |
Fair point, however that would be way over my head in terms of how skilled I am in Nix, I wouldn't even know where to start in making something like that. |
Cleanups: - Removed unneeded neovim.meta.description reset. - Remove unnecessary -x checks in `postBuild`. - Use a ${placeholder "out"} if needed. Changes: - Switch to symlinkJoin, so e.g manpages link to the environment (NixOS#87929). - Use nvim-node from $out/bin/ just like all other providers. - Compute all arguments to makeWrapper in pure Nix "before" `postBuild`. - Prevent double wrapping in case `configure != {}` and rplugin.vim needs to be generated. Co-authored-by: Silvan Mosberger <contact@infinisil.com>
With #88110, the man pages and everything else is now in the wrapped neovim |
Cleanups: - Removed unneeded neovim.meta.description reset. - Remove unnecessary -x checks in `postBuild`. - Use a ${placeholder "out"} if needed. Changes: - Switch to symlinkJoin, so e.g manpages link to the environment (NixOS#87929). - Use nvim-node from $out/bin/ just like all other providers. - Compute all arguments to makeWrapper in pure Nix "before" `postBuild`. - Prevent double wrapping in case `configure != {}` and rplugin.vim needs to be generated. Co-authored-by: Silvan Mosberger <contact@infinisil.com>
Motivation for this change
Install the neovim manpage along with the neovim package. Before, the manpage was only installed with neovim-unwrapped.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after) (Difference of +624
)