treewide: do not overwrite lsp-client-settings changed by user

Hi-Angel · Hi-Angel · commit 42b0e44d79df · 2024-04-08T23:46:15.000+03:00
If a user has set a setting inside `lsp-client-settings`, changing its value will result in a silent bug in user configuration, that looks like a regression from `lsp-mode` mode update. Let's avoid that by adding a new function `lsp-register-new-settings` that avoids overwritting whatever was set by user and make use of it treewide. While at it, prohibit `lsp-register-custom-settings` from internal use for all the same reasons. Fixes: #4420
diff --git a/clients/lsp-ansible.el b/clients/lsp-ansible.el
@@ -188,7 +188,7 @@ Python virtual environment."
                 '(:npm :package "@ansible/ansible-language-server"
                        :path "ansible-language-server"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("ansible.ansible.path" lsp-ansible-ansible-path)
    ("ansible.ansible.useFullyQualifiedCollectionNames" lsp-ansible-use-fully-qualified-collection-names t)
    ("ansible.validation.enabled" lsp-ansible-validation-enabled t)
diff --git a/clients/lsp-css.el b/clients/lsp-css.el
@@ -198,7 +198,7 @@ server."
           (const "messages")
           (const "verbose")))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("css.trace.server" lsp-css-trace-server)
    ("css.lint.unknownAtRules" lsp-css-lint-unknown-at-rules)
    ("css.lint.idSelector" lsp-css-lint-id-selector)
diff --git a/clients/lsp-elixir.el b/clients/lsp-elixir.el
@@ -169,7 +169,7 @@ be available here: https://github.com/elixir-lsp/elixir-ls/releases/"
              :binary-path lsp-elixir-server-command
              :set-executable? t))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("elixirLS.dialyzerEnabled" lsp-elixir-dialyzer-enabled t)
    ("elixirLS.dialyzerWarnOpts" lsp-elixir-dialyzer-warn-opts)
    ("elixirLS.dialyzerFormat" lsp-elixir-dialyzer-format)
diff --git a/clients/lsp-fsharp.el b/clients/lsp-fsharp.el
@@ -249,7 +249,7 @@ available, else the globally installed tool."
         (push '(:AutomaticWorkspaceInit . t) opts)
       opts)))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  `(("FSharp.KeywordsAutocomplete" lsp-fsharp-keywords-autocomplete t)
    ("FSharp.ExternalAutocomplete" lsp-fsharp-external-autocomplete t)
    ("FSharp.Linter" lsp-fsharp-linter t)
diff --git a/clients/lsp-go.el b/clients/lsp-go.el
@@ -301,7 +301,7 @@ $GOPATH/pkg/mod along with the value of
   :risky t
   :package-version '(lsp-mode "8.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("gopls.usePlaceholders" lsp-go-use-placeholders t)
    ("gopls.hoverKind" lsp-go-hover-kind)
    ("gopls.buildFlags" lsp-go-build-flags)
diff --git a/clients/lsp-groovy.el b/clients/lsp-groovy.el
@@ -48,7 +48,7 @@
   :risky t
   :type 'lsp-string-vector)
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("groovy.classpath" lsp-groovy-classpath)))
 
 (lsp-register-client
diff --git a/clients/lsp-haxe.el b/clients/lsp-haxe.el
@@ -170,7 +170,7 @@
   :type 'string
   :group 'lsp-haxe)
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("haxe.hxml" lsp-haxe-hxml)
    ("haxe.postfixCompletion" lsp-haxe-postfix-completion)
    ("haxe.exclude" lsp-haxe-exclude)
diff --git a/clients/lsp-html.el b/clients/lsp-html.el
@@ -159,7 +159,7 @@ styles."
   :group 'lsp-html
   :package-version '(lsp-mode . "6.1"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("html.trace.server" lsp-html-trace-server)
    ("html.autoClosingTags" lsp-html-auto-closing-tags t)
    ("html.validate.styles" lsp-html-validate-styles t)
diff --git a/clients/lsp-idris.el b/clients/lsp-idris.el
@@ -48,7 +48,7 @@
   :type '(choice (:tag "off" "messages" "verbose"))
   :package-version '(lsp-mode . "9.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
   '(("idris2-lsp.trace.server" lsp-idris2-lsp-trace-server)
     ("idris2-lsp.path" lsp-idris2-lsp-path)))
 
diff --git a/clients/lsp-javascript.el b/clients/lsp-javascript.el
@@ -648,7 +648,7 @@ name (e.g. `data' variable passed as `data' parameter)."
   :type 'boolean
   :package-version '(lsp-mode . "9.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("javascript.autoClosingTags" lsp-javascript-auto-closing-tags t)
    ("javascript.implicitProjectConfig.checkJs" lsp-javascript-implicit-project-config-check-js t)
    ("javascript.implicitProjectConfig.experimentalDecorators" lsp-javascript-implicit-project-config-experimental-decorators t)
