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

Recommend relationship between error ranges and selections for quickfixes #1897

Open
mheiber opened this issue Feb 8, 2024 · 1 comment
Open

Comments

@mheiber
Copy link

mheiber commented Feb 8, 2024

Given an error range and a selection range, when should we show the error? There are some choices:

A: overlap and selection is multi-character

------- error
      ----------- selection

B: error contains selection and selection is multi-character

--------------error
      ------selection

C

      ------errror
   --------------selection    

D

       ------errror
                 ----------selection       

E: error overlaps with selection (inclusive) and selection is single-character

       ------errror
               ^ selection       

F: error overlaps with selection (inclusive) and selection is single-character

       ------errror
                 ^ selection       

It would be nice if languages were consistent, so at least a recommendation would be most welcome!

@mheiber
Copy link
Author

mheiber commented Feb 8, 2024

If #1897 is implemented, then ideally the same recommendation should apply there.

facebook-github-bot pushed a commit to facebook/hhvm that referenced this issue Feb 20, 2024
Summary:
Here we're a bit more restrictive about when we show quickfixes:

https://pxl.cl/4kgXh

Before this change, we also showed quickfixes when the selection contained the error span:

https://pxl.cl/4kh51

With this change, we remove cases where we would show a quickfix but neither TypeScript nor Rust Analyzer would.

## Future Work

VSCode language experiences are not consistent in how they decide whether to show a quickfix: I saw different behavior in TypeScript and Rust Analyzer, for example. We're asking for guidance on this LSP issue: microsoft/language-server-protocol#1897

Reviewed By: ljw1004

Differential Revision: D53568626

fbshipit-source-id: fe3ba85c19bd96fd491c60f362e8b0a17a1f2199
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant