Skip to content

neovim: add extraLuaPackages to neovim, fixes #1964.#1970

Closed
ashinkarov wants to merge 4 commits intonix-community:masterfrom
ashinkarov:tema/master
Closed

neovim: add extraLuaPackages to neovim, fixes #1964.#1970
ashinkarov wants to merge 4 commits intonix-community:masterfrom
ashinkarov:tema/master

Conversation

@ashinkarov
Copy link
Copy Markdown
Contributor

@ashinkarov ashinkarov commented Apr 29, 2021

Description

Extend neovim program to allow specifying additional lua libraries that will become available to neovim at runtime. See #1964 for further details.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all. The tests die on my machine, but aparently for the reasons that has nothing to do with my changes. I hope that github's CI would pick it up so we can discuss what's going on.

  • Test cases updated/added. See example. I am not sure whether this change needs any tests. Does it?

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

Similarly to extraPytonPackages, we add extraLuaPackages that make
lua libraries available at runtime.
@ashinkarov ashinkarov requested a review from rycee as a code owner April 29, 2021 14:48
Copy link
Copy Markdown
Contributor

@sumnerevans sumnerevans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple questions. I think this looks pretty good, though.

Comment thread modules/programs/neovim.nix
Comment thread modules/programs/neovim.nix
@sumnerevans
Copy link
Copy Markdown
Contributor

sumnerevans commented Apr 29, 2021

To address your comment in the original comment: I am pretty sure this is backwards compatible. No existing configs will break with this change.

@sumnerevans sumnerevans requested a review from teto April 30, 2021 14:38
@sumnerevans
Copy link
Copy Markdown
Contributor

@ashinkarov please run ./format from the root of the repo to format the code.

@teto
Copy link
Copy Markdown
Collaborator

teto commented May 1, 2021

we can keep the interface but the logic should be moved to nixpkgs in the long term. Also worth a test.

@ashinkarov
Copy link
Copy Markdown
Contributor Author

@ashinkarov please run ./format from the root of the repo to format the code.

Done.

@ashinkarov
Copy link
Copy Markdown
Contributor Author

we can keep the interface but the logic should be moved to nixpkgs in the long term. Also worth a test.

@teto , I have tried extending neovim tests in the last commit. However, I am having trouble when executing the instance of neovim. I'd like to run something like nvim -c "lua require('lua-utf8')" -c "q", but the following attempt doesn't work:

${programs.neovim}/bin/nvim -c "lua require('lua-utf8')" -c "q"

(seems like it is stuck in an infinite loop). Sorry for stupid questions, but I have zero familiarity with the framework.

@stale
Copy link
Copy Markdown

stale bot commented Oct 27, 2021

Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously, when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the issue

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

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.

3 participants