Allow CompletionItem to control how text is being inserted

[jrieken]

Today, when inserting completion items (and snippets) that span multiple lines we adjust whitespace. That's not always wanted (see microsoft/language-server-protocol#83) and we should expose a flag that controls how the CompletionItem#insertText is being inserted.

In Roslyn, CompletionItemRules exist and we should either have something comparable or inline a property ala "insertAsIs"

[jrieken]

This will be finalised in September.

[jrieken]

Moving out as we have gotten too little feedback on this

[jrieken]

Plan is to add CompletionItemInsertTextRules that define how/if the insertText is modified when accept a completion. For now, only KeepWhitespace for the future maybe Format. This is the current proposal: https://github.com/Microsoft/vscode/blob/993787821f95c2e376d33dec4f00d054ce97d17d/src/vs/vscode.proposed.d.ts#L16-L37

[jrieken]

cc @kdvolder

[jrieken]

@kdvolder One think we didn't yet consider is the multi cursor case. We hide the fact that there can be multiple cursors in the document, only the primary cursors asks for completions, and for each cursor we adjust whitespace. Now, with KeepWhitespace this will become problematic unless all cursor are in the same column. A solution could be to honor KeepWhitespace only for the primary cursor and not for secondary cursors (on different columns). Ideas? Suggestions?

[kdvolder]

One thing we didn't yet consider is the multi cursor case.

Let me be frank here. Actually... I don't care about it at all. Personally I find that its not worth making things like this overly complex just to support that feature. It is an unimportant edge case to me and I think 'multi cursor' is more of a gimmick that looks rather good in carefully crafted demo sequence videos rather than an actually useful editor feature that I rely on when I'm actually editing code day to day.

In fact when somehow the multi-cursor does activate for me (which really rarely seems to happen to me at all). I often find it annoying more than helpful because it usually doesn't make the changes that I wanted anyways and I then end up having go and manually fix/undo changes at the other cursor sites.

I realize this is just personal opinion and it doesn't solve your problem. So let me try to say something constructive as well.

I think the key here is to just do a 'best effort' approach. I.e. I think you seem to be in a mindset that you want to handle 100% of all possible multi-cursor situations perfectly. But I think you don't have to make a 100% guarantee that you will actually apply the edits on all cursor sites for every possible edit, no matter how complex it is.

If the edits are too complex, you can just 'bail out' and only apply the edit on the main cursor alone. I think that would actually be better than doing something that's probably not what the user wanted anyway.

Basically what I'm saying is this. You should formulate a set of 'testable pre-conditions' that you can do that will detect whether a edit looks 'simple enough' so that you can reasonably apply it to all the cursor sites and you can be confident that the result will probably be what the user wanted. This will hopefully deal with the bulk of simple completions (we assume that most of them are just simple 'on the same line' text substitutions).

And my advice is then, don't worry too much about the other super complex cases. They are not common. If you hit them, simply ignore the other cursors and only apply the edit to the main cursor. As a user, I think that probably won't be any worse than turning my other cursors into something I didn't want anyway.

[jrieken]

hehe. if you don't care then you are probably not the best advisor on the topic 🙊 well, this is what I have done: when running with keepWhitespace and when hitting a secondary cursor that has different leading whitespace then we ignore keepWhitespace. That has some flaws and it would be smarter to only add the whitespace-delta but I am willing to wait for that bug...

I think you seem to be in a mindset that you want to handle 100% of all possible multi-cursor situations perfectly

This is by experience and we had bugs with multicursor, snippets, and different prefixes that should be replaced... But yeah, being pragmatic is the way to do. I think the API proposal is properly inline with what we have today and a logical extension. We have no plans to leak multi-cursor to the extension API.

It is an unimportant edge case to me and I think 'multi cursor' is more of a gimmick that looks rather good in carefully crafted demo sequence videos rather than an actually useful editor feature that

You should give it another try. Trust me this can unlock editing scenarios that blow your mind. I usually use this in combination with Cmd+D to get started (nobody cmd/alt clicks), multi-cursor word navigation, and then editing (including IntelliSense).

[kdvolder]

hehe. if you don't care then you are probably not the best advisor on the topic

Yes, that's a fair point. But you asked so I replied. Of course its entirely upto you to make up your own mind and appropriately assess the usefulness of my suggestions :-)

You should give it another try. ... with Cmd+D to get started

Okay, I will remember that. I admit it may just be that I need to figure out how to properly use this. I never really actively seek it out.

I still feel though that multi-cursor support should never really be used as a justification to gimp or diminish single cursor editing experience / features. getting in the way of plain single-cursor support.

We have no plans to leak multi-cursor to the extension API.

That is good to hear. Too bad that the LSP protocol already has 'leaked' some complexities into the single-cursor completion protocol that, as far as I understand, are only there because of worries around multi-cursor. I'm talking about the whole debacle around main-edit vs extra edits and in particular the 'strange' restrictions that the main edit is explicitly subjected too (microsoft/language-server-protocol#92)

Anyhoo ... I am going of topic a bit. So you don't have to reply (besides I think you aren't really responsible / in control of LSP spec). But I do wish in that instance a more 'pragmatic' approach would have been taken too (i.e allowing the more complex edits, even if the flawless working of multi-cursor stuff can't be gauranteed for the more complex edits). That would have been so much better for some of our use-cases than outright 'banning' the more complex edits altogether right in the LSP spec. We have basically had to work around these annoying limitation by simply implementing our own commands on the client-side to apply our edits exactly how we want and just put a 'dummy' edit into the completion item.

[jrieken]

Fair feedback. In fact, I have not contributed a single line to code or design spec to the LSP. I am in charge of the VS Code extension API - which is driven by VS Code usages and also by its limitations (!). It's obvious that the LSP is influenced by the extension API (and vice versa), but ideally the LSP isn't going too much in any direction but keeps the balance to please all editors. This is where the community is important because we happen to be experts in VS Code.

[jrieken]

This is now closed a proposed API. We will finalise this in November.

[jrieken]

To verify, write an extension that inserts multiline completions items and the controls how insertion happens via this new, proposed api: https://github.com/Microsoft/vscode/blob/25589f6cc4aa3450450fa0f49979fcf7835f52d4/src/vs/vscode.proposed.d.ts#L16-L39