Code Actions with snippets that contain "choices" are not handled correctly

[DanTup]

Found by @bwilkerson:

This is likely because of this hack:

Dart-Code/src/extension/analysis/analyzer_lsp_snippet_text_edits.ts

Lines 46 to 50 in bb045c4

	// HACK: This should be checking InsertTextFormat:
	// https://github.com/microsoft/language-server-protocol/issues/724#issuecomment-800334721
	const hasSnippet = /\$(\d+|\{\d+:([^}]*)\})/.test(textEdit.newText);
	// if ((textEdit as any).insertTextFormat === InsertTextFormat.Snippet) {
	if (hasSnippet) {

We should find a way to access insertTextFormat to make this reliable (but if it's not quick, we need to improve this regex to handle choices in the short-term).

[DanTup]

It turns out there are two issues here:

1. The VS Code extension not correctly detecting the snippet and applying the client-side support
2. The server producing a 0th snippet that has choices/suggestions. The 0th tabstop is special (it is the "final tabstop") and it turns out it does not support choices/suggestions

There are two possible fixes for the second one:

1. We use tabstop number 1 (eg. generate ${1|length,i|} = name.length). This will mean the implicit final tabstop goes at the end of the edit (which is just after the = ). This means if the user presses tab after selecting between length/i the cursor will end up after the =
2. We drop the choices/suggestions and just insert the first one (length), keeping it as the 0th/final tabstop. This will mean we exit snippet mode immediately (pressing tab would insert a literal tab).

Neither of these are perfect, so the question is whether it's better to keep the full list of suggestions (and have the odd final tabstop) or provide only the first suggestion without the drop-down list that would usually appear.

Here's how (1) would look (note the grey bar after = .. that's where the final tabstop is):

And (2) would look like this (no choices):

@bwilkerson do you think one of these is particularly better than the other?

[bwilkerson]

I don't have a strong opinion, but I can give you one factor I can think of that might influence the choice.

Server computes the list based on both the name of the outermost function being executed and on some common names given the type. In the case above, that work well and the first name in the list is probably the best. But in other cases that doesn't work as well. For example:

void f(List<int> list) {
  list.getTheThirdElement;
}

extension on List<int> {
  int get getTheThirdElement => this[2];
}

The first suggested name is getTheThirdElement, but the best name (element) is the fourth item in the list.

As a result, I'm kind of favoring keeping the list around.

On the other hand, maybe we should put shorter items (based on the name) first and then always take the first one.

It would be nice to have some information about what names users actually pick in these cases, but we don't, so the choice here is more subjective than I'd like.

[DanTup]

As a result, I'm kind of favoring keeping the list around.

On the other hand, maybe we should put shorter items (based on the name) first and then always take the first one.

If we took the first one, I think the original example above would end up with i instead of length, so that might not be perfect either?

Although I don't really like the additional final tabstop, I also think keeping the list might be overall better. It's also what my local workaround will be (to ship a fix for this before the next SDK release) since it's a simple string replacement.

Unfortunately I had a dig through the LSP client code, and there's no current way for us to read the insertTextFormat field so detecting the snippets with a string-check will have to remain for now (although I'll file an issue to see if we can get a hook for this).

[DanTup]

@bwilkerson I've pushed a new pre-release version of the extension if you'd like the fix for this before the stable release. You can opt-in to pre-release versions in VS Code here (the pre-releases should be generally quite stable.. they're published in the final week before a stable release to get some advanced testing from a larger portion of users than just me):

Pre-release versions have version numbers like x.(ODD).(YYYYMMDD) (because VS Code doesn't yet support proper pre-release version numbers) although this one is 3.41.202205301 with a trailing 1 because I'd already done a pre-release this morning before this fix.

[bwilkerson]

Thanks! I can confirm that that solved the problem for me too.