Skip to content
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

vim module: init #19438

Merged
merged 1 commit into from
Oct 12, 2016
Merged

vim module: init #19438

merged 1 commit into from
Oct 12, 2016

Conversation

primeos
Copy link
Member

@primeos primeos commented Oct 10, 2016

Motivation for this change

My main motivation is to provide the services.vim.defaultEditor wich a user could set in order to make vim (instead of nano) the system wide default text editor via the EDITOR environment variable (and it'll install vim if that isn't already the case). I don't know if the services.vim.install option makes sense since the same effect can be achieved by simply adding vim to the systemPackages.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Note: If both services.vim.defaultEditor and services.emacs.defaultEditor are set vim will be the default editor (not tested...) since the priority is lower (mkForce / mkOverride 10 vs mkOverride 900). Is that ok or should I also use 900? I just thought mkForce (which apparently uses 10) would fit better. I must also admit that this probably isn't the best solution due to the emacs collision, however the user should avoid that anyway. An alternative would be to introduce a generic defaultEditor option.

@mention-bot
Copy link

@primeos, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @bjornfor and @offlinehacker to be potential reviewers.

@joachifm
Copy link
Contributor

This makes more sense if moved to the programs namespace, as it doesn't really provide a service (whereas the emacs module does).

The install option is redundant, imo.

@primeos
Copy link
Member Author

primeos commented Oct 10, 2016

@joachifm I moved it to the programs namespace and removed the install option - thanks 😄

I additionally changed mkForce to mkOverride 900 so that it produces an error message if both services.emacs.defaultEditor and programs.vim.defaultEditor are set:

error: The option `environment.variables.EDITOR' is defined multiple times, in `/root/nixpkgs/nixos/modules/services/editors/emacs.nix' and `/root/nixpkgs/nixos/modules/programs/vim.nix'.

@primeos primeos changed the title vim service: init vim module: init Oct 10, 2016
@ericsagnes
Copy link
Contributor

It would be nice if some of the vim_configurable functionality could be integrated in this module. (Or at least to be able to select which vim package to use)

@jagajaga jagajaga self-assigned this Oct 12, 2016
@jagajaga jagajaga merged commit ce3624f into NixOS:master Oct 12, 2016
@primeos
Copy link
Member Author

primeos commented Oct 13, 2016

@ericsagnes yeah sounds like a good idea.

I'm currently trying to add an option to select a vim-package and provide a global vimrc over at #19502 - hope that's roughly what you've expected 😄

Would be interested if that's going in the right direction or not if you have the time 😉

I'll also have to take a closer look at vim_configurable since I'm still pretty new to Nix(OS) development...

@LnL7 LnL7 mentioned this pull request Mar 4, 2017
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants