vimUtils: customize accepts alternate {g}vimExecutable value #87639

mitchellh · 2020-05-12T00:57:37Z

Motivation for this change

I use Nix on darwin and use MacVim. MacVim uses "mvim" as the shell executable to launch the GUI version. I want to wrap pkgs.macvim with vimUtil.makeCustomizable so I can customize the vimrc but this doesn't work because of the hardcoded "vim" and "gvim" executables.

I have to use makeCustomizable rather than call vimWithRC directly because programs.vim in home-manager uses pkgs.vim_configurable to set all the Vim settings. So I have to make an overlay that sets pkgs.vim_configurable to the customizable version of macvim. But this is a blocker! See the vim module in home-manager here. Also, calling vimWithRC directly in general feels kinda weird, like I'm digging too far into the abstraction.

This introduces an argument to set the base name for these commands. I'm unsure what to name this so open to other requests.

This is my first PR so please let me know what else needs to be done. I followed the CONTRIBUTING.md guidelines as close as possible!

Things done

This enables vim implementations that don't strictly use the "vim" and "gvim" pattern for CLI and graphical respectively to still be wrapped using customize so that a vimrc can be generated. The primary example is MacVim on Darwin, which uses "vim" and "mvim".

mitchellh · 2020-05-12T01:03:49Z

An alternate idea I had but more invasive was to change customize or vimWithRC to accept a list of binary names to write a script for, i.e. vimExecutables = [ "vim" "gvim" ] or something so we could accommodate additional binaries. But, it felt too invasive and difficult to maintain backwards compatibility.

jonringer

Workflow looks to be transparent to users that are already using this.

jonringer · 2020-05-12T20:31:50Z

Also, thanks for opening your first PR with nixpkgs @mitchellh :)

jonringer · 2020-05-12T20:43:08Z

I use Nix on darwin and use MacVim. MacVim uses "mvim" as the shell executable to launch the GUI version. I want to wrap pkgs.macvim with vimUtil.makeCustomizable so I can customize the vimrc but this doesn't work because of the hardcoded "vim" and "gvim" executables.

If an installation of gvim and macvim are mutually exclusive, then it may also make sense for the macvim package to also export a gvim alias/symbolic link to mvim. I don't really use mac, so I'm not 100% sure.

This could be similar to viAlias that exists for neovim which create symlinks in the underlying neovim package https://github.com/rycee/home-manager/blob/e9945ee6eefee0e261fe4d5b3a7c5de4f0f8d91b/modules/programs/neovim.nix#L42

mitchellh · 2020-05-12T20:46:00Z

I use Nix on darwin and use MacVim. MacVim uses "mvim" as the shell executable to launch the GUI version. I want to wrap pkgs.macvim with vimUtil.makeCustomizable so I can customize the vimrc but this doesn't work because of the hardcoded "vim" and "gvim" executables.

If an installation of gvim and macvim are mutually exclusive, then it may also make sense for the macvim package to also export a gvim alias/symbolic link to mvim. I don't really use mac, so I'm not 100% sure.

This could be similar to viAlias that exists for neovim which create symlinks in the underlying neovim package https://github.com/rycee/home-manager/blob/e9945ee6eefee0e261fe4d5b3a7c5de4f0f8d91b/modules/programs/neovim.nix#L42

Ah! So MacVim does BOTH so it supports gvim already. But at least in circles I’m familiar with it seems more idiomatic on Mac to use “mvim”. So this is just trying to get nix to play nice with the idiom.

Hence that other comment of proposing maybe a new config to support multiple but that’d be a deeper change.

I’m open to any of this

timokau · 2020-05-12T21:27:00Z

I see two alternative options:

The most expressiveway to model this would be a mapping from input name to output name (deprecating vimExecutableName and gvimExecutableName). This would default to

wrapperMapping = {
    "vim" = "my-custom-vim-executable-name";
    "gvim" = "gvim";
}

The simplest way to do it would be to just always wrap mvim if it exists. We could also add an additional mvimExecutableName or just let the user use gvimExecutableName for that purpose. I'd argue the renaming functionality should not have been added in the first place anyway.

Those are the two ends of the complexity spectrum, with your solution living somewhere in the middle. I think I'd favor the simplest option, configuring vim is already complicated enough.

mitchellh · 2020-05-12T21:38:42Z

Those are the two ends of the complexity spectrum, with your solution living somewhere in the middle. I think I'd favor the simplest option, configuring vim is already complicated enough.

What do you view as the “simplest”? I’m fine with whatever you think should be done and I’m happy to make those changes.

jonringer

assuming that name is usually vim and gvimExecutableName is usually gvim, I think this was the original intended behavior.

jonringer · 2020-05-12T22:19:15Z

pkgs/misc/vim-plugins/vim-utils.nix

+      vimExecutable ? "vim",
+      gvimExecutable ? "gvim",


Suggested change

vimExecutable ? "vim",

gvimExecutable ? "gvim",

jonringer · 2020-05-12T22:19:41Z

pkgs/misc/vim-plugins/vim-utils.nix

+      vimExecutable = "${vim}/bin/${vimExecutable}";
+      gvimExecutable = "${vim}/bin/${gvimExecutable}";


Suggested change

vimExecutable = "${vim}/bin/${vimExecutable}";

gvimExecutable = "${vim}/bin/${gvimExecutable}";

vimExecutable = "${vim}/bin/${vimExecutableName}";

gvimExecutable = "${vim}/bin/${gvimExecutableName}";

mitchellh · 2020-05-12T22:35:24Z

I'm good with those suggestions of reusing the existing arguments in a smarter way. I think that has a bigger risk of breaking userspace if anyone out there is doing something wacky like vimExecutableName = "something-else" (which would've worked prior).

timokau · 2020-05-13T10:53:01Z

Those are the two ends of the complexity spectrum, with your solution living somewhere in the middle. I think I'd favor the simplest option, configuring vim is already complicated enough.

What do you view as the “simplest”? I’m fine with whatever you think should be done and I’m happy to make those changes.

The second option, having it "just work". That means withVimRC just checks if there's a mvim executable and if there is one just wraps that one as well. No additional configuration necessary.

I'd wait until at least one other person agrees with this before implementing it though. The current implementation is also fine, but I'm very hesitant to add even more complexity to the vim configuration from a user perspective.

I'm good with those suggestions of reusing the existing arguments in a smarter way. I think that has a bigger risk of breaking userspace if anyone out there is doing something wacky like vimExecutableName = "something-else" (which would've worked prior).

Yes, support renaming the executable is exactly the thing which I think should not have been added in the first place. Now it would need proper deprecation to remove though, which is out of scope for this PR.

mitchellh · 2020-05-14T00:00:06Z

Agreed @timokau.

@jonringer not sure what your thumbs up on my last comment is supposed to imply 😄

jonringer · 2020-05-14T00:31:45Z

I'm largely of the opinion that the macvim package should just also export a gvim link, similar to llvm exporting a "false" gcc installation.

  postInstall = ''
    ln -s $out/bin/mvim $out/bin/gvim
  '';

jonringer · 2020-05-14T00:33:13Z

@jonringer not sure what your thumbs up on my last comment is supposed to imply smile

Just agreeing that it has the potential to break users who have set it to something else.

mitchellh · 2020-05-14T00:33:35Z

I'm largely of the opinion that the macvim package should just also export a gvim link, similar to llvm exporting a "false" gcc installation.
  postInstall = ''
    ln -s $out/bin/mvim $out/bin/gvim
  '';

It already does this, and this works no problem. Its just more idiomatic to use mvim with it. I think I mentioned this a few comments up. So, if the stance is we don't want to support mvim, that's fine its just a somewhat weird experience and have to rewire a decade or so of muscle memory. 😄

jonringer · 2020-05-14T00:46:24Z

It already does this, and this works no problem. Its just more idiomatic to use mvim with it. I think I mentioned this a few comments up. So, if the stance is we don't want to support mvim, that's fine its just a somewhat weird experience and have to rewire a decade or so of muscle memory. smile

Oh, right, the mvim isn't being modified by home-manager, so it's just a vanilla experience.

