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

Update hls-retrie-plugin to be usable with 9.2.4. #3120

Merged
merged 11 commits into from
Sep 7, 2022

Conversation

drsooch
Copy link
Collaborator

@drsooch drsooch commented Aug 23, 2022

This is the first pass at getting hls-retrie-plugin enabled. Much of the
changes were updating to match the changes in the upstream retrie
package.

@drsooch
Copy link
Collaborator Author

drsooch commented Aug 23, 2022

@pepeiborra - Here's a first pass. Functionality seems to be the same. I don't see any major differences that would break anything.

Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

Thank you so much for helping with this!

This is the first pass at getting hls-retrie-plugin enabled. Much of the
changes were updating to match the changes in the upstream `retrie`
package.
@drsooch
Copy link
Collaborator Author

drsooch commented Aug 24, 2022

Is the intended operation for getParsedModule to fire multiple times? It looks like when a file contains CPP it will continuously run getParsedModule, until for whatever reason it will stop and finally create the command.

Here's a sample output for running unfold in the current file on qualify in Retrie.hs: https://gist.github.com/drsooch/f63e9a45e6c17a4229c7e85bd2b79746

The PARSED and other lines, are just me trying to trace the logic. In the gist, you can see my memory usage explodes to ~10GB.

I compared with current master branch, and it appears this continuous parsing is a by-product of the file itself. So it doesn't look like this is a new issues.

@drsooch drsooch mentioned this pull request Aug 25, 2022
8 tasks
@pepeiborra
Copy link
Collaborator

Is the intended operation for getParsedModule to fire multiple times? It looks like when a file contains CPP it will continuously run getParsedModule, until for whatever reason it will stop and finally create the command.

Here's a sample output for running unfold in the current file on qualify in Retrie.hs: https://gist.github.com/drsooch/f63e9a45e6c17a4229c7e85bd2b79746

The PARSED and other lines, are just me trying to trace the logic. In the gist, you can see my memory usage explodes to ~10GB.

I compared with current master branch, and it appears this continuous parsing is a by-product of the file itself. So it doesn't look like this is a new issues.

Yes, this is how retrie deals with CPP files. The memory usage is unfortunate, maybe there's room for improvement there

@drsooch drsooch marked this pull request as ready for review September 1, 2022 00:05
Copy link
Collaborator

@kokobd kokobd left a comment

Choose a reason for hiding this comment

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

hls-code-range-plugin looks good to me.

@drsooch
Copy link
Collaborator Author

drsooch commented Sep 1, 2022

Looks like there's some files that snuck through to master without formatting. Those are cleaned up here.

@drsooch drsooch added the merge me Label to trigger pull request merge label Sep 7, 2022
@mergify mergify bot merged commit aad896c into haskell:master Sep 7, 2022
sloorush pushed a commit to sloorush/haskell-language-server that referenced this pull request Sep 12, 2022
* Update hls-retrie-plugin to be usable with 9.2.4.

This is the first pass at getting hls-retrie-plugin enabled. Much of the
changes were updating to match the changes in the upstream `retrie`
package.

* Replace GHC.Paths.libdir by querying a ModSummary for the LibDir

* Looks like formatting was missed

* Revert "Looks like formatting was missed"

This reverts commit 4f6eee5.

* Don't build retrie for 9.4

Co-authored-by: Pepe Iborra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants