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

Suggestions about the YouCompleteMe plugin #2

Open
micbou opened this issue Mar 17, 2019 · 9 comments
Open

Suggestions about the YouCompleteMe plugin #2

micbou opened this issue Mar 17, 2019 · 9 comments

Comments

@micbou
Copy link

micbou commented Mar 17, 2019

I'd like to make two suggestions to the docs about YCM.

First, there is a mention that g:ycm_clangd_binary_path should be set to clangd because the bundled one won't be updated. That's not true. As one of the maintainers of YCM, I assure you that it will be updated as soon as a new version of LLVM is released so I would suggest to remove the comment "which doesn't get updates". By the way, the exepath call is not needed when setting that option since YCM already searches the executable in the PATH.

Second, the "and may be easier to install than YouCompleteMe" statement in the LanguageClient-neovim section should be removed. Docs should stay as neutral as possible especially when talking about editors and plugins.

I can prepare a PR if you agree with these suggestions.

@micbou
Copy link
Author

micbou commented Mar 17, 2019

By the way, the exepath call is not needed when setting that option since YCM already searches the executable in the PATH.

Forget about this. I thought we were but we don't. That's something we could easily implement though.

@sam-mccall
Copy link
Member

sam-mccall commented Mar 17, 2019

@gribozavr as I'm out on leave.

First, there is a mention that g:ycm_clangd_binary_path should be set to clangd because the bundled one won't be updated. That's not true.

This was meant to imply it won't be updated as part of the usual distro-based update process - as I understand YCM has no auto-update process that will cause newer versions of clangd to be installed. If this is correct, we can clarify the wording here.

By the way, the exepath call is not needed when setting that option since YCM already searches the executable in the PATH.
Forget about this. I thought we were but we don't. That's something we could easily implement though.
This would be nice to have I think - IIRC @kadircet wanted to add this in the initial PR but was asked to take it out. I think the current state is: this is a policy question, and clangd/YCM people have different recommendations for how clangd should be distributed for use with YCM.

Second, the "and may be easier to install than YouCompleteMe" statement in the LanguageClient-neovim section should be removed. Docs should stay as neutral as possible especially when talking about editors and plugins.

I'm not sure what neutral here would look like - there's lots of vim LSP clients, and listing them all without suggestion or descriptions doesn't seem to do users any favours. YCM works the best of the clients we tried, but is the hardest to install - seems useful to mention?

I can prepare a PR if you agree with these suggestions.

Sadly, the github copy of the docs are not the source of truth here - we haven't yet worked out a plan to sync them.

@micbou
Copy link
Author

micbou commented Mar 17, 2019

This was meant to imply it won't be updated as part of the usual distro-based update process - as I understand YCM has no auto-update process that will cause newer versions of clangd to be installed. If this is correct, we can clarify the wording here.

So the comment is only targeting rolling Linux distributions? I think that's a mistake considering that YCM also officially supports other kind of Linux distributions, macOS, and Windows. It would be less biased towards a category of users to just say "Set this option to use a custom Clangd instead of the YCM bundled one".

This would be nice to have I think - IIRC @kadircet wanted to add this in the initial PR but was asked to take it out. I think the current state is: this is a policy question, and clangd/YCM people have different recommendations for how clangd should be distributed for use with YCM.

We asked to change it because it was the default behavior. The suggestion here is to search the value of g:ycm_clang_binary_path in the PATH when it's not empty (which is not the default).

I'm not sure what neutral here would look like - there's lots of vim LSP clients, and listing them all without suggestion or descriptions doesn't seem to do users any favours. YCM works the best of the clients we tried, but is the hardest to install - seems useful to mention?

Fair enough. I guess this gives us an incentive to ease our installation process.

@kadircet
Copy link
Member

So the comment is only targeting rolling Linux distributions? I think that's a mistake considering that YCM also officially supports other kind of Linux distributions, macOS, and Windows. It would be less biased towards a category of users to just say "Set this option to use a custom Clangd instead of the YCM bundled one".

I believe you misunderstood Sam's point. We know you update your bundled third_party binaries as soon as newer versions with none or small regressions are available. The problem is even if you update that binary inside ycmd repo, it won't be updated on user's machine, unless user goes into installed plugin directory and re-performs install step. Most of the users simply won't care/know about this step.

Which is not the case with distro-based updates. If a user has clang-tools installed, everytime he/she updates her system it will be updated along with other packages implicitly.

We asked to change it because it was the default behavior. The suggestion here is to search the value of g:ycm_clang_binary_path in the PATH when it's not empty (which is not the default).

Definitely that sounds good to me.

@micbou
Copy link
Author

micbou commented Mar 18, 2019

We know you update your bundled third_party binaries as soon as newer versions with none or small regressions are available. The problem is even if you update that binary inside ycmd repo, it won't be updated on user's machine, unless user goes into installed plugin directory and re-performs install step. Most of the users simply won't care/know about this step.

Why not explaining that then? If users want to update Clangd, they should run again the install.py script. That's necessarily something they are familiar with since they have to do it at least once to install the plugin. I think it's better to say this than giving the impression that YCM will never update Clangd and assuming that all Vim users have Clangd in their path.

Definitely that sounds good to me.

I'll send a PR for that.

@kadircet
Copy link
Member

Why not explaining that then? If users want to update Clangd, they should run again the install.py script. That's necessarily something they are familiar with since they have to do it at least once to install the plugin.

I am not sure people would be familiar with something they only did once a few months ago, but even if that's the case; an LLVM release happens once every 6 months, so saying "you should also update YCM every 6 months to get latest features" on this page, where they would (hopefully) only look once, would persist in their minds for such a long time.
Whereas if they install clangd through clang-tools, it will get updated automatically without any extra effort from user.

I think it's better to say this than giving the impression that YCM will never update Clangd and assuming that all Vim users have Clangd in their path.

The assumption is rather on the users that landed on our page to setup clangd for their workflow without any prior setup, not all vim users, therefore we start with instructions on installing clangd. So if they followed the instructions it is safe to assume that they have clangd in their path.

I also see your point, it is unfortunate that text can be interpreted as YCM never updates clangd we should definitely clarify that YCM is making new releases of third party binaries but user needs to explicitly fetch those updates.

@micbou
Copy link
Author

micbou commented Mar 19, 2019

The assumption is rather on the users that landed on our page to setup clangd for their workflow without any prior setup, not all vim users, therefore we start with instructions on installing clangd. So if they followed the instructions it is safe to assume that they have clangd in their path.

In that case, I would suggest to say that YCM can be configured to work with Clangd by simply adding the following to vimrc:

let g:ycm_clangd_binary_path = exepath('clangd')

(which will become let g:ycm_clangd_binary_path = 'clangd' once PR ycm-core/ycmd#1210 is merged) and then mention that if Clangd is not installed on the system, YCM offers the possibility to install and use a bundled Clangd by passing the --clangd-completer option to the install.py script and leaving out the g:ycm_clangd_binary_path option. Finally, add a note that the script must be run again to update that bundled Clangd.

@bstaletic
Copy link

ycm-core/ycmd#1210 has been merged a long time ago. We've just had a user with installation troubles because the clangd website recommends let g:ycm_clangd_binary_path = exepath('clangd'). The user's system only had clangd version 8 installed, which shows there are two assumptions wrong here:

  1. Users who are reading clangd.github.io have latest clangd installed. (What about LTS releases that don't get updated for 2 years?)
  2. Bundled clangd doesn't get updated. (To everyone who didn't read this issue that means "doesn't get updated at all".)

Can we do anything to get this issue resolved?

@puremourning
Copy link

Another user today following the advice on clangd.github.io was broken because their clangd was too old for YCM. Can we remove this recommendation please ?

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

No branches or pull requests

5 participants