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.

Sign up for GitHub

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

Jump to bottom

Fix incorrect syntax in hover help for module #709

Merged
merged 2 commits into from
Jan 22, 2023
Merged

Fix incorrect syntax in hover help for module #709

merged 2 commits into from
Jan 22, 2023

Conversation

cknitt
Copy link
Member

@cknitt cknitt commented Jan 21, 2023

Fixes #708

@cknitt cknitt requested a review from zth January 21, 2023 16:59
zth
zth reviewed Jan 21, 2023
View reviewed changes
Copy link
Collaborator

@zth zth left a comment

Choose a reason for hiding this comment

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

Awesome, looking great! Could we add separate tests that details this exact behavior? I don't see module type printed in the tests for example.

@cknitt
Copy link
Member Author

cknitt commented Jan 21, 2023

module type already occurs in Hover.res:

rescript-vscode/analysis/tests/src/Hover.res

Line 50 in 25caa49

module type Logger = {

The output for module type is unchanged, as it was correct before.
So both cases are covered and I'm not sure if we actually need more tests.

@zth zth self-requested a review January 21, 2023 18:13
@zth
Copy link
Collaborator

zth commented Jan 21, 2023

Changelog would be good, other than that good to go!

@zth zth merged commit 97ef1fa into rescript-lang:master Jan 22, 2023
@cknitt cknitt deleted the fix-module-syntax branch January 22, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zth zth zth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect syntax in hover help for module
2 participants
@cknitt @zth