Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

ycmd/responses.py line 334 at r1 (raw file):

  def BuildFixitChunkData( chunk ):
    if hasattr( chunk, 'replacement_text' ):

minor, but maybe we should actually put the "to ycmd protocol" as a instance method on the chunk type.

ycmd/completers/language_server/language_server_completer.py line 3593 at r1 (raw file):
so previously we only ever processed one of these 'changes' vs 'documentChanges'.

Now we seem to include both if present.

This seems against the protocol which says:

A workspace edit represents changes to many resources managed in the workspace. The edit should either provide changes or documentChanges. If the client can handle versioned document edits and if documentChanges are present, the latter are preferred over changes.

So I think that we should prefer documentChanges.

ycmd/completers/language_server/language_server_completer.py line 3612 at r1 (raw file):

        chunks.append(
            responses.RenameChunk(
              old_filepath = lsp.UriToFilePath( text_document_edit[ 'oldUri' ] ),

If it recall correctly, this throws InvalidUriException in case that the server sends some bullshit URI like jdt:// and we have to catch it everywhere.

ycmd/completers/language_server/language_server_protocol.py line 310 at r1 (raw file):

        'dynamicRegistration': True
      },
      'workspaceEdit': {

we should consider specifying our FailureHandlingKind

   /**
    * The failure handling strategy of a client if applying the workspace edit
    * fails.
    *
    * @since 3.13.0
    */
   failureHandling?: FailureHandlingKind;

I guess export const Abort: FailureHandlingKind = 'abort'; is what we have.

Regarding "options", like these:

1. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#createFileOptions
2. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#renameFileOptions
3. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#deleteFileOptions

Should we just shove options with the chunks and pass them on to YCM?

Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)

ycmd/responses.py line 334 at r1 (raw file):

Done.

ycmd/completers/language_server/language_server_completer.py line 3593 at r1 (raw file):

This is against the protocol, like I mentioned here:
ycm-core/YouCompleteMe#4267 (comment)

If you really think we should prefer documentChanges, I will do it.

JDT seems to send an empty changes and a populated documentChanges.

ycmd/completers/language_server/language_server_completer.py line 3612 at r1 (raw file):

JDT has made that opt-in long ago.

ycmd/completers/language_server/language_server_protocol.py line 310 at r1 (raw file):

Done.

Should we just shove options with the chunks and pass them on to YCM?

yeah why not. we might as well just use the LSP syntax and semantics for these; why swim against the tide.

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

actually, looking at the LSP spec, it's bonkers:

	/**
	 * Overwrite existing file. Overwrite wins over `ignoreIfExists`
	 */
	overwrite?: boolean;

	/**
	 * Ignore if exists.
	 */
	ignoreIfExists?: boolean;

erm... don't they mean the same thing!

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

No, they mean opposite things. If overwrite, then do not care that $FILE is there, just overwrite it. If ignoreIfExists, ignore the operation and leave that file alone.

Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)

yeah, but I don't see how that's different from having just one of them. What am I missing!?

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)