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

Order of suggestions in Quick Fix #297

Closed
sub-mersion opened this issue Apr 12, 2021 · 10 comments
Closed

Order of suggestions in Quick Fix #297

sub-mersion opened this issue Apr 12, 2021 · 10 comments
Assignees
Labels
1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature 3-fixed Issue resolution: Issue has been fixed on the develop branch
Milestone

Comments

@sub-mersion
Copy link

sub-mersion commented Apr 12, 2021

When correcting a word with Quick Fix on say the mistyped 'helol' word, the order of suggestions is

  • Add 'helol' to the dictionary
  • Hide false positive
  • Disable rule
  • Use 'hello'

Most of the time, one needs to correct the word rather that mark it as an exception.

I suggest the default order for Quick Fix to be reversed, as in

  • Use 'hello'
  • Add 'helol' to the dictionary
  • Hide false positive
  • Disable rule

(but maybe it's vscode who enforces quick fixes in alphabetical order?)

@sub-mersion sub-mersion added the 1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature label Apr 12, 2021
@valentjn
Copy link
Owner

I think this is a duplicate of #129.

@sub-mersion
Copy link
Author

It is similar and I am indeed on macOS where do not wrap to the end of the context menu.

However, my point is that correcting an obvious mistyped word requires at least one more keystroke after the quick fix trigger — and at least 3 (!) on macOS.
On the other hand adding the mistyped word to the dictionary is the default option and requires none.
It does not feel right that allowing the user to repeat his error afterwards is the default selected action.

Besides, even for Linux / Windows, the "solution" does not work as soon as there are more than one suggestion in the context menu, since they are sorted by score from top to bottom.

Given that word corrections are so common, I think the quick fix menu should present it first.
It would follow the same logic as sorting the suggestions by score: it would sort the suggested actions by frequency.

@valentjn
Copy link
Owner

From a UX perspective, you're totally right. But the problem with the example in #129 (comment) (screenshot) remains. I wouldn't expect other entries like Add ... to dictionary after all those Use '...' entries, especially if the Add ... to dictionary is not visible at all. Switching the order depending on the number of entries is probably confusing.

IMO, the ideal solution would be to have nested menus/quick fixes, but this would be an issue for VS Code or LSP.

@valentjn
Copy link
Owner

I just found out you can assign keyboard shortcuts to specific "kinds" of quick fixes. The kind for Use '...' is quickfix.ltex.acceptSuggestions, so how about adding something like this to your keybindings.json:

  {
    "key": "cmd+shift+q",
    "command": "editor.action.codeAction",
    "args": {
      "kind": "quickfix.ltex.acceptSuggestions"
    }
  }

Then you can press Cmd+Shift+Q at an underlined word (without having to press Cmd+. first). If there is only one suggestion, it will use it without any further keystrokes. If there are multiple suggestions, a context menu will open that only contains the Use '...' quick fixes.

@sub-mersion
Copy link
Author

That's neat, it's an improvement 🙂

Except that cmd+shift+q is the default shortcut to close the session on macOS.
I bind it to cmd+;, setting is here for future users

 {
    "key": "cmd+;",
    "command": "editor.action.codeAction",
    "args": {
      "kind": "quickfix.ltex.acceptSuggestions"
    }
 }

@sub-mersion
Copy link
Author

I wouldn't expect other entries like Add ... to dictionary after all those Use '...' entries, especially if the Add ... to dictionary is not visible at all.
Switching the order depending on the number of entries is probably confusing.

I understand your point and #129 is indeed a pathological example.

In place of context menus, maybe a viable alternative is to put the Add ... to dictionary and similar entries not at the bottom of the list, but after say the first n top suggestions, with n ~ 4 — if only it is allowed by vscode extension features.

For example It is doam would give

- Use 'dam'
- Use 'foam'
- Use 'doom'
- Use 'dorm'
- Add 'doam' to dictionary
- Hide false positive
- Disable rule
- Use 'roam'
- Use 'loam' 
- ...

While the above keybinding works perfectly for me, I still think the out of the box config should suggest corrections before exceptions.

@valentjn
Copy link
Owner

I've come to the conclusion that having more than 5 or so suggestions is already bad UX: If the correct word is not in the top 5, then just fixing the mistake by hand is probably faster than searching for the correct word in a long list. So I'll just truncate the list to this length and reorder the quick fixes.

However, while experimenting, I noticed that VS Code does reorder the quick fixes, at least when computing them for a selection range (could also be the case for the other editors LTEX is supporting). This means that while I'm trying to fix this, I won't guarantee any order. If it's still wrong then, you have to open an issue over at microsoft/vscode.

@valentjn valentjn self-assigned this Apr 22, 2021
@valentjn valentjn added this to the 10.2.0 milestone Apr 22, 2021
valentjn added a commit to valentjn/ltex-ls that referenced this issue Apr 22, 2021
@valentjn
Copy link
Owner

Implemented as explained in my previous comment. Thanks for the suggestion.

@valentjn valentjn added the 3-fixed Issue resolution: Issue has been fixed on the develop branch label Apr 22, 2021
@sub-mersion
Copy link
Author

Thank you for having rethink this, I think it is the correct fix.
And thank you for this extension in general, keep the nice work! 😄

@valentjn
Copy link
Owner

valentjn commented May 1, 2021

Feature released in 10.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature 3-fixed Issue resolution: Issue has been fixed on the develop branch
Projects
None yet
Development

No branches or pull requests

2 participants