fix: move additionalTextEdits switch #397
Conversation
Signed-off-by: shane.xb.qian <[email protected]>
|
perhaps this is ok to merge too, it is to make that switch only control the additionalTextedit, and it was for that. |
|
If 'completionTextEdit' option is set, then the expectation is that a snippet completion plugin will expand the snippet and apply the text edits. So I am not sure the |
|
If 'completionTextEdit' option is set, then the expectation is that a snippet completion plugin will expand the snippet and apply the text edits. So I am not sure the `command` in the completion reply can be applied using codeaction.DoCommand() if this option is set. Is it possible to test this change with a snippet plugin to verify this?
supposed i got your concern: you're worrying that switch should toggle 'additionaltext' with 'command' together?
actually i donnot know but per general sense 'command' is not same scope with 'additionaltext',
and/but most specifical thing is i added this 'switch' was just for 'additionaltext'.
and i didnot see any lsp server combine them, but @vimposter said haskell did?
// BTW: i roughly check vsnip, looks it didnot mention any about 'command', so at least for now seems it is not part of it.
…--
shane.xb.qian
|
| completionData = lspserver.resolveCompletion(completionData, true) | ||
| endif | ||
| if !completionData->get('additionalTextEdits', {})->empty() | ||
| \ && opt.lspOptions.completionTextEdit |
There was a problem hiding this comment.
Moving the line here seems a bit fishy to me for another reason:
Suppose that completionTextEdit == false, then only guarding this if condition may perform redundant work. The delayed lspserver.resolveCompletion directly above this if-block might still be executed, but this is completely unnecessary as we won't use the result anywhere.
There was a problem hiding this comment.
well, simplest change i had said in #389 (comment) (but ignore that cursor issue, you had refined it), if just added and moved 'command' part before 'textedit', then this PR no required and nothing needed change;
i am still not sure what case required such 'command' action at here, could you verify your faced case to verify if it's ok to move that part to there (of course please be sure 'command' if was really required)?
There was a problem hiding this comment.
if just added and moved 'command' part before 'textedit', then nothing needed change
I already told you in my original PR that this is not possible, the command has to be applied after the textedit.
if it's ok to move that part to there
With the latest commit a07a550 your PR seems fine to me, although I am not an expert with snippet plugins, as I don't use any.
There was a problem hiding this comment.
if just added and moved 'command' part before 'textedit', then nothing needed change
I already told you in my original PR that this is not possible, the command has to be applied after the textedit.
yes, but i hear I am not entirely sure in that comment.
or now anyway seems only 'haskell' (as you said) worked like this, i have no idea any others, and seems you confirmed it should work like this way, then i think this PR is ready to go.
Signed-off-by: shane.xb.qian <[email protected]>
3 participants
refine #389