Add user options {gopls,rls,tsserver,tern,...}_binary_path

[yan12125]

Hi, I'm the current maintainer of neovim-youcompleteme-core-git [1] package on unofficial Arch User Repository (AUR). Recently @bstaletic reached out to me about fixing that package for language completers like gopls or rls. As I aim to provide a package that use system packages than bundled ones, could ycmd also look into $PATH for language completers? For example, check rls in $PATH if ycmd/third_party/rls/bin/rls is not found, and gopls in $PATH if ycmd/third_party/go/src/golang.org/x/tools/cmd/gopls/gopls is not found.

[1] https://aur.archlinux.org/packages/neovim-youcompleteme-core-git/

[bstaletic]

Hi. That is exactly what we're doing for TypeScript completer, though it was never documented. The reason why it wasn't documented is because we never claimed support for it. It was more aimed at developers.

So let's be clear on what means to use the server from $PATH:

• If we update the submodule faster than the package does and something breaks, we won't be making workarounds to keep compatibility with the old version.
  • This is unlikely to be the case for a rolling distribution.
• If the package/distro moves faster and something in YCM is broken with the new version, a pull request would be very appreciated.

In the past, the most severely outdated dependency was requests (python-requests on arch). However, the breaking change in the upstream requests repo wasn't affecting third party packages. Other late updates included a very late move to Omnisharp-Roslyn and RLS.

Let's discuss completer executable paths:

• TSServer can already be found in $PATH
• I have seen OmniSharp-Roslyn already done as a dependency in a different PKGBUILD
• Clangd is used from the path if the user has let g:ycm_use_clangd=1 and let g:ycm_clangd_binary_path='clangd'
• That only leaves JDT.LS for java (aur/jdtls exists and is up to date). Considering that that one was hard to get working and that every update broke the tests, I personally avoid touching it. That said, pull requests are welcome and if the change is not too intrusive, why not.

[bstaletic]

For now, a quick and untested patch for rust and go:

diff --git a/ycmd/completers/go/go_completer.py b/ycmd/completers/go/go_completer.py
index a7cebc49..8100ba63 100644
--- a/ycmd/completers/go/go_completer.py
+++ b/ycmd/completers/go/go_completer.py
@@ -31,7 +31,7 @@ from ycmd.completers.language_server import ( simple_language_server_completer,
                                               language_server_completer )
 
 
-PATH_TO_GOPLS = os.path.abspath( os.path.join( os.path.dirname( __file__ ),
+PATH_TO_GOPLS = utils.PathToFirstExistingExecutable( [ os.path.abspath( os.path.join( os.path.dirname( __file__ ),
   '..',
   '..',
   '..',
@@ -43,7 +43,8 @@ PATH_TO_GOPLS = os.path.abspath( os.path.join( os.path.dirname( __file__ ),
   'tools',
   'cmd',
   'gopls',
-  utils.ExecutableName( 'gopls' ) ) )
+  utils.ExecutableName( 'gopls' ) ) ),
+  'gopls' ] )
 
 
 def ShouldEnableGoCompleter( user_options ):
diff --git a/ycmd/completers/rust/rust_completer.py b/ycmd/completers/rust/rust_completer.py
index 941cec4f..0f15c8b1 100644
--- a/ycmd/completers/rust/rust_completer.py
+++ b/ycmd/completers/rust/rust_completer.py
@@ -36,8 +36,8 @@ LOGFILE_FORMAT = 'rls_'
 RLS_BIN_DIR = os.path.abspath(
   os.path.join( os.path.dirname( __file__ ), '..', '..', '..', 'third_party',
                 'rls', 'bin' ) )
-RUSTC_EXECUTABLE = utils.FindExecutable( os.path.join( RLS_BIN_DIR, 'rustc' ) )
-RLS_EXECUTABLE = utils.FindExecutable( os.path.join( RLS_BIN_DIR, 'rls' ) )
+RUSTC_EXECUTABLE = utils.PathToFirstExistingExecutable( [ os.path.join( RLS_BIN_DIR, 'rustc' ), 'rustc' ] )
+RLS_EXECUTABLE = utils.PathToFirstExistingExecutable( [ os.path.join( RLS_BIN_DIR, 'rls' ), 'rls' ] )
 RLS_VERSION_REGEX = re.compile( r'^rls (?P<version>.*)$' )
 
 

[yan12125]

For now, a quick and untested patch for rust and go:

Awesome! With a quick test, the rust patch makes YCM work fine with the rust package [1] on Arch Linux. gopls is currently missing from the go-tools package [2], so I didn't test it.

The reason why it wasn't documented is because we never claimed support for it. It was more aimed at developers.

Thanks for clarification. A -git package usually indicates that users might need to take risks. It might be broken and even not packaged as upstream intends.

If we update the submodule faster than the package does and something breaks

If the official packages are old, we can either ask Arch devs to update or create another AUR package to track the latest packages. For example rls-git [3] might be useful in case rls from the official rust package is too old. For Omnisharp-Roslyn, I'm fine to co-maintain the AUR package [4] in case its maintainer is too busy.

Considering that that one was hard to get working and that every update broke the tests, I personally avoid touching it.

Thanks for the info. I haven't tested JDT.LS. I may re-add it as a submodule if using system binaries/libraries is not feasible.

Clangd is used from the path if the user has let g:ycm_use_clangd=1 and let g:ycm_clangd_binary_path='clangd'

Thanks for the tip! I will consider switch to such an approach.

TSServer can already be found in $PATH

That's a good news :)

[1] https://www.archlinux.org/packages/extra/x86_64/rust/
[2] https://bugs.archlinux.org/task/63008
[3] https://aur.archlinux.org/packages/rls-git/
[4] https://aur.archlinux.org/packages/omnisharp-roslyn/

[bstaletic]

Glad we're on the same page. My personal wish, which might not be shared by other YCM maintainers, is to eventually end up in a state where ycmd can be packaged outside of YouCompleteMe/third_party, but let's take it one step at a time.

You might also be interested in additional LSP completers. We've made LSP completers pluggable, but...

• They don't have tests and the whole infrastructure supporting them is on best-effort basis, so maybe something sometimes breaks.
  • There's an idea to make some sort of voting system, so we can promote the servers that are in demand to official support with tests.
• They don't/can't have :YcmCompleter FixIt support as it is too server dependent.
  • Again, moan at us to promote the server if you want FixIts.
• Additional contributions to that repository would be appreciated. Or just inform us that "X server works with Y configuration".
• The biggest problem with this is that LSP server developers test against vscode and often disregard the protocol specifications. Evidenced by:
  • Server returns snippets even though client doesn't support them redhat-developer/yaml-language-server#161
  • Server returns snippets even if client doesn't claim to support them vscode-langservers/vscode-json-languageserver-bin#2
  • LSP: Validate responses against client capabilities rust-lang/rust-analyzer#144
  • vue-language-server requires a config to be provided (probably shouldn't) vuejs/vetur#977
  • completion request should include textEdit according to protocol eclipse-jdtls/eclipse.jdt.ls#465
• When testing other servers, I've found that some AUR packages don't work, while the local installation outside of pacman does work.

[puremourning]

As commented on another PR, I like the current clangd completer approach (as it was discussed at length when implemented). It drives a good trade-off between what we can support and flexibility we can provide.

I think we should follow that approach at least for consistency.

All the original discussion points for clangd apply to the other servers mentioned here. JDT.LS is a bit of a wildcard because we do sometimes see regressions or test issues, though they are not always strict jdt.ls bugs. Moreover, the way it is launched is fiddly and awkward (it's not just java ), including requiring the workspace dir etc. etc. etc.

[yan12125]

From a perspective of a packager, the *_binary_path approach used by clangd is better, too. Users will (hopefully) understand that such a configuration is not supported by upstream when adding those vim variables. If everyone agrees, the title of this issue can be changed to Add {gopls,rls,...}_binary_path options. It mighe be better to change the typescript completer to use the binary_path approach to achieve consistency.

Regarding pluggable language completers: the current status sounds horrible. I'd like to skip them in my package for now.

[bstaletic]

Yeah, going with <server>_binary_path sounds good to me, even for TypeScript. As for the pluggable completer, your point of view is understandable, though D and Kotlin work fine. I'd also like to see a haskell user try the haskell language server, because I couldn't figure out how to test it.

[yan12125]

D and Kotlin work fine

Great to know that :)