"In computer science, there's three values which make sense: 0, 1, many"
Maybe it would be best to just have an array with executablesToWrap ? [ "vim" "gvim" ]. This would also future proof against other editors which make have different name, but support vim plugins/configuration.

timokau · 2020-05-14T11:52:13Z

Maybe it would be best to just have an array with executablesToWrap ? [ "vim" "gvim" ]. This would also future proof against other editors which make have different name, but support vim plugins/configuration.

If you then make that silently ignore the case where the executable is not present and default to [ "vim" "gvim" "nvim" "mvim" ] that would also cover my suggestion and work without configuration most of the times.

If we wanted to get fancy, we could also add a vimExecutables passthru to the different vims, but that is probably overkill.

teto · 2020-10-31T20:17:44Z

Might be of interest #101265 . You could expose lower-level APIs so that the caller can customize the wrapper arguments itself.

jonringer · 2020-11-01T23:01:42Z

totally forgot about dealing with the 20.09 release.

SuperSandro2000 · 2021-01-18T17:49:54Z

@mitchellh @jonringer ping

stale · 2021-07-21T02:11:53Z

I marked this as stale due to inactivity. → More info

mitchellh requested a review from jonringer as a code owner May 12, 2020 00:57

ofborg bot added 6.topic: vim 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels May 12, 2020

jonringer approved these changes May 12, 2020

View reviewed changes

jonringer requested review from timokau and evanjs May 12, 2020 20:31

timokau requested a review from teto May 12, 2020 21:27

jonringer reviewed May 12, 2020

View reviewed changes

stale bot added the 2.status: stale label Jul 21, 2021

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

stale bot removed the 2.status: stale label Jun 12, 2023

mitchellh closed this by deleting the head repository Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vimUtils: customize accepts alternate {g}vimExecutable value #87639

vimUtils: customize accepts alternate {g}vimExecutable value #87639

mitchellh commented May 12, 2020 •

edited

Loading

mitchellh commented May 12, 2020

jonringer left a comment •

edited

Loading

jonringer commented May 12, 2020

jonringer commented May 12, 2020

mitchellh commented May 12, 2020

timokau commented May 12, 2020

mitchellh commented May 12, 2020

jonringer left a comment

jonringer May 12, 2020

jonringer May 12, 2020

mitchellh commented May 12, 2020

timokau commented May 13, 2020

mitchellh commented May 14, 2020

jonringer commented May 14, 2020

jonringer commented May 14, 2020

mitchellh commented May 14, 2020

jonringer commented May 14, 2020

timokau commented May 14, 2020

teto commented Oct 31, 2020

jonringer commented Nov 1, 2020

SuperSandro2000 commented Jan 18, 2021

stale bot commented Jul 21, 2021

		vimExecutable = "${vim}/bin/${vimExecutable}";
		gvimExecutable = "${vim}/bin/${gvimExecutable}";

vimUtils: customize accepts alternate {g}vimExecutable value #87639

vimUtils: customize accepts alternate {g}vimExecutable value #87639

Conversation

mitchellh commented May 12, 2020 • edited Loading

Motivation for this change

Things done

mitchellh commented May 12, 2020

jonringer left a comment • edited Loading

Choose a reason for hiding this comment

jonringer commented May 12, 2020

jonringer commented May 12, 2020

mitchellh commented May 12, 2020

timokau commented May 12, 2020

mitchellh commented May 12, 2020

jonringer left a comment

Choose a reason for hiding this comment

jonringer May 12, 2020

Choose a reason for hiding this comment

jonringer May 12, 2020

Choose a reason for hiding this comment

mitchellh commented May 12, 2020

timokau commented May 13, 2020

mitchellh commented May 14, 2020

jonringer commented May 14, 2020

jonringer commented May 14, 2020

mitchellh commented May 14, 2020

jonringer commented May 14, 2020

timokau commented May 14, 2020

teto commented Oct 31, 2020

jonringer commented Nov 1, 2020

SuperSandro2000 commented Jan 18, 2021

stale bot commented Jul 21, 2021

mitchellh commented May 12, 2020 •

edited

Loading

jonringer left a comment •

edited

Loading