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

Add inlay hint for omitted hash values #1186

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

RyanBrushett
Copy link
Contributor

@RyanBrushett RyanBrushett commented Nov 14, 2023

Motivation

In this discussion, @andyw8 mentions how we could add an inlay hint for an omitted hash value to be a bit clearer about what's going on.

Implementation

  • Followed how the inlay hint for implied rescue StandardError works
  • I added an example but it's kind of hard to write out how it works... let me know if I could make it better
  • I think there's only three Node types that node.value could be when the node is an ImplicitNode but let me know if I'm missing anything

Automated Tests

I added some expectation tests!

Manual Tests

You could test this by opening the ruby-lsp project in VS code, restart the extension so it loads the development version, and then write some code with implied hash values. This should do it:

some_variable = "hello"
{some_variable:} # hint will be added here

Also

❤️

@RyanBrushett RyanBrushett requested a review from a team as a code owner November 14, 2023 00:27
@RyanBrushett RyanBrushett force-pushed the ryanb-omitted-hash-value-hints branch from d0c42bb to 964c42f Compare November 14, 2023 08:33
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

lib/ruby_lsp/requests/inlay_hints.rb Outdated Show resolved Hide resolved
RyanBrushett and others added 2 commits November 21, 2023 12:09
- Add test using hash with slightly more complex formatting
- Cleanup

Co-authored-by: Alexandre Terrasa <[email protected]>
Co-authored-by: Vinicius Stock <[email protected]>
@RyanBrushett RyanBrushett force-pushed the ryanb-omitted-hash-value-hints branch from ffc1277 to 3d44258 Compare November 21, 2023 17:11
@RyanBrushett RyanBrushett requested a review from Morriar November 21, 2023 17:12
Copy link
Contributor

@Morriar Morriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@vinistock vinistock merged commit 549d404 into Shopify:main Nov 22, 2023
16 checks passed
@davidalejandroaguilar
Copy link

davidalejandroaguilar commented Nov 22, 2023

Thanks for this PR! However, how can we disable this? The reason I omit the value is because I don't want it to be shown 😅

(I'm guessing this PR is the one that added this):

image image

EDIT: I just realized you can disable inlayHint, however, that might disable ALL inlay hints, which is not desired. Only this particular hint (the one that shows omitted hash/keyword argument values) should be disabled.

@RyanBrushett RyanBrushett deleted the ryanb-omitted-hash-value-hints branch November 22, 2023 23:36
@vinistock
Copy link
Member

There's currently no way to configure inlay hints with that level of granularity. We still need to accept a more complex configuration for it.

@Petercopter
Copy link

Please add a configuration option, this is infuriating and unwanted. The whole point was to make code more clean and concise, it's frustrating that it was undone with this change.

@tonybruess
Copy link

Can we make a new issue for a configuration option here?

@vinistock
Copy link
Member

It's already being worked on as part of #834, which is related.

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

Successfully merging this pull request may close these issues.

6 participants