Support numberic textDocumentSync capability

[bashful-strix]

Information

VIM version

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Dec 17 2021 18:32:08) macOS version - x86_64 Included patches: 1-2671, 3402, 3409, 3428, 3489

Operating System: MacOS Monterey 12.2.1

What went wrong

According to the spec, language servers are allowed to return a number for the textDocumentSync capability, rather than a dictionary of specific values (the purescript-language-server does this for instance), but this change prevents these servers running on save events.

After much digging (hard to track down since the server and ALE are all working correctly. including omnicomplete, etc.) I have rather crudely fixed this locally with this patch

M autoload/ale/lsp.vim
@@ -292,6 +292,9 @@ function! s:UpdateCapabilities(conn, capabilities) abort
                 let a:conn.capabilities.includeText = 1
             endif
         endif
+    elseif type(get(a:capabilities, 'textDocumentSync')) is v:t_number
+    \&& get(a:capabilities, 'textDocumentSync') > 0 " 1 = Full 2 = Incremental
+        let a:conn.capabilities.did_save = 1
     endif
 endfunction

I'm afaird I don't know enough about vimscript to say if this is the right way or to update tests...

Reproducing the bug

1. Use a language server which privodes a numberic textDocumentSync value (eg. purescript-language-server)
2. Save a file
3. Observe no diagnostics, no logs in ALE or the language server

:ALEInfo

 Current Filetype: purescript
Available Linters: ['purescript-language-server']
  Enabled Linters: ['purescript-language-server']
  Ignored Linters: []
 Suggested Fixers: 
  'purs-tidy' - Format PureScript files with purs-tidy.
  'purty' - Format PureScript files with purty.
  'remove_trailing_lines' - Remove all blank lines at the end of a file.
  'trim_whitespace' - Remove all trailing whitespace characters at the end of every line.
 Linter Variables:

let g:ale_purescript_ls_config = {'purescript': {'addSpagoSources': v:true, 'addNpmPath': v:true, 'buildCommand': 'spago --quiet build --purs-args --json-errors'}}
let g:ale_purescript_ls_executable = 'purescript-language-server'
let g:ale_purescript_ls_use_global = 0
 Global Variables:

let g:ale_cache_executable_check_failures = v:null
let g:ale_change_sign_column_color = v:null
let g:ale_command_wrapper = ''
let g:ale_completion_delay = v:null
let g:ale_completion_enabled = 0
let g:ale_completion_max_suggestions = 1000
let g:ale_disable_lsp = 0
let g:ale_echo_cursor = 1
let g:ale_echo_msg_error_str = 'Error'
let g:ale_echo_msg_format = '%code: %%s'
let g:ale_echo_msg_info_str = 'Info'
let g:ale_echo_msg_warning_str = 'Warning'
let g:ale_enabled = 1
let g:ale_fix_on_save = 0
let g:ale_fixers = {'purescript': ['purs-tidy']}
let g:ale_history_enabled = 1
let g:ale_history_log_output = 1
let g:ale_keep_list_window_open = v:null
let g:ale_lint_delay = 200
let g:ale_lint_on_enter = 1
let g:ale_lint_on_filetype_changed = 1
let g:ale_lint_on_insert_leave = 0
let g:ale_lint_on_save = 1
let g:ale_lint_on_text_changed = 'never'
let g:ale_linter_aliases = {}
let g:ale_linters = {}
let g:ale_linters_explicit = 0
let g:ale_linters_ignore = {}
let g:ale_list_vertical = v:null
let g:ale_list_window_size = v:null
let g:ale_loclist_msg_format = v:null
let g:ale_max_buffer_history_size = 20
let g:ale_max_signs = v:null
let g:ale_maximum_file_size = v:null
let g:ale_open_list = v:null
let g:ale_pattern_options = v:null
let g:ale_pattern_options_enabled = v:null
let g:ale_root = {}
let g:ale_set_balloons = 0
let g:ale_set_highlights = 1
let g:ale_set_loclist = 1
let g:ale_set_quickfix = 0
let g:ale_set_signs = 1
let g:ale_sign_column_always = v:null
let g:ale_sign_error = v:null
let g:ale_sign_info = v:null
let g:ale_sign_offset = v:null
let g:ale_sign_style_error = '~>'
let g:ale_sign_style_warning = '~~'
let g:ale_sign_warning = v:null
let g:ale_sign_highlight_linenrs = v:null
let g:ale_statusline_format = v:null
let g:ale_type_map = {}
let g:ale_use_global_executables = v:null
let g:ale_virtualtext_cursor = 0
let g:ale_warn_about_trailing_blank_lines = 1
let g:ale_warn_about_trailing_whitespace = 1
  Command History:

(executable check - success) <user path>/node_modules/.bin/purescript-language-server
(started) ['/bin/bash', '-c', '''<user path>/node_modules/.bin/purescript-language-server'' --stdio']

[hsanson]

LSP spec is difficult to follow, at least for me. Indeed the textDocumentSync property can be either a dictionary or a numeric value but wether the client sends the didSave notification to the server depends on if the server capabilities include the "save" property inside the "textDocumentSync" property:

/**

• If present save notifications are sent to the server.
• If omitted, the notification should not be sent. */ save?: boolean | SaveOptions;

By configuring the server to send didSave notifications if textDocumentSync is numeric larger than 0 I believe we are breaking the protocol and may affect other LSP server implementations.

On the other hand it seems the support for numeric values is for backward compatibility so maybe on previous versions of the protocol was implicit that didSave was sent if this value was either 1 (FULL) or 2 (INCREMETAL). If this is the case then your suggested fix should do the work.

[hsanson]

Other approach would be to ask purescript-language-server developert to update their server to implement a more recent version of the protocol (e.g. 3.16).

[bashful-strix]

Yeah, I found the spec pretty unhelpful. I did consider raising this on purescript-language-server, but thought that since it was documented in the spec it should be raised here instead. I'm happy to raise it as an enhancement there as well, though.

Like I said, that fix was pretty crude and was simply what appeared to fix it while I was debugging, though I'm very unfamiliar with both vimscript and the LSP spec. When I get some time I'll dig around some more; given that the didSave event seems to be separate from didChange, it might be something else going on, though given that all other aspects of ALE and the language server are behaving correctly, it does seem to be something about the save/change communication.

[bashful-strix]

After doing some digging through the implementation of coc.nvim and vim-lsp, and going back through the spec, I believe numeric textDocumentSync should be supported.

The latest spec says

Server Capability:

• property path (optional): textDocumentSync
• property type: TextDocumentSyncKind | TextDocumentSyncOptions.

indicating that both the enum (numeric) and object values are valid for textDocumentSync.

Assuming I found the correct parts of the code, vim-lsp and coc.nvim both account for this situation as well.