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

Cabal go to module's definition #4380

Merged
merged 30 commits into from
Aug 22, 2024

Conversation

VenInf
Copy link
Contributor

@VenInf VenInf commented Aug 14, 2024

Go to the module's definition

If you click go-to definition on the field under exposed-module or other-module
it will open the file where this module was defined.

Video with an example:

go-to-definition-cut.mp4

Implementation details

The go-to definition function compares the highlighted text with modules in the cabal file.
If there is a match, it takes the respective build target and tries to fetch their hsSourceDirs from the PackageDescription. (by looking at all buildInfos with matching names).

After finding them, it constructs a path using directory where the cabal file is located, the info from hsSourceDirs and a name of the module converted to a path.
If the file exists it returns the Definition with the acquired location.

What isn't implemented

  • No tests
  • Current solution can't differentiate modules with same names.
  • Current solution to choose if a definition can be provided isn't great, and is just a chain of if's
  • The range in location is just zero, instead of highlighting the name of the module.

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, couple of nitpicks, then this should be good to go!

plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs Outdated Show resolved Hide resolved
plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs Outdated Show resolved Hide resolved
plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs Outdated Show resolved Hide resolved
plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs Outdated Show resolved Hide resolved
plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs Outdated Show resolved Hide resolved
@VenInf
Copy link
Contributor Author

VenInf commented Aug 20, 2024

Noteworthy change:
Split one large gotoDefinition function to multiple specific ones.
This will hurt the performance a little bit, but hopefully make the addition of other definitions easier.

Copy link
Collaborator

@VeryMilkyJoe VeryMilkyJoe 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, this looks great! I just have some small comments.

@VenInf VenInf requested review from VeryMilkyJoe and fendor and removed request for VeryMilkyJoe August 21, 2024 17:54
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!

@fendor fendor merged commit 9cc8c62 into haskell:master Aug 22, 2024
33 of 34 checks passed
soulomoon pushed a commit to soulomoon/haskell-language-server that referenced this pull request Sep 23, 2024
If you click go-to definition on the field under `exposed-module` or `other-module`
it will open the file where this module was defined.

The go-to definition function compares the highlighted text with modules in the cabal file.
If there is a match, it takes the respective build target and tries to fetch their `hsSourceDirs` from the `PackageDescription`. (by looking at all `buildInfos` with matching names).

After finding them, it constructs a path using directory where the cabal file is located, the info from `hsSourceDirs` and a name of the module converted to a path.
If the file exists it returns the `Definition` with the acquired location.

---------

Co-authored-by: fendor <fendor@users.noreply.github.com>
Co-authored-by: Chrizzl <hochrainer.christoph@gmail.com>
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.

4 participants