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

remove make_installer and all auto-install logic #334

Closed
justinmk opened this issue Aug 30, 2020 · 16 comments · Fixed by #498
Closed

remove make_installer and all auto-install logic #334

justinmk opened this issue Aug 30, 2020 · 16 comments · Fixed by #498

Comments

@justinmk
Copy link
Member

justinmk commented Aug 30, 2020

We need to get out of the business of auto-installing things. It's too much scope.

in the configs where we have make_installer , 70-80% of the code ends up dealing with install logic. That isn't really the point of this repo , it's a distraction.

Note:

  • Documention of common install steps is valuable and appropriate for this repo. E.g.:
    • URL to download the server installer
    • URL to upstream documentation explaining how to install the server
    • brief instructions that user can copy/paste into their shell to install via common package managers such as apt-get/homebrew
  • It could be valuable for :checkhealth to be the entrypoint that users can reach for, to detect common problems with LSP server installation. For that purpose some of the existing installer logic might be useful.
@justinmk justinmk pinned this issue Aug 30, 2020
@tjdevries
Copy link

I've discussed this a few times in gitter, but I think a better strategy may be to make an org "neovim-lsp" or "neovim-lsp-configurations" and split the different servers into different repos. We can give people access to be maintainers of those repos. Each repo can decide if auto install is possible or not for that server.

I do think the ability to auto install is very helpful, particularly to new comers. So it would be a shame to lose it completely or have people make many many worse versions rather than coming together to make one good one.

@justinmk
Copy link
Member Author

split the different servers into different repos

Many repos adds a lot cognitive and mechanical overhead. The entire purpose of this repo was to give users a convenient way to install "all the configs" (which in terms of data is small).

We can give people access to be maintainers of those repos

What's wrong with PRs to one repo? Many repos means we need at least one maintainer for each repo, and if they go missing then Neovim org has to manage all of those repos...

Each repo can decide if auto install is possible or not for that server.

Contributors can create separate repos already. Putting them under the Neovim org implies some level of commitment, so it doesn't avoid the motivation for removing the install logic in this project.

The "microservice" approach is a way of side-stepping responsibility which is useful for giant organizations, but in OSS that approach is implicit and ever-present.

I do think the ability to auto install is very helpful, particularly to new comers. So it would be a shame to lose it completely or have people make many many worse versions rather than coming together to make one good one.

The doc element can/should specify useful install steps (links to upstream docs, basic CLI instructions), and solves 95% of the problem without involving the very large scope of trying to automate the steps for the myriad environments users operate in (aka "ad-hoc package manager").

@colinxs
Copy link

colinxs commented Aug 31, 2020

First off, thanks for the awesome work in putting together this repo! It makes using Neovim's LSP support a (mostly) painless experience. As someone considering creating an language plugin for Neovim, I think relaxing the one-repo-per-server approach to allow for multiple LSP's per repo makes the most sense. This means you could have:

  • A barebones plugin that only provides a single LSP (e.g. something like nvim-lsp-pylance)
  • A comprehensive language plugin that provides an LSP server in addition to the traditional ftplugin/syntax/etc. features (e.g. the LSP server config for C/C++ could live in the vim-cpp repo). For languages with multiple LSPs, like Python, the repo could contain configuration for each server.

I think this has a few advantages:

  • I think the LSP configuration is more tightly coupled with the related language than Neovim itself. The code/logic for starting the language server may be non-trivial and easier done in whatever language the LSP is for, rather than Lua (e.g. LSPNeovim.jl linked above or the Julia VSCode plugin). The mono-repo approach prevents the developers of the LSP from fully owning the code for Neovim configuration, setting up CI pipelines, and quickly pushing bugfixes without waiting for a PR to be approved here.
  • Languages may have several LSPs available (e.g. Python, which has pylance/pyright/pyls etc.) and code for configuring each LSP can (optionally) be shared.
  • Similarly, languages may support multiple Editors/IDE and keeping everything under "one roof" has its advantages (as is done with the JuliaEditorSupport org).

That being said, I think having a common repo/plugin for additional utilities (e.g. nvim_lsp.root_pattern) and features (e.g. the :checkhealth, seeing the LSP log) is useful. In the future, this code could be moved into Neovim itself as part of the standard LSP plugin interface.

Perhaps one day Neovim will have a full blown plugin manager with real support for dependencies which greatly simplify LSP/plugin installation. That's one of the really nice things about something like VSCode and it's use of Node for packaging: installing extensions is a breeze.

@justinmk
Copy link
Member Author

justinmk commented Sep 1, 2020

The mono-repo approach prevents the developers of the LSP from fully owning the code for Neovim configuration, setting up CI pipelines, and quickly pushing bugfixes without waiting for a PR to be approved here.

That approach is already/always available for anyone to do outside of the Neovim org umbrella. Meanwhile:

  • If anyone has ideas for CI pipelines, pushing bugfixes, etc.... waiting for a PR to be reviewed here is totally reasonable.
    • If PRs take too long to review here, they'll take even longer if we spread out maintenance across N new repos.
  • We do not hesitate to add new maintainers when someone shows interest and is a regular, reliable contributor. So, question of separate repos is orthogonal to empowering contributors.

Again, 100 repos instead of 1 repo solves very, very little, and adds a high cost in terms user signaling, maintenance, documentation, cognitive burden, and more.

@adelarsq
Copy link
Contributor

adelarsq commented Sep 3, 2020

The doc element can/should specify useful install steps (links to upstream docs, basic CLI instructions), and solves 95% of the problem without involving the very large scope of trying to automate the steps for the myriad environments users operate in (aka "ad-hoc package manager").

Really like this approach.

Again, 100 repos instead of 1 repo solves very, very little, and adds a high cost in terms user signaling, maintenance, documentation, cognitive burden, and more.

I agree.

I think that enabling to point to a LSP installed elsewhere is a valid approach. I'm already using like this with F# and isn't so painful as appears at first.

@teto
Copy link
Member

teto commented Sep 20, 2020

I agree wholeheartdly with @justinmk on all points (I should probably have +1 the first message but this echoed so strongly with my opinion that I had to write it down xD).

@HiPhish
Copy link

HiPhish commented Sep 30, 2020

I have a question: what about configurations which already have an installer in them? I have been using the JDT language server (Java) and wanted to contribute some improvements from my own config back to this project. The code could be cleaned up a bit by removing the installer and letting the user specify where the server is located. Is it OK to just rip out all the installer code?

@pretentious7
Copy link
Contributor

I have a question: what about configurations which already have an installer in them? I have been using the JDT language server (Java) and wanted to contribute some improvements from my own config back to this project. The code could be cleaned up a bit by removing the installer and letting the user specify where the server is located. Is it OK to just rip out all the installer code?

Same question here with typescript-language-server

@teto
Copy link
Member

teto commented Dec 29, 2020

we should erase all of them to be consistent. If a madman wants to maintain an nvim-lspinstall he can but that wont be us :)

@mjlbach
Copy link
Contributor

mjlbach commented Dec 29, 2020

we should erase all of them to be consistent.

I'm working on a PR for this. One thing, is I would like to make clear to people when it is necessary to manually specify the cmd portion of default config. A lot of commands are expected to be on (or wrapped) on the path of neovim (pyright, rust-analyzer for example). Several (elixir) require some assumptions/pointing to the build directory.

@adelarsq
Copy link
Contributor

we should erase all of them to be consistent. If a madman wants to maintain an nvim-lspinstall he can but that wont be us :)

I'm working on this 😬

@mjlbach
Copy link
Contributor

mjlbach commented Dec 29, 2020

@adelarsq If you're going to be working on this, you can check #498 to see what was removed. I think the most you'll have to do is override the default_config.cmd field to point to whatever your plugin installs.

@mjlbach mjlbach unpinned this issue Jan 3, 2021
@adelarsq
Copy link
Contributor

adelarsq commented Jan 3, 2021

@mjlbach Thanks. I will be working on that on next weeks. Just will be a little slow since I'm short of time to work on OSS projects.

@maisiliym
Copy link

The incorrectness of including a build system in an editor extension was pointed out several months ago, and the one who pointed it out was, in a manner of speaking, shooed out of the room. Apparently, the issue was somehow deleted, hence this comment, as a reminder. Perhaps this will help the concerned person's credibility in the future, which would save everyone involved a lot of time and energy.

@mjlbach
Copy link
Contributor

mjlbach commented Jan 6, 2021

This is just my unsolicited opinion.

I think I remember the issue you are referring to, and the aggrieved commenter was being toxic, cursing, and insulting contributors. If you look at the discussion on this feature request, and the fact that the feature was implemented, I think you'll see what the effect of how you say things, and treat others, can have on promoting a productive discussion, which ultimately leads to the realization of what you are asking for.

This is all volunteer work, I personally spent several hours working on the PR that closed this, so it's really demoralizing when someone comes in and diminishes the achievements of what others have done out of kindness for the larger open source software community.

@maisiliym
Copy link

maisiliym commented Jan 6, 2021

Li does not address people, therefore it is logically impossible for him to be insulting.

note: Li will henceforth cease to publish any unsolicited insight, as the cost-return has proven to be abyssymal. Hob is the planned replacement platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants