Skip to content

fix: show all enums on hover#942

Merged
msivasubramaniaan merged 1 commit intoredhat-developer:mainfrom
twelvelabs:enums-on-hover
Dec 19, 2023
Merged

fix: show all enums on hover#942
msivasubramaniaan merged 1 commit intoredhat-developer:mainfrom
twelvelabs:enums-on-hover

Conversation

@twelvelabs
Copy link
Contributor

What does this PR do?

This PR updates the hover service to display all possible enum values, rather than just the currently selected one. Additionally, it will display the enums regardless of whether the schema contains explicit descriptions (though it will continue to display the descriptions if present - see included test case).

Screen Shot 2023-12-03 at 3 56 05 PM

Also:

  • Included a minor change to how examples are displayed (i.e. added markdown bullets) to make them visually consistent with the enums.
  • Added a ensureLineBreak helper to make sure there is consistent spacing between each of the various hover sections.

What issues does this PR fix or reference?

Fixes #941

Is it tested? How?

@twelvelabs
Copy link
Contributor Author

cc @gorkem and @msivasubramaniaan for visibility.

@coveralls
Copy link

Coverage Status

coverage: 84.071% (+0.3%) from 83.813%
when pulling f73cc82 on twelvelabs:enums-on-hover
into ed03cbf on redhat-developer:main.

Copy link
Contributor

@msivasubramaniaan msivasubramaniaan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @twelvelabs for your contribution

@msivasubramaniaan msivasubramaniaan merged commit d354c58 into redhat-developer:main Dec 19, 2023
@behemothsecurity
Copy link

behemothsecurity commented Jun 19, 2024

@msivasubramaniaan @twelvelabs This feature really broke our workflow, we use arrays where items are defined with a huge list of enums (and markdownEnumDescriptions), and now hover will not work as it tries to render the full list of enum instead of trying to show the markdownEnumDescription for the enum entry being hovered on first. I also think this feature has limited purpose as the list is available with autocompletion. I suggest to put this change behind a setting which can be disabled.

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.

Show enum values on hover

4 participants