diff --git a/clients/lsp-json.el b/clients/lsp-json.el
@@ -54,7 +54,7 @@ here, https://github.com/emacs-lsp/lsp-mode/issues/3368#issuecomment-1049635155.
   :group 'lsp-json
   :package-version '(lsp-mode . "6.3"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("json.schemas" lsp-json-schemas)
    ("http.proxy" lsp-http-proxy)
    ("http.proxyStrictSSL" lsp-http-proxyStrictSSL)))
diff --git a/clients/lsp-kotlin.el b/clients/lsp-kotlin.el
@@ -149,7 +149,7 @@ Requires lsp-inlay-hints-mode."
   :type 'boolean
   :group 'lsp-kotlin)
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("kotlin.externalSources.autoConvertToKotlin" lsp-kotlin-external-sources-auto-convert-to-kotlin t)
    ("kotlin.externalSources.useKlsScheme" lsp-kotlin-external-sources-use-kls-scheme t)
    ("kotlin.debugAdapter.path" lsp-kotlin-debug-adapter-path)
diff --git a/clients/lsp-lua.el b/clients/lsp-lua.el
@@ -488,7 +488,7 @@ and `../lib` ,exclude `../lib/temp`.
   :package-version '(lsp-mode . "8.0.0")
   :group 'lsp-lua-language-server)
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("files.associations" lsp-lua-files-associations t)
    ("files.exclude" lsp-lua-files-exclude t)
    ("Lua.workspace.useGitIgnore" lsp-lua-workspace-use-git-ignore t)
diff --git a/clients/lsp-magik.el b/clients/lsp-magik.el
@@ -121,7 +121,7 @@
                       (lsp--set-configuration (lsp-configuration-section "magik"))))
   :server-id 'magik))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  `(("magik.javaHome" lsp-magik-java-home)
    ("magik.smallworldGis" lsp-magik-smallworld-gis)
    ("magik.typing.typeDatabasePaths" lsp-magik-typing-type-database-paths)
diff --git a/clients/lsp-markdown.el b/clients/lsp-markdown.el
@@ -79,7 +79,7 @@ For a complete list of plugins, check:
                 '(:npm :package "unified-language-server"
                        :path "unified-language-server"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  `(("unified-language-server.remark-parse.plugins" lsp-markdown-remark-plugins)
    ("unified-language-server.remark-parse.checkTextWith.setting" lsp-markdown-remark-check-text-with-setting)
    ("unified-language-server.remark-parse.checkTextWith.mutator" lsp-markdown-remark-check-text-with-mutator)))
diff --git a/clients/lsp-openscad.el b/clients/lsp-openscad.el
@@ -60,7 +60,7 @@
   :type 'string
   :group 'lsp-openscad)
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("openscad.search_paths" lsp-openscad-search-paths)
    ("openscad.fmt_exe" lsp-openscad-format-exe)
    ("openscad.fmt_style" lsp-openscad-format-style)))
diff --git a/clients/lsp-perl.el b/clients/lsp-perl.el
@@ -88,7 +88,7 @@ Defaults to 0."
   :group 'lsp-perl
   :package-version '(lsp-mode . "8.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("perl.perlCmd" lsp-perl-perl-cmd)
    ("perl.perlInc" lsp-perl-perl-inc)
    ("perl.fileFilter" lsp-perl-file-filter)
diff --git a/clients/lsp-perlnavigator.el b/clients/lsp-perlnavigator.el
@@ -111,7 +111,7 @@ default to ~/.perlcriticrc. (no aliases, .bat files or ~/)."
   :group 'lsp-perlnavigator
   :package-version '(lsp-mode . "9.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("perlnavigator.trace.server" lsp-perlnavigator-trace-server)
    ("perlnavigator.logging" lsp-perlnavigator-logging t)
    ("perlnavigator.includePaths" lsp-perlnavigator-include-paths)
diff --git a/clients/lsp-pls.el b/clients/lsp-pls.el
@@ -103,7 +103,7 @@ changes behavior depending on the contents of @ARGV."
   :group 'lsp-pls
   :package-version '(lsp-mode . "9.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("pls.cmd"                      lsp-pls-executable)
    ("pls.args"                     lsp-pls-arguments)
    ("pls.cwd"                      lsp-pls-working-dir)
diff --git a/clients/lsp-pwsh.el b/clients/lsp-pwsh.el
@@ -195,7 +195,7 @@ extension."
   :group 'lsp-pwsh
   :package-version '(lsp-mode . "6.2"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("powershell.developer.featureFlags" lsp-pwsh-developer-feature-flags)
    ("powershell.developer.editorServicesWaitForDebugger" lsp-pwsh-developer-editor-services-wait-for-debugger t)
    ("powershell.codeFormatting.useCorrectCasing" lsp-pwsh-code-formatting-use-correct-casing t)
diff --git a/clients/lsp-pyls.el b/clients/lsp-pyls.el
@@ -430,7 +430,7 @@ So it will rename only references it can find."
             (setenv "PYENV_VERSION" pyenv-version)
             python-env))))))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("pyls.rope.ropeFolder" lsp-pyls-rope-rope-folder)
    ("pyls.rope.extensionModules" lsp-pyls-rope-extension-modules)
    ("pyls.plugins.rope_rename.enabled" (lambda () (eq lsp-pyls-rename-backend 'rope)) t)
diff --git a/clients/lsp-pylsp.el b/clients/lsp-pylsp.el
@@ -573,7 +573,7 @@ So it will rename only references it can find."
             (setenv "PYENV_VERSION" pyenv-version)
             python-env))))))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("pylsp.rope.ropeFolder" lsp-pylsp-rope-rope-folder)
    ("pylsp.rope.extensionModules" lsp-pylsp-rope-extension-modules)
    ("pylsp.plugins.rope_rename.enabled" (lambda () (eq lsp-pylsp-rename-backend 'rope)) t)
diff --git a/clients/lsp-rf.el b/clients/lsp-rf.el
@@ -108,7 +108,7 @@ Glob patterns as strings are accepted (eg. *bad.robot between double quotes)"
                              x))
                 seq)))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(
    ("rfLanguageServer.trace.server" lsp-rf-language-server-trace-server)
    ("rfLanguageServer.logLevel" lsp-rf-language-server-log-level)
diff --git a/clients/lsp-rust.el b/clients/lsp-rust.el
@@ -285,7 +285,7 @@ is often the type local variable declaration."
   :group 'lsp-rust-rls
   :package-version '(lsp-mode . "6.1"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("rust.show_hover_context" lsp-rust-show-hover-context t)
    ("rust.full_docs" lsp-rust-full-docs t)
    ("rust.build_command" lsp-rust-build-command)
diff --git a/clients/lsp-sml.el b/clients/lsp-sml.el
@@ -70,7 +70,7 @@ more information."
   :type 'string
   :group 'lsp-sml)
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("millet.format.engine" lsp-sml-millet-format-engine)
    ("millet.server.diagnostics.filter" lsp-sml-millet-server-diagnostics-filter)
    ("millet.server.diagnostics.moreInfoHint.enable" lsp-sml-millet-server-diagnostics-moreInfoHint-enable)
diff --git a/clients/lsp-solargraph.el b/clients/lsp-solargraph.el
@@ -136,7 +136,7 @@
       (append '("bundle" "exec") lsp-solargraph-server-command)
     lsp-solargraph-server-command))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("solargraph.logLevel" lsp-solargraph-log-level)
    ("solargraph.folding" lsp-solargraph-folding t)
    ("solargraph.references" lsp-solargraph-references t)
diff --git a/clients/lsp-svelte.el b/clients/lsp-svelte.el
@@ -225,7 +225,7 @@ Example: `((css-unused-selector . ignore) (unused-export-let . error))"
   :type 'boolean
   :package-version '(lsp-mode . "8.0.0"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("svelte.plugin.svelte.rename.enable" lsp-svelte-plugin-svelte-rename-enable t)
    ("svelte.plugin.svelte.selectionRange.enable" lsp-svelte-plugin-svelte-selection-range-enable t)
    ("svelte.plugin.svelte.codeActions.enable" lsp-svelte-plugin-svelte-code-actions-enable t)
diff --git a/clients/lsp-verilog.el b/clients/lsp-verilog.el
@@ -160,7 +160,7 @@
                      :library-folders-fn 'lsp-clients-svlangserver-get-workspace-additional-dirs
                      :server-id 'svlangserver))
 
-(lsp-register-custom-settings '(("systemverilog.includeIndexing" lsp-clients-svlangserver-includeIndexing)
+(lsp-register-new-settings '(("systemverilog.includeIndexing" lsp-clients-svlangserver-includeIndexing)
                                 ("systemverilog.excludeIndexing" lsp-clients-svlangserver-excludeIndexing)
                                 ("systemverilog.defines" lsp-clients-svlangserver-defines)
                                 ("systemverilog.launchConfiguration" lsp-clients-svlangserver-launchConfiguration)
diff --git a/clients/lsp-vetur.el b/clients/lsp-vetur.el
@@ -264,7 +264,7 @@ Set a source to \"\" to disable it.
   :group 'lsp-vetur
   :package-version '(lsp-mode . "6.1"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("vetur.trace.server" lsp-vetur-trace-server)
    ("vetur.ignoreProjectWarning" lsp-vetur-ignore-project-warning t)
    ("vetur.format.scriptInitialIndent" lsp-vetur-format-script-initial-indent t)
diff --git a/clients/lsp-volar.el b/clients/lsp-volar.el
@@ -76,7 +76,7 @@
                 '(:system "vue-language-server")
                 '(:npm :package "@vue/language-server" :path "vue-language-server"))
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("typescript.tsdk"
     (lambda ()
       (if-let ((project-root (lsp-workspace-root))
diff --git a/clients/lsp-xml.el b/clients/lsp-xml.el
@@ -255,7 +255,7 @@ The value for `enabled' can be always, never or onValidSchema."
   :group 'lsp-xml
   :package-version '(lsp-mode . "6.1"))
 
-(lsp-register-custom-settings '
+(lsp-register-new-settings '
  (("xml.validation.schema" lsp-xml-validation-schema)
   ("xml.validation.resolveExternalEntities" lsp-xml-validation-resolve-external-entities)
   ("xml.validation.enabled" lsp-xml-validation-enabled t)
diff --git a/clients/lsp-yaml.el b/clients/lsp-yaml.el
@@ -129,7 +129,7 @@ Limited for performance reasons."
 (defvar lsp-yaml--schema-store-schemas-alist nil
   "A list of schemas fetched from schema stores.")
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("yaml.format.enable" lsp-yaml-format-enable t)
    ("yaml.format.singleQuote" lsp-yaml-single-quote t)
    ("yaml.format.bracketSpacing" lsp-yaml-bracket-spacing)
diff --git a/clients/lsp-zig.el b/clients/lsp-zig.el
@@ -250,7 +250,7 @@ and not the global storage."
 ;;
 ;;; Core
 
-(lsp-register-custom-settings
+(lsp-register-new-settings
  '(("zls.enable_snippets" lsp-zls-enable-snippets t)
    ("zls.enable_argument_placeholders" lsp-zig-enable-argument-placeholders t)
    ("zls.enable_build_on_save" lsp-zig-enable-build-on-save t)
diff --git a/lsp-mode.el b/lsp-mode.el
@@ -8539,12 +8539,38 @@ optional flag that should be non-nil for boolean settings, when it is nil the
 property will be ignored if the VALUE is nil.
 
 Example: `(lsp-register-custom-settings `((\"foo.bar.buzz.enabled\" t t)))'
-\(note the double parentheses)"
+\(note the double parentheses)
+
+Internal note: this function should not be used in by `lsp' interanlly
+due to potentially overriding user settings. Use
+`lsp-register-new-settings' instead."
   (mapc
    (-lambda ((path . rest))
      (puthash path rest lsp-client-settings))
    props))
 
+(defun lsp-register-new-settings (props)
+  "Register PROPS that did not exist yet.
+
+This function avoids overwriting a setting that has already existed. For
+overwriting purposes use `lsp-register-custom-settings'.
+
+PROPS is list of triple (path value boolean?) where PATH is the path to the
+property; VALUE can be a literal value, symbol to be evaluated, or either a
+function or lambda function to be called without arguments; BOOLEAN? is an
+optional flag that should be non-nil for boolean settings, when it is nil the
+property will be ignored if the VALUE is nil.
+
+Example: `(lsp-register-new-settings `((\"foo.bar.buzz.enabled\" t t)))'
+\(note the double parentheses)
+
+"
+  (mapc
+   (-lambda ((path . rest))
+     (unless (gethash path lsp-client-settings)
+       (puthash path rest lsp-client-settings)))
+   props))
+
 (defun lsp-region-text (region)
   "Get the text for REGION in current buffer."
   (-let (((start . end) (lsp--range-to-region region)))
@@ -8572,7 +8598,7 @@ TBL - a hash table, PATHS is the path to the nested VALUE."
   (let ((path (plist-get args :lsp-path)))
     (cl-remf args :lsp-path)
     `(progn
-       (lsp-register-custom-settings
+       (lsp-register-new-settings
         (quote ((,path ,symbol ,(equal ''boolean (plist-get args :type))))))
 
        (defcustom ,symbol ,standard ,doc
diff --git a/test/lsp-common-test.el b/test/lsp-common-test.el
@@ -169,8 +169,8 @@
   :risky t
   :type 'string)
 
-(lsp-register-custom-settings '(("section2.nested.prop1" lsp-nested-prop1)))
-(lsp-register-custom-settings '(("section2.nested.prop2" lsp-nested-prop2)))
+(lsp-register-new-settings '(("section2.nested.prop1" lsp-nested-prop1)))
+(lsp-register-new-settings '(("section2.nested.prop2" lsp-nested-prop2)))
 
 (ert-deftest lsp--custom-settings-test-2 ()
   (let ((actual (lsp-ht->alist (lsp-configuration-section "section2"))))
@@ -192,7 +192,7 @@
   :risky t
   :type 'string)
 
-(lsp-register-custom-settings '(("section3.prop1" lsp-prop3 t)))
+(lsp-register-new-settings '(("section3.prop1" lsp-prop3 t)))
 
 (ert-deftest lsp--boolean-property ()
   (cl-assert (equal (lsp-ht->alist  (lsp-configuration-section  "section3"))
@@ -203,7 +203,7 @@
     (cl-assert (equal (aref (lsp--build-workspace-configuration-response request) 0)
                       :json-false))))
 
-(lsp-register-custom-settings '(("section4.prop1" "value")))
+(lsp-register-new-settings '(("section4.prop1" "value")))
 
 (ert-deftest lsp--non-boolean-property ()
   (cl-assert (equal (lsp-ht->alist  (lsp-configuration-section "section4"))
