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

fix(47562): Add option to suppress type hint if variable name matches type name #48529

Merged
merged 5 commits into from
May 25, 2022

Conversation

huytd
Copy link
Contributor

@huytd huytd commented Apr 2, 2022

Fixes #47562

The approach I'm proposing here is to add a new option called includeInlayVariableTypeHintsWhenTypeMatchesName for the UserPreferences.

When the user turns off this option, we will check if the variable name matches the type display string and suppress the hint for this case.

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Apr 2, 2022
@sandersn sandersn added this to Not started in PR Backlog Apr 21, 2022
@sandersn sandersn requested a review from gabritto April 22, 2022 15:44
@sandersn sandersn moved this from Not started to Waiting on reviewers in PR Backlog Apr 22, 2022
PR Backlog automation moved this from Waiting on reviewers to Needs merge Apr 22, 2022
@gabritto
Copy link
Member

@andrewbranch we'd need to add the new option to vscode too, right?

@andrewbranch
Copy link
Member

Yep.

src/services/inlayHints.ts Outdated Show resolved Hide resolved
src/services/inlayHints.ts Outdated Show resolved Hide resolved
PR Backlog automation moved this from Needs merge to Waiting on author Apr 22, 2022
@huytd
Copy link
Contributor Author

huytd commented Apr 25, 2022

Thank you so much for the feedbacks @andrewbranch @gabritto I'll update the PR to fix the issues you mentioned 👍

@andrewbranch
Copy link
Member

Thanks @huytd! We’re going to discuss this in our meeting with VS Code next week. Hang tight until then 👍

@andrewbranch
Copy link
Member

We discussed this today, but were missing a few key people on vacation. I think the issue probably needed a little more design specification before asking for a PR—we discussed the possibility of addressing the noisiness of inlay hints with fuzzy string matching a bit more holistically rather than adding a new setting for each one we eliminate. We’re going to bring it up again next week when we have the full group back.

@huytd
Copy link
Contributor Author

huytd commented Apr 27, 2022

Thank you so much for keeping me updated! @andrewbranch

PR Backlog automation moved this from Waiting on author to Needs merge May 25, 2022
@andrewbranch
Copy link
Member

Thanks for your patience @huytd!

@mjbvz, the user preference is already in here.

@andrewbranch andrewbranch merged commit 1fb2b2d into microsoft:main May 25, 2022
PR Backlog automation moved this from Needs merge to Done May 25, 2022
mjbvz added a commit to mjbvz/vscode that referenced this pull request May 26, 2022
From microsoft/TypeScript#48529

Let users control is variable type inlay hints are suppresed if the variable name matches the type name, such as:

```ts
const range = new Range();
```
mjbvz added a commit to microsoft/vscode that referenced this pull request May 26, 2022
…#150489)

From microsoft/TypeScript#48529

Let users control is variable type inlay hints are suppresed if the variable name matches the type name, such as:

```ts
const range = new Range();
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Inlay hints suppression should be case insensitive and look within variable names
4 participants