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

hls-cabal-plugin: refactor context search to use readFields #4186

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Apr 21, 2024

Replace the more brittle parser with cabal's readFields.

Ready for review :)

@fendor fendor requested a review from pepeiborra as a code owner April 21, 2024 12:19
@fendor fendor marked this pull request as draft April 21, 2024 12:19
@fendor fendor changed the title WIP: Refactor context search to use readFields hls-cabal-plugin: refactor context search to use readFields Apr 21, 2024
@fendor fendor force-pushed the enhance/cabal-readfields branch from 46ad7dd to b5587a8 Compare May 7, 2024 16:06
@fendor fendor marked this pull request as ready for review May 27, 2024 14:10
@fendor fendor requested a review from wz1000 as a code owner May 27, 2024 14:10
@fendor fendor force-pushed the enhance/cabal-readfields branch 3 times, most recently from ad60fbb to 818e6ee Compare May 27, 2024 16:56
@fendor fendor requested a review from VeryMilkyJoe May 27, 2024 17:00
@fendor fendor force-pushed the enhance/cabal-readfields branch 5 times, most recently from c5711f4 to 6d9778e Compare May 27, 2024 21:03
Copy link
Collaborator

@soulomoon soulomoon left a comment

Choose a reason for hiding this comment

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

look good to me, very interesting idea!

@soulomoon
Copy link
Collaborator

Why is the test not running

@fendor
Copy link
Collaborator Author

fendor commented May 28, 2024

Hm, I am also confused. Perhaps all my force pushing messed up the CI :)

@soulomoon
Copy link
Collaborator

Will it help with #3333

@fendor
Copy link
Collaborator Author

fendor commented May 28, 2024

No, this tries to fix some bugs in the cabal file completions that I encountered while dogfooding the plugin. This does not affect the tests at all.

The tests are flaky, for the same reason hlint tests are/were flaky, afaict.

fendor and others added 2 commits May 28, 2024 15:54
Add utils that allows to define parameterised tests for files that
require cursor positions.
This enables us to define run the same tests for multiple inputs
efficiently, and with readable error messages.

The main advantage is the improved specification of the test cases, as
we allow to specify the cursor position directly in the source of the
test files.
Instead of custom parsing of the cabal file, we use `readFields` to parse
the cabal file, as accurately as cabal supports. This allows us to
additionally benefit from future improvements to the cabal lexer.

Then, we traverse the fields and find the most likely location of the
cursor in the cabal file.
Based on these results, we can then establish the context accurately.

Further, we extend the known rules for the cabal plugin, to avoid
expensive reparsing using `readFields`.

Co-authored-by: VeryMilkyJoe <jana.chadt@nets.at>
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.

None yet

2 participants