Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remembered suggestion selections can be annoying #41060

Closed
SDanton opened this issue Jan 2, 2018 · 37 comments
Closed

Remembered suggestion selections can be annoying #41060

SDanton opened this issue Jan 2, 2018 · 37 comments
Assignees
Labels
feature-request Request for new features or functionality on-testplan release-notes Release notes issues suggest IntelliSense, Auto Complete
Milestone

Comments

@SDanton
Copy link

SDanton commented Jan 2, 2018

  • VSCode Version: Version 1.19.1 (1.19.1)
  • OS Version: 10.12.6 (16G1036)

Steps to Reproduce:

  1. Type as per normal e.g. 'actor', see code completion list
  2. Notice that fully matching value 'actor' is not matched, but the next closest value is 'ActorMgr'
  3. (Side effect) Press Escape to avoid matching undesired value

https://imgur.com/lpt0DKg

Expect:

If I type in an exact match, then that should be the value I will get after tabbing / entering, not the next closest match.

@jrieken
Copy link
Member

jrieken commented Jan 3, 2018

The reason for that is that VS Code remembers selected suggestions per prefix and language. When typing actor and selecting ActorMgr it will preselect that on subsequent invocations. Select the actor-suggestion and it's back to 'normal'.

Not yet sure how to handle these situations as there are cases. In some scenarios the same prefix is used for different suggestions in an alternating fashion... Open for suggestions

@jrieken jrieken added suggest IntelliSense, Auto Complete under-discussion Issue is under discussion for relevance, priority, approach labels Jan 3, 2018
@SDanton
Copy link
Author

SDanton commented Jan 3, 2018

The current approach feels like it's trying to be too smart. If we have a case sensitive exact match I feel like that should always override a predictive match. Do we have scenarios where this isn't the case?

Alternatively, if I had entered 'ac' and then pressed tab, then yes I can see ActorMgr matching. Then again, case sensitivity is something I'm very aware of when I code, so I feel it should still put 'actor' first.

Possible rules:

1, Exact match always wins
2, Case matters, if 'ac' is entered, 'ac*' always matches before 'Ac*'

A few other thots:

// Can I turn off "suggestion matching" and enable case-sensitive-exact-match-wins intellisense? Isn't this how intellisense used to work?

// It's unclear to me this kind of matching will ever work in code bases with variables / class names that share common sub-strings. Which I would guess is most code bases.

Consider a code base which has actor, ActorMgr and ActorCtrl. If I use all of these frequently intellisense is going to guess incorrectly 66% of the time. Just because I used 'actor' last time does not mean I will use it this time. The more I think about it the more I'm convinced this just won't work -- it's just too easy to guess incorrectly. I can see cases for "auto-correction" e.g. if I type 'actro' and I've used 'actor' several times in this context before, perhaps it should auto-correct to 'actor'?

(Below are more ideas... they might help in some cases, but I think the feature has fundamental challenges.)

// Perhaps we only suggest after we have context. Consider...

I enter "actor", no suggesting, just case-sensitive intellisense
With actor as my context, we now suggest, so if I enter actor.Anim we suggest Animator

... again this presents challenges. If actor has an Animator and an AnimatorManager, it will guess incorrectly half the time.

// Visual highlighting of the "current match" should be stronger, I don't think this will help much but it will at least make things more obvious.

// Can we better leverage context? It feels like we currently remember selections by project...? Could matches be relative to the code file, class or even function? Not sure this really helps, might make things more confusing actually :).

// What means do we have to "override" a match. Consider...

1, Entering 'actor' pressing tab
2, Getting 'ActorMgr' match
3, Backspacing over 'ActorMgr'
4, Entering 'actor' pressing Escape

... backspacing and then escaping out of the suggestions definitely has intent from where I am sitting, can the system reliable leverage that? Perhaps escaping when there is an exact match matters?

@SDanton
Copy link
Author

SDanton commented Jan 3, 2018

Just wanted to add to this... I'm now more confused as to how the suggestion system works.

Consider below...

https://imgur.com/Fb5Yidl

... having entered '_canRo' how is it that '_canGuardCounter' or '_canChargeCounter' match? Further, how is that _canGuardCounter is the selected match?

I see that the letters 'r' and 'o' are in *GuardCounter and *ChargeCounter but I would have to be a really bad typist to have entered _canRo and expect either of the suggested options.

(By the way, I know how hard everyone works on these features... I don't in any way want to come across as not appreciating all that hard work -- I've been in your shoes :). At the same time I want to help improve VS Code b/c it's the tool I use everyday. If I'm negative or direct, it's only b/c I've found that's the fastest way to make something better. It's never personal.)

@jrieken
Copy link
Member

jrieken commented Jan 4, 2018

Further, how is that _canGuardCounter is the selected match?

Consider two sides of this: Sorting and Selection. The sorting is based on a rank that is computed with the current prefix and each suggestion. From looking at your screen shots the sorting is correct (better matches comes first). Now selection: By default the first item in the list is selected, it's the best match. However, if a user selects a different suggestion (not the best match) for a prefix we remember that and we will select that item the next time this case comes up.

So that's what's happening here, your problem isn't sorting but pre-selection. You can easily "fix" this by just accepting a different suggestion and we believe depending on the current task that happens. For instance I usually complete cons to const but when I'm ghetto-debugging I complete cons to console. It becomes a problem when using then interleaving which I believe is rare.

... having entered '_canRo' how is it that '_canGuardCounter' or '_canChargeCounter'

see https://hackernoon.com/heres-to-fat-fingers-f39a5c778c4c

So, given the new insights how can the remembering be improved? Suggestions, Ideas?

@SDanton
Copy link
Author

SDanton commented Jan 4, 2018

Thanks for the context. Helpful.

A few ideas:

  1. Case should matter. cons should match const before Constant

  2. Exact match should "win". Entering const and pressing tab/enter should match (and be highlighted / selected) even if there is a constant in the selection list.

  3. Consider breaking filter down into primary and secondary matches. If I enter _canRo then anything with that specific string is my primary match. Matches containing any of the values in _canRo are secondary matches. Further, secondary matches have a min-sub-string length that must be met before matching, perhaps 2 consecutive values? That would mean _canGuardCounter or _canChargeCounter will match while I've entered _can, but drop off as soon as I enter _canR as that specific sub-string cannot be found in either of them.

3b. Reading the hackernoon article... it seems we want to auto-correct for cases in which a dev swaps two letters "accidently" -- not sure what you call this, feels like a form of Spoonerism. But we don't want, although seem to currently have, "deep-matching" of just any singleton values across a string. For instance, we want funciton to get replaced with the function, but just because I have it in my candidate string doesn't mean I match everything with fun and an i or t. This would seem to suggest that we do indeed want a min-length on sub-string matches. Given that rule, we wouldn't see _canGuardCounter or _canChargeCounter as suggestions, right?

  1. Escape should act as a selection candidate. If I enter const and press escape then continue working then we should update the selection system to reco const as if I'd pressed tab/enter.

  2. Escape + backspacing / deleting selection should count against a selection. If I enter const press escape then backspace / delete const or update const to be something like constWidth then my updated string constWidth should be considered my selection as if I'd selected it from the suggestion list.

@chronicgamedev
Copy link

Hello - Interesting discussion. I'm running into this issue in my current project as well where the autocomplete suggestions appeared to insert the wrong item. I realized that the suggestions were "remembering" the previous selection and that's what led me here. I'd argue that similar variable names and "interleaving" them while coding is not rare at all. By making the selection feature smart, it feels like the UX takes control away from the user instead.

For example, I have Client and Server implementations of several objects and as I work back and forth intellisense (subjectively) feels like it's always picking the wrong one. It's definitely slowing me down with extra keystrokes, deletions and interrupted focus.

I would love it if I there were a setting to disable the remembered selection and always default to the best match at the top of the autocomplete list. That would let folks pick the behavior that matches their work style.

@jrieken
Copy link
Member

jrieken commented Jan 5, 2018

@chronicgamedev Thanks for the feedback... Adding a setting is always in reach but also less ideal because folks need to discover and we all need to argue about the default ;-) The client, server sample is good input. Today we remember things per language but maybe we should store it per file. So instead such a key storage://workspace/Users/jrieken/Code/vscode/suggest/memories/typescript we could have storage://workspace/Users/jrieken/Code/vscode/suggest/memories/my/server/file.ts and remember selections for that

@SDanton
Copy link
Author

SDanton commented Jan 5, 2018

Never fun to realise the dev team isn't actually reading your comments... I suggested scoping things in my second comment...

// Can we better leverage context? It feels like we currently remember selections by project...? Could matches be relative to the code file, class or even function?

Good to see you read @chronicgamedev's note, but now I feel like I'm just wasting my time here :|.

@tempit
Copy link

tempit commented Jan 7, 2018

Hi there. Interesting discussion indeed, from both user and extension writer perspectives.
To throw my two bits in, I think the 'remember last selection' feature is overkill, and I agree with @chronicgamedev that the situation where two or more completions apply and I use them interchangeably is more common than you'd think. I'm for cancelling this feature, as it seems to cause more confusion than help.
As for case-sensitivity, I am very strongly for keeping it the way it is. it is most useful when investigating poorly documented APIs (TS compiler TypeChecker, in my case). When trying to see all methods that return a type of some sort, it is very useful to type 'gettype' or even 'gtype' and get both 'getType' and 'getUntypedSymbol' (I made up the function names, but the example is clear, I hope).

@NakOh
Copy link

NakOh commented Jan 8, 2018

Yeah.. I am embarrassed with smart suggestions.

2018-01-08 4 54 37

Maybe this suggestion don't need.

Even if I choose another proposal at least once, there is not enough ground to judge whether the choice will continue to be valid or not.

I want to turn off the ability to remember my choice, but I do not know how.

@jrieken jrieken changed the title Intellisense / Code Completion -- Matching incorrectly Remembered suggestion selections can be annoying Jan 12, 2018
@andy-wood
Copy link

andy-wood commented Jan 13, 2018

I unwittingly opened a duplicate bug, because it's very unclear what the autocomplete behavior actually is. IMO a strong hint that it is "too smart."

FWIW, this issue is costing me ridiculous amounts of productivity, as I constantly have to correct the autocomplete, being careful to step around it even though I'm just typing a word verbatim. I'm running into this every few minutes, so I feel like this can't possibly be the best behavior. Yet I'm also very reluctant to turn off autocomplete entirely, so I'm not sure what the solution is.

It's been stated that VSCode is remembering a past selection. Well, how did all of these erroneous selections get selected to begin with? In my case, I'm seeing extremely common names autocompleted with names I would never, ever have any reason to select on purpose.

It's definitely helpful to learn that I can "fix" it by selecting the correct name from the dropdown. But I'm encountering this issue all day, every day, to the point that it's becoming one of my single biggest editing time-sinks.

I think for now my best option is to turn off editor.acceptSuggestionOnCommitCharacter.

@tomrav
Copy link

tomrav commented Jan 15, 2018

I'm encountering similar problems with completions, for me it's manifesting itself mostly while writing tests.

screen shot 2018-01-15 at 11 51 09

That very sticky suggested completion of .toString has really driven me crazy over the last couple of days.

Remembering my last choice feels counter-intuitive to me, and I would much prefer simply getting the first result of the suggested list highlighted (benefitting from the already weighted and sorted list of completions).

@jrieken
Copy link
Member

jrieken commented Jan 15, 2018

Trying to improve this and the plan is to limit the memory to the top bucket. In the scenario below there are two equal matches (both score 40 as the suggest-debug-mode shows) and we would remember if Actor has been expanded to ActorMgr or ActorRegistry. Different than today, expanding it to actor or one of the other suggestion wouldn't be stored, only selection within the 'top bucket'.

screen shot 2018-01-15 at 13 54 27

Now in the scenario below there is only one top suggestion and we will always select that.

screen shot 2018-01-15 at 13 54 37

In addition to that we can also limit storage further, e.g instead of scoping to the language, scope to the file (we unfortunately don't have project information at this level) and we can also turn this off via a setting. Still, I'd like to make those changes and see how/if that removes annoyances first. Then we take the bigger hammer.

@jrieken
Copy link
Member

jrieken commented Jan 15, 2018

I have pushed b4ee65d which implements the above mentioned strategy. Limiting the suggestion memory to the top bucket will make suggest behave like so.

Please try this out with the next insiders release. Thanks for helping!

jan-15-2018 14-42-12

@tempit
Copy link

tempit commented Jan 16, 2018

@jrieken - How do I turn on suggest-debug-mode?
Couldn't find it in settings or in vscode repo.
As an extension developer, this is a very useful feature to have.

@jrieken
Copy link
Member

jrieken commented Jan 16, 2018

How do I turn on suggest-debug-mode?

For now only in source by uncommenting this line. Tho I have a todo to expose that in some way

@EmilyChews
Copy link

Sorry, can I ask - is there a way to turn this feature off. When you set up your own code snippets this feature is infuriating, in the sense that unless you call your snippet nothing to do with what it does like "ttt" if defaults to other suggestions. See this thread on StackOverflow https://stackoverflow.com/questions/48304160/over-ride-default-snippet-prefix-in-vs-code/48305768#48305768

@jrieken
Copy link
Member

jrieken commented Jan 19, 2018

@emilyChewsCheese Please try this with latest insiders: https://code.visualstudio.com/insiders/. The problem that you are describing has been fixed (as described here #41060 (comment))

@jrieken
Copy link
Member

jrieken commented Jan 23, 2018

@squalsoft Not sure that you are on the right issue. This issue is about selecting a completion which isn't the top completion in the list. You screen shot shows that the first/top completion is selected.

@jrieken
Copy link
Member

jrieken commented Jan 24, 2018

You stated above that new option is not necessary, which I don't agree

I'd say that a miss-understanding of what I said. What I wanted to get across is that I first want to improve how the feature works (hard) and then add a setting (easy). Ideally, a feature works so well that it doesn't need a switch to be turned off.

There are many people, like you and I, that deeply care about this. I see that some people are upset by the latest changes and I want to understand why. Yes, plain dislike is one kind of feedback but little actionable.

@shraddha-kapoor, @tempit In the hunt for actionable feedback: Did you try this with recent Insiders which has been improved in this respect? Did you never encounter a situation in which you have typed cons, got console, const suggested and had to repeatedly select const( because console was selected)? What do you do in such situations? Do you use the arrow keys to select something? Do you continue to type until you have what you want, e.g typing t in the sample above?

See, I wanna understand how you use IntelliSense and how/when the feature gets into the way. It's great that you care and I like understand and learn how you do it and what you are wishing for.

@tempit
Copy link

tempit commented Jan 24, 2018

@jrieken - Did not mean to offend; my original feedback is in one of the earlier comments.

I have encountered this issue since, when trying to refactor a bit of code, and either calling new variables 'newVar1', 'newVar2', 'newVar3' etc. or renaming old vars to 'oldVar1', etc. (names made up for clarity)
This means the auto-selected completion was rarely the correct one, and it was cumbersome to use.

I have not tried it in the insiders version, since from what I understood about the current improvement it would not fix this issue. I also have encountered this issue on the extension development side, where my suggested completions have the same score, and so would not be affected by the fix, if I got it correctly.

@usernamehw
Copy link
Contributor

usernamehw commented Jan 26, 2018

@jrieken Just wondering: can the "smart suggestions" just go up in suggest widget? I kinda like the idea, but having to visually search for selected item is not so great...

con
// before
--------------
confirm
--------------
console
--------------
const         selected (remembered)
--------------

// after
--------------
const         selected (remembered)
--------------
confirm
--------------
console
--------------

Also, having an indicator why this item is selected(or above) would be nice.

@jrieken
Copy link
Member

jrieken commented Jan 26, 2018

@usernamehw Yeah, we did discuss that. We have been throwing ideas around which are similar, remembered suggestions could show above 'normal' suggestion or just below. I didn't go deeper on that because I don't wanna confuse people further as the sorting suddenly becomes hard to understand (and you needing to read/understand those history items). What I have pushed now is changes to make this less confusing. However, now that we have a setting we can explore different idea too.

@chronicgamedev
Copy link

the solution sounds pretty good, thanks for considering all the feedback.

@jrieken jrieken added feature-request Request for new features or functionality and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jan 29, 2018
@jrieken
Copy link
Member

jrieken commented Jan 29, 2018

Closing as we now have a setting and because the default has been made less annoying. Added the feature-label to make sure we test this during this weeks endgame.

@ghost
Copy link

ghost commented Jan 30, 2018

Guys please test editor.selectSuggestions with never, byRecency, and byPrefix to help @jrieken to decide which one is need to be by default. Currently default is byRecency, but I tested it and I can say that byPrefix is by far the best.

@tempit
Copy link

tempit commented Jan 30, 2018

After playing with it briefly in insiders I can confirm the different options work as described (for me).

The option I will be using once this make it to the main version is 'never', since in my workflow when I have 'newVar', 'newClass', 'newType' etc. I don't want vscode remembering which one I used, as that constantly changes.

I do think the names of both the setting and the values are confusing now, I had to refer back here to understand which is which.
I would have the setting name be 'editor.rememberAcceptedSuggestions' and the values be 'never' (current 'never'), 'always' (your 'byPrefix'), and 'weak' or similar (your 'byRecency').

Thanks for the responsiveness and all the work!

@jrieken
Copy link
Member

jrieken commented Jan 30, 2018

I would have the setting name be 'editor.rememberAcceptedSuggestions' and the values be 'never' (current 'never'), 'always' (your 'byPrefix'), and 'weak' or similar (your 'byRecency').

Thanks. The names aren't set in stone (yet) and I'd like to hear other opinions about this too

@tomrav
Copy link

tomrav commented Jan 31, 2018

Been using the insiders build for the last couple of days - seems that the new default of byRecency has solved the issue for me. Thanks!

In addition, I agree with @tempit that the current names are confusing, the difference between byPrefix and byRecency isn't entirely clear, even after reading the description text.

@jrieken
Copy link
Member

jrieken commented Feb 1, 2018

We have changed the names of the settings and its values. See #42477 (comment)

@user3323
Copy link

user3323 commented Feb 8, 2018

@jrieken, hello! And still nothing about #32201 ?

@peabnuts123
Copy link

peabnuts123 commented Feb 8, 2018

For people coming here looking for what the settings first, recentlyUsed and recentlyUsedByPrefix mean, here is @jrieken's comment but with the old development names replaced with the new names:

Samples of each setting value with and without a prefix:

first, no prefix

Select the first item. The first is the first because of lexicographical sorting.

first, no prefix

first, prefix

Select the first item. The first is the first because of smart sorting/ranking based on the prefix.

first, prefix

recentlyUsedByPrefix, no prefix

That's the same as 'first, no prefix' as the prefix is used as cache key. (We could allow the empty string as cache key).

recentlyUsedByPrefix, prefix

Select items based on prefixes and items selected in the past, e.g once groupEnd as been selected for the e prefix.

recentlyUsedByPrefix, prefix

recentlyUsed, no prefix

Selects the last selected (and available) suggestion, e.g. in this case exception, but only when there is no other hint.

recentlyUsed, no prefix

recentlyUsed, prefix

That's the same as 'first, prefix' because the memory is considered a weaker source than the prefix you have just typed.

Awesome to see this be addressed and actually resolved with a pile of thought and community involvement 👍

EDIT: Oh I see in the commit there are actually descriptions tagged against the enum values… how does one view these? 🤔

@egamma
Copy link
Member

egamma commented Feb 16, 2018

@peabnuts123

Oh I see in the commit there are actually descriptions tagged against the enum values… how does one view these?

They are currently only visible on the right side of the settings editor
image

@DevoidCoding
Copy link

When I used editor.suggestSelection set to recentlyUsedByPrefix, it workds like a charm for snippets.
But when I set it to recentlyUsed, it doesn't remember when I select a snippet (not the insider version), is it the desired behavior ?

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality on-testplan release-notes Release notes issues suggest IntelliSense, Auto Complete
Projects
None yet
Development

No branches or pull requests