Skip to content

Cabal autogen-modules and autogen-includes completion #4534

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

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

Conversation

rm41339
Copy link

@rm41339 rm41339 commented Apr 1, 2025

Added support for the autogen-modules: field in .cabal file completions inside library, executable, test-suite, and benchmark stanzas and autogen-includes in all relevant stanza types via libExecTestBenchCommons.

Behavior was tested manually in a cabal file.

Closes #4526

@VeryMilkyJoe
Copy link
Collaborator

Thank you for the PR!
You will need to rebase your changes on top of the current master branch.
If you have trouble with the rebasing, it might be easier to reset to current master and then cherry pick your commit afterwards.
You will also need to add some tests before we can merge this, you can take some inspiration from #4535 if you want, but please do not copy paste.

@fendor fendor requested a review from VeryMilkyJoe April 2, 2025 10:19
@rm41339 rm41339 force-pushed the cabal-autogen-completion branch from 71c918b to 3dd647a Compare April 2, 2025 15:32
@rm41339
Copy link
Author

rm41339 commented Apr 2, 2025

Hi, I updated the changes to reflect on top of the current master branch and added tests as well. Please let me know if there's anything else you'd like me to change!

the tests can be run with: cabal test hls-cabal-plugin-tests --test-options="--pattern autogen"

@rm41339
Copy link
Author

rm41339 commented Apr 2, 2025

Hello, I've made the requested changes.

I apologize for some of the unnecessary edits, I think when I was updating to the latest version some changes got lost. I have reverted them back.

Please let me know if there is an issue with anything else, and thank you for your patience.

@rm41339
Copy link
Author

rm41339 commented Apr 2, 2025

I have caught an error in my commit. My apologies, am fixing it right now. I'm still not quite sure by what you meant about adding autogen-modules to libExecTestBenchCommons?

@VeryMilkyJoe
Copy link
Collaborator

Looks good to me now, if the CI is green I think we can merge :)
Just one last nitpick, I think we should rename the testdata/autogen-completion directory to testdata/completion so we can add more testdata for completions there in the future.
You already made the change, adding them both to libExecTestBenchCommons, so no worries, I'm happy now!

@rm41339
Copy link
Author

rm41339 commented Apr 2, 2025

Sounds great, thank you for checking! I've renamed the directory :)

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.

Changes look good to me!

@fendor fendor requested a review from VeryMilkyJoe April 3, 2025 16:59
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.

hls-cabal-plugin: autogen-module completion is missing
3 participants