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

add doc and ci test for semantic tokens #3938

Merged
merged 2 commits into from
Jan 10, 2024
Merged

add doc and ci test for semantic tokens #3938

merged 2 commits into from
Jan 10, 2024

Conversation

soulomoon
Copy link
Collaborator

No description provided.

@soulomoon soulomoon marked this pull request as ready for review January 9, 2024 19:36
@soulomoon soulomoon mentioned this pull request Jan 9, 2024
Open
25 tasks
@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 9, 2024 •
edited
Loading 

    datatype family: FAIL
      test/testdata/TDataFamily.hs: copyFile:atomicCopyFileContents:withReplacementFile:copyFileToHandle:withBinaryFile: does not exist (No such file or directory)
      Use -p '/datatype family/' to rerun this test only.

Golden test seems fragile here.
look like a cabal problem?

@michaelpj
Copy link
Collaborator

I don't think that's what's happening here? Where do those files get copied?

@soulomoon

This comment was marked as resolved.

Sign in to view
@hololeap
Copy link
Contributor

hololeap commented Jan 9, 2024 •
edited
Loading

@soulomoon

I did not, to my knowledge, but right now the tests are disabled for a different reason:

RESTRICT="test" # Depends on masked ghcide-test-utils
# hololeap <hololeap@protonmail.com> (2023-10-15)
# This package currently depends on an outdated version of
# lsp-test.
dev-haskell/ghcide-test-utils

Since the packages are installed to the system, we can only have one version of each package installed, and since ghcide-test-utils depends on an old version of lsp-test, the package was disabled.

@soulomoon soulomoon enabled auto-merge (squash) January 9, 2024 23:21
@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 9, 2024 •
edited
Loading

@michaelpj
you are right, it turns out to be just a case sensitivity issue. Fixed and ready to merge.

@soulomoon soulomoon added the merge me Label to trigger pull request merge label Jan 10, 2024
fendor
fendor approved these changes Jan 10, 2024
View reviewed changes
Copy link
Collaborator

@fendor fendor 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 for following up on the tests :)

docs/features.md
@@ -389,7 +398,8 @@ Code action kind: `refactor.rewrite`
Rewrites record selectors to use overloaded dot syntax

![Explicit Wildcard Demo](../plugins/hls-overloaded-record-dot-plugin/example.gif)
## Missing features

### Missing features
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should stay ##, it isn't a subsection of the plugin above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah agree

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sorry for the mistake, I'll fix it in #3940

@soulomoon soulomoon merged commit b645a99 into haskell:master Jan 10, 2024
36 checks passed
michaelpj
michaelpj reviewed Jan 10, 2024
View reviewed changes
docs/features.md

Provides semantic tokens for each token to support semantic highlighting.

![Semantic Tokens Demo](https://private-user-images.githubusercontent.com/14073857/290981908-9619fae2-cb92-4d4e-b8f8-6507851ba9f3.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDQ4MjgwODYsIm5iZiI6MTcwNDgyNzc4NiwicGF0aCI6Ii8xNDA3Mzg1Ny8yOTA5ODE5MDgtOTYxOWZhZTItY2I5Mi00ZDRlLWI4ZjgtNjUwNzg1MWJhOWYzLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAxMDklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMTA5VDE5MTYyNlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTBjOTUxNTM0ZDcyNmFmZjEyN2JlNzkwNWNjZjA4NTAzNDVkMzdlNmMxNDNiMzgxNGMzMTQ1NDRiMzUxZjM5OWQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.n-CL6e2R0TWHpmzVo1i60QEDczTEJ-8zvQWxjaBsnks)
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, for this sort of thing it might be better if we checked in the gif, like we have for the others?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

semantic tokens feature is static, which means no action involed, I guess GIF is for some thing with an action?

plugins/hls-semantic-tokens-plugin/test/testdata/TDataFamily.hs
@@ -0,0 +1,11 @@
{-# LANGUAGE TypeFamilies #-}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to also delete the old files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I have delete them in the local, oddly I do have the old files in my local branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll fix it in #3940

@soulomoon soulomoon mentioned this pull request Jan 10, 2024
Merged
4 tasks
@chenrui333 chenrui333 mentioned this pull request Jan 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelpj michaelpj michaelpj left review comments

@fendor fendor fendor approved these changes

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@soulomoon @michaelpj @hololeap @fendor