Repository: autozimu/LanguageClient-neovim (language: rust)

Issue:
Add support for global project settings
Currently, the file pointed to by `settingsPath` (by default) is a per-project `.vim/settings.json`. However, in many cases, it makes sense to define some settings globally (e.g., `build_on_save`). It'd be nice if LanguageClient read `~/.vim/settings.json` *and* `./.vim/settings.json`, and merged them (with the latter overriding the former).

Second, it'd be fantastic if we could have support for per-language settings similar to [what exists](https://github.com/jonasbb/rls_vscode/blob/e3884bbae7a0012fd7adea9565a2e36343c2f0e5/package.json#L263) in VSCode. For example, if my `settings.json` contains `rust.build_on_save`, the setting `build_on_save` should only be sent if the language is `rust`.


Discussion / hints:
I very much support this idea, but think it would be better to follow the XDG spec. I couldn't find any discussion of how .vim was selected in #179 , but I don't think it's the most intuitive folder name - neovim doesn't even read any files from ~/.vim, as far as I can tell. Trying to read $XDG_CONFIG_HOME/nvim/settings.json, then if not defined ~/.config/nvim/settings.json, and then if that does not exist ~/.vim/settings.json sounds like a good option. I honestly think ~/.vim should be removed altogether, but that would probably break some people's configs by surprise.

In terms of per-project options, it think it might also make sense to just save it as ./.LSPsettings.json or something like that, similar to the way pylint does it, where I believe it checks for ./.pylintrc for per-project settings.

(If the file name is changed, it would of course make sense to change the name of the global settingsPath file to LSPsettings.json or whatever is selected as well).
For per project settings, you can already change the path, and it can also
be absolute path. I’d rather not change the decent default.

For per language server settings, it is  supported. Check for the rust
example in tests folder.

As for global settings, one alternative is pass those settings as
parameters  when specifying language server commands.

On Tue, May 15, 2018 at 09:53 Genevieve Mendoza <notifications@github.com>
wrote:

> I very much support this idea, but think it would be better to follow the
> XDG spec. I couldn't find any discussion of how .vim was selected in #179
> <https://github.com/autozimu/LanguageClient-neovim/pull/179> , but I
> don't think it's the most intuitive folder name - neovim doesn't even read
> any files from ~/.vim, as far as I can tell. Trying to read
> $XDG_CONFIG_HOME/nvim/settings.json, then if not defined
> ~/.confing/nvim/settings.json, and then if that does not exist
> ~/.vim/settings.json sounds like a good option. I honestly think ~/.vim
> should be removed altogether, but that would probably break some people's
> configs by surprise.
>
> In terms of per-project options, it think it might also make sense to just
> save it as ./.LSPsettings.json or something like that, similar to the way
> pylint does it, where I believe it checks for ./.pylintrc for per-project
> settings.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/autozimu/LanguageClient-neovim/issues/431#issuecomment-389237552>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABYt74aZ8hngZWApd4fAbARAQduFrdDjks5tywf6gaJpZM4T_2Xx>
> .
>

Ah, I did not know about the language-specific features! That's great! It'd probably be good if this feature was documented somewhere though.

I'm not sure exactly what you mean about specifying them when giving commands? And would this still then merge the settings with the per-project ones, and make the per-project settings override the global ones when some setting is specified by both?
Somehow related question: is there any doc on what options are allowed in this settings.json file? Can't find anything on Google. Is it related to LSP itself or is it LanguageClient-neovim specific?
It is related to the specific options provided by servers. For example, here are the RLS options: https://github.com/rust-lang-nursery/rls#configuration
(note, however, that that list is [non-exhaustive](https://github.com/rust-lang-nursery/rls/issues/870#issuecomment-389286606))
> specifying them when giving commands

This is an example of achieving global settings by specifying it within commands, https://github.com/cquery-project/cquery/wiki/Initialization-options

I don't think it will be applicable to all settings and all language servers, but thought it might be helpful to some use cases.
@autozimu in particular, as one data point, Rust's RLS does not take any command-line options to configure its behavior. I filed https://github.com/rust-lang/rls/issues/1325
I think it would be great if the language client can
1. check for project specific settings in current folder, and fallback to global settings if not found
2. merge the project specific settings with the global setting (override specified fields only)

One use case is that, MSPYLS requires python interpreter path in initialization options, which is different for projects using virtual environment. But there's other settings like cache directory that I want to share across different projects.

For now, I need to copy the whole settings to project folder, and change the settings path using something like https://github.com/embear/vim-localvimrc (not all projects have local per-project settings). Also if I want to change the cache directory, I need to change settings from all projects.

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 9046d395..56285457 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -357,6 +357,16 @@ name = "itoa"
 version = "0.4.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "json-patch"
+version = "0.2.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "serde 1.0.99 (registry+https://github.com/rust-lang/crates.io-index)",
+ "serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)",
+ "treediff 3.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "jsonrpc-core"
 version = "12.0.0"
@@ -388,6 +398,7 @@ dependencies = [
  "failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "json-patch 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "jsonrpc-core 12.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "log4rs 0.8.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -889,6 +900,14 @@ name = "traitobject"
 version = "0.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "treediff"
+version = "3.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "typemap"
 version = "0.3.3"
@@ -1067,6 +1086,7 @@ dependencies = [
 "checksum iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe6e417e7d0975db6512b90796e8ce223145ac4e33c377e4a42882a0e88bb08"
 "checksum itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5b8467d9c1cebe26feb08c640139247fac215782d35371ade9a2136ed6085358"
 "checksum itoa 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "501266b7edd0174f8530248f87f99c88fbe60ca4ef3dd486835b8d8d53136f7f"
+"checksum json-patch 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "f995a3c8f2bc3dd52a18a583e90f9ec109c047fa1603a853e46bcda14d2e279d"
 "checksum jsonrpc-core 12.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "288dca7f9713710a29e485076b9340156cb701edb46a881f5d0c31aa4f5b9143"
 "checksum kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d"
 "checksum lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bc5729f27f159ddd61f4df6228e827e86643d4d3e7c32183cb30a1c08f604a14"
@@ -1128,6 +1148,7 @@ dependencies = [
 "checksum thread_local 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "c6b53e329000edc2b34dbe8545fd20e55a333362d0a321909685a19bd28c3f1b"
 "checksum time 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)" = "db8dcfca086c1143c9270ac42a2bbd8a7ee477b78ac8e45b19abfb0cbede4b6f"
 "checksum traitobject 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "efd1f82c56340fdf16f2a953d7bda4f8fdffba13d93b00844c25572110b26079"
+"checksum treediff 3.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "761e8d5ad7ce14bb82b7e61ccc0ca961005a275a060b9644a2431aa11553c2ff"
 "checksum typemap 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "653be63c80a3296da5551e1bfd2cca35227e13cdd08c6668903ae2f4f77aa1f6"
 "checksum unicode-bidi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "49f2bd0c6468a8230e1db229cff8029217cf623c767ea5d60bfbd42729ea54d5"
 "checksum unicode-normalization 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "141339a08b982d942be2ca06ff8b076563cbe223d1befd5450716790d44e2426"
diff --git a/Cargo.toml b/Cargo.toml
index 0e4b6a9f..11840b13 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -26,6 +26,7 @@ maplit = "1"
 serde = "1"
 serde_derive = "1"
 serde_json = "1"
+json-patch = "0.2"
 crossbeam = "0.7.3"
 jsonrpc-core = "12"
 lsp-types = { version = "0.70.0", features = ["proposed"] }
diff --git a/autoload/LanguageClient.vim b/autoload/LanguageClient.vim
index 21d40fe5..29b88d71 100644
--- a/autoload/LanguageClient.vim
+++ b/autoload/LanguageClient.vim
@@ -282,6 +282,16 @@ function! s:GetVar(...) abort
     endif
 endfunction
 
+" if the argument is a list, return it unchanged, otherwise return the list
+" containing the argument.
+function! s:ToList(x) abort
+    if type(a:x) == v:t_list
+        return a:x
+    else
+        return [ a:x ]
+    endif
+endfunction
+
 function! s:ShouldUseFloatWindow() abort
     let floatingHoverEnabled = s:GetVar('LanguageClient_useFloatingHover', v:true)
     return s:FLOAT_WINDOW_AVAILABLE && floatingHoverEnabled
diff --git a/doc/LanguageClient.txt b/doc/LanguageClient.txt
index 367b50da..f6546a8b 100644
--- a/doc/LanguageClient.txt
+++ b/doc/LanguageClient.txt
@@ -202,8 +202,10 @@ Valid options: "Error" | "Warning" | "Info" | "Log"
 
 2.12 g:LanguageClient_settingsPath              *g:LanguageClient_settingsPath*
 
-Path for language server settings. If not an absolute path this is relative
-to the workspace directory.
+Path for language server settings, or list of such paths. If not an absolute
+path this is relative to the workspace directory. If several paths are
+provided, then the corresponding settings are merged with precedence going to
+the last file.
 
 Example settings file content: >
     {
diff --git a/src/language_server_protocol.rs b/src/language_server_protocol.rs
index fe3318ae..68e876b4 100644
--- a/src/language_server_protocol.rs
+++ b/src/language_server_protocol.rs
@@ -79,7 +79,7 @@ impl LanguageClient {
             HashMap<String, Vec<String>>,
             Option<String>,
             Option<String>,
-            String,
+            Vec<String>,
             u64,
             Option<RootMarkers>,
             Option<f64>,
@@ -97,7 +97,7 @@ impl LanguageClient {
                 "s:GetVar('LanguageClient_serverCommands', {})",
                 "get(g:, 'LanguageClient_selectionUI', v:null)",
                 "get(g:, 'LanguageClient_trace', v:null)",
-                "expand(get(g:, 'LanguageClient_settingsPath', '.vim/settings.json'))",
+                "map(s:ToList(get(g:, 'LanguageClient_settingsPath', '.vim/settings.json')), 'expand(v:val)')",
                 "!!get(g:, 'LanguageClient_loadSettings', 1)",
                 "get(g:, 'LanguageClient_rootMarkers', v:null)",
                 "get(g:, 'LanguageClient_changeThrottle', v:null)",
@@ -275,13 +275,39 @@ impl LanguageClient {
             return Ok(Value::Null);
         }
 
-        let path = Path::new(root).join(&self.get(|state| state.settingsPath.clone())?);
-        let buffer = read_to_string(&path).with_context(|err| {
-            format!("Failed to read file ({}): {}", path.to_string_lossy(), err)
-        })?;
-        let value = serde_json::from_str(&buffer)?;
-        let value = expand_json_path(value);
-        Ok(value)
+        let mut res = Value::Null;
+        let mut last_err = None;
+        let mut at_least_one_success = false;
+        for orig_path in self.get(|state| state.settingsPath.clone())? {
+            let path = Path::new(root).join(orig_path);
+            let buffer = read_to_string(&path).with_context(|err| {
+                format!("Failed to read file ({}): {}", path.to_string_lossy(), err)
+            });
+            let buffer = match buffer {
+                Err(e) => {
+                    last_err = Some(e.into());
+                    continue;
+                }
+                Ok(x) => x,
+            };
+            let value = serde_json::from_str(&buffer);
+            let value = match value {
+                Err(e) => {
+                    last_err = Some(e.into());
+                    continue;
+                }
+                Ok(x) => x,
+            };
+            let value = expand_json_path(value);
+            json_patch::merge(&mut res, &value);
+            at_least_one_success = true;
+        }
+
+        match last_err {
+            // no file was read and an error happened
+            Some(e) if !at_least_one_success => Err(e),
+            _ => Ok(res),
+        }
     }
 
     fn define_signs(&self) -> Fallible<()> {
diff --git a/src/types.rs b/src/types.rs
index 030d5c32..b7f65b61 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -173,7 +173,7 @@ pub struct State {
     pub diagnostics_max_severity: DiagnosticSeverity,
     pub documentHighlightDisplay: HashMap<u64, DocumentHighlightDisplay>,
     pub windowLogMessageLevel: MessageType,
-    pub settingsPath: String,
+    pub settingsPath: Vec<String>,
     pub loadSettings: bool,
     pub rootMarkers: Option<RootMarkers>,
     pub change_throttle: Option<Duration>,
@@ -252,7 +252,7 @@ impl State {
             diagnostics_max_severity: DiagnosticSeverity::Hint,
             documentHighlightDisplay: DocumentHighlightDisplay::default(),
             windowLogMessageLevel: MessageType::Warning,
-            settingsPath: format!(".vim{}settings.json", std::path::MAIN_SEPARATOR),
+            settingsPath: vec![format!(".vim{}settings.json", std::path::MAIN_SEPARATOR)],
             loadSettings: false,
             rootMarkers: None,
             change_throttle: None,


Test patch:
diff --git a/tests/test.sh b/tests/test.sh
index 2d2d43b0..d7903550 100755
--- a/tests/test.sh
+++ b/tests/test.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 set -o xtrace