Override completion inserts extra pair of () when committing with (

[dibarbet]

When committing an override completion item with (, an extra () gets inserted.

[dibarbet]

Issue appears to be that the commit character ( gets inserted and the after resolve text edit runs (which doesn't know about the commit character). The extra ) is coming from vscode brace completion. cc @genlu for ideas - looks like there was some notion of this here, but I couldn't figure out what the intention was - https://github.com/dotnet/vscode-csharp/blob/main/src/lsptoolshost/commands.ts#L137

There is some additional complexity - await completion likely wants the commit character inserted (it inserts a complex edit that ends before the commit character location - async Task DoSomethingAsync()\r\n {\r\n await - so committing with space should insert space after)

Note - ( is only a commit character if override completion starts on the method name. After just typing override for example (

Some ideas

1. Have the textedit returned by the server include the entire word range, then use vscode's document.getWordRangeAtPosition(textEdit.start) to replace the entire word range. - probably won't work - the word range doesn't include the inserted (
2. Call the server to re-resolve the edit based on the new document with the commit char
3. Somehow extend the text edit to include the commit character (not sure - we don't know how the item was committed)
   a. LSP spec guarantees commit character is a single character - server could indicate the possible commit char
   b. Cannot just check if text edit text from the start exactly matches document text - the text edit might start above (for example you just type override which returns a text edit like public override <...>

Some example text edits

"newText": "public override void AbstractMethod()\r\n    {\r\n        throw new NotImplementedException();\r\n    }\r\n\r\n"

"newText": "bstractMethod()\r\n    {\r\n        throw new NotImplementedException();\r\n    }"

[genlu]

Hmm, in VS we disable further type char command handler to bypass things like brace completion. I have to examine commit chars for our existing complex edit items to have a better sense of the issue, but from the examples above (overrride and await completion), it seems one thing we might use to decide whether a commit char should be inserted is if it's part of the display text of a complex edit? e.g. ( is part of DoSomething() but is not part of await. This would be a very crude heuristic, maybe it would work for most cases in vscode?

[dibarbet]

it seems one thing we might use to decide whether a commit char should be inserted is if it's part of the display text of a complex edit?

A few issues

1. We don't decided whether or not the commit character is inserted, the LSP client does and currently has no way to not insert the commit character
2. The display text or label on the completion item does not contain the parenthesis - it would only be DoSomething.
3. We don't know what the display text or label is at the time we do the followup text edit (though we could add it onto the data object).

One potential idea was to check if the word at the cursor location + the next character was present inside the followup text edit text - and if so expand the range to the next character? Main issue I was worrying about is if the followup character happened to already be in the document (and wasn't used as a commit character). I haven't been able to come up with a broken scenario for that, but I'm not convinced.