- Notifications
You must be signed in to change notification settings - Fork471
Show doc comments before type expansions in hover information#7774
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Show doc comments before type expansions in hover information#7774
Uh oh!
There was an error while loading.Please reload this page.
Conversation
1dbdc67 to6a89573Comparerescript@rescript/darwin-arm64@rescript/darwin-x64@rescript/linux-arm64@rescript/linux-x64@rescript/win32-x64commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR improves hover information display by reordering the presentation to show docstrings before type expansions, making documentation more visible when type expansions are lengthy.
- Reordered hover information to display docstrings immediately after the main type, before expanded types
- Modified the
hoverWithExpandedTypesfunction to accept and integrate docstring parameters - Updated test expectations to reflect the new hover information order
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| analysis/src/Hover.ml | Core implementation changes to reorder hover elements and integrate docstring display |
| tests/analysis_tests/tests/src/expected/Hover.res.txt | Updated test expectations for the new hover format |
| tests/analysis_tests/tests/src/expected/JsxV4.res.txt | Updated test expectations for JSX component hover format |
| CHANGELOG.md | Added changelog entry documenting the feature |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
| Markdown.codeBlock typeString :: expandedTypes|>String.concat"\n" | ||
| let typeString= | ||
| match docstringwith | ||
| |Some[]|None ->Markdown.codeBlock typeString |
CopilotAIAug 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] The patternSome [] suggests that an empty docstring list should be treated the same as no docstring. Consider using a helper function or guard clause to make this logic clearer, as the pattern matching combines two conceptually different cases.
| |Some[]|None ->Markdown.codeBlock typeString | |
| letis_empty_docstring=function | |
| |None|Some[] ->true | |
| |Some_ ->false | |
| in | |
| match docstringwith | |
| |dwhen is_empty_docstring d ->Markdown.codeBlock typeString |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Great stuff!
dfa1610 intorescript-lang:masterUh oh!
There was an error while loading.Please reload this page.
When generating hover information, we currently show in order:
A frequent issue I run into is that my expanded types hover information is long enough that the hover preview box must be scrolled to view docstrings.
In this PR, I've changed the order we present this information to: