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

Activity

1. DanTup

   added

   is bug

   in editorRelates to code editing or language featuresRelates to code editing or language features

   on May 30, 2022
2. DanTup

   added this to the v3.42.0 milestone on May 30, 2022
3. DanTup commented on May 30, 2022

   

   DanTup

   on May 30, 2022

   MemberAuthor

   More actions

   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?
4. bwilkerson commented on May 30, 2022

   

   bwilkerson

   on May 30, 2022

   More actions

   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.
5. DanTup commented on May 30, 2022

   

   DanTup

   on May 30, 2022

   MemberAuthor

   More actions

   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).
6. DanTup

   closed this as completedin 5b4d2e2on May 30, 2022
7. DanTup commented on May 30, 2022

   

   DanTup

   on May 30, 2022

   Last edited by DanTup

   MemberAuthor

   More actions

   @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.
8. bwilkerson commented on May 30, 2022

   

   bwilkerson

   on May 30, 2022

   More actions

   Thanks! I can confirm that that solved the problem for me too.
9. copybara-service

   added a commit that references this issue on May 31, 2022

   [analysis_server] Don't use 0 for LSP choice placeholders

   ...

   8cc19f9
10. keertip

    added a commit that references this issue on Sep 2, 2025

    Fix handling of choice tabstops + workaround servers 0th choice tabstops

    ...

    This issue will close once commit 5b4d2e2 is merged into the 'master' branch.

    5b4d2e2