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

Hover does not work for type symbols from external packages #717

Closed
jacg opened this issue Dec 15, 2019 · 9 comments · Fixed by #4241
Closed

Hover does not work for type symbols from external packages #717

jacg opened this issue Dec 15, 2019 · 9 comments · Fixed by #4241
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@jacg
Copy link
Contributor

jacg commented Dec 15, 2019 •
edited by Anton-Latukha
Loading

... as documented by the tests titled type constructor from other package, value from other package and external class is signature.

To be precise, hover does work for external values, but not types.
jacg referenced this issue in jacg/ghcide Dec 17, 2019
@jacg
Fix #249 and #250
490b296 
This fixes hover for types, classes and type variables.

Information about spans includes a `Maybe Type` which is `Just` for data-level
expressions and `Nothing` for type-level expressions.

`AtPoint.atPoint` which is the oddly-named function responsible for constructing
hover information, runs in the `Maybe` monad, and aborted at the first sight of
a `Nothing`, thus producing no hover information for type-level spans.

In the process of fixing this, I have refactored the function to

+ separate the construction of data-level and type-level hover info

+ make the components that make up the hover info (and their construction) more
  clear

I can see plenty little improvements that could be made to the functionality of
the code (and lots that could be made to its organization), but the most
important fixes of the basic missing functionality are here.

Fix #249
Fix #250
jacg referenced this issue in jacg/ghcide Dec 17, 2019
@jacg
Fix #249 and #250
4ed27a1 
This fixes hover for types, classes and type variables.

Information about spans includes a `Maybe Type` which is `Just` for data-level
expressions and `Nothing` for type-level expressions.

`AtPoint.atPoint` which is the oddly-named function responsible for constructing
hover information, runs in the `Maybe` monad, and aborted at the first sight of
a `Nothing`, thus producing no hover information for type-level spans.

In the process of fixing this, I have refactored the function to

+ separate the construction of data-level and type-level hover info

+ make the components that make up the hover info (and their construction) more
  clear

I can see plenty little improvements that could be made to the functionality of
the code (and lots that could be made to its organization), but the most
important fixes of the basic missing functionality are here.

Fix #249
Fix #250
@jneira
Copy link
Member

jneira commented Sep 16, 2020

The tests mentioned are:

https://github.com/haskell/ghcide/blob/8d478394afe072b02cb40a2a98baadbf1a787eda/test/exe/Main.hs#L2211-L2212

https://github.com/haskell/ghcide/blob/8d478394afe072b02cb40a2a98baadbf1a787eda/test/exe/Main.hs#L2220

@jneira
Copy link
Member

jneira commented Sep 16, 2020

I've just test this and goto def to external packages does not work but hover on external values/types does

@jneira jneira transferred this issue from haskell/ghcide Dec 29, 2020
@jneira
Copy link
Member

jneira commented Dec 29, 2020

Test in hls are in:

haskell-language-server/ghcide/test/exe/Main.hs

Lines 2487 to 2488 in d4f5d43

, test broken yes xtcL5 xtc "type constructor external #248,249"
, test broken yes xvL20 xvMsg "value external package #249" -- 120

haskell-language-server/ghcide/test/exe/Main.hs

Lines 2517 to 2518 in d4f5d43

, test broken broken outL45 outSig "top-level signature #310"
, test broken broken innL48 innSig "inner signature #310"

@jneira jneira changed the title Hover / goto def do not work for symbols from external packages Hover does not work for symbols from external packages Dec 29, 2020
@jneira jneira added component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Dec 29, 2020
@Anton-Latukha
Copy link
Collaborator

I believe it works now.

@jneira
Copy link
Member

jneira commented Dec 21, 2021 •
edited
Loading

Afaik the broken mark in tests should make the test fail if they are fixed. Although maybe they fail for another reason.
In other similar issue i removed the broken mark to check if the test continue failing for the same reason.

@jacg
Copy link
Contributor Author

jacg commented Dec 21, 2021

I regret that I don't have the time to work on this any more. 😞

Afaik the broken mark in tests should make the test fail if they are fixed.

Yes, that was the original intention when I wrote these tests. IIRC, it did actually work like that, at least at the beginning.

@jneira
Copy link
Member

jneira commented Dec 21, 2021

Although maybe they fail for another reason.

It happened to me for the top level signature/inner signature (they are handled in another issue)
I removed the broken mark and both continue failing obviusly. But visually top-level signature was fixed!
The test was failing due to the hover position, slightly different. Fixing hover position in the test made it green.

@jneira jneira changed the title Hover does not work for symbols from external packages Hover does not work for type symbols from external packages Jan 22, 2022
@jneira jneira mentioned this issue Jan 22, 2022
Closed
@hasufell
Copy link
Member

I'm not sure if this ticket is still relevant. There are other tickets for goto definition like #708

Feel free to re-open this ticket with a clarification.

@soulomoon
Copy link
Collaborator

soulomoon commented May 17, 2024 •
edited
Loading

We should enable the test for it too. doing at #4241

soulomoon added a commit that referenced this issue May 17, 2024
@soulomoon
enable test for #717
65a9068
@soulomoon soulomoon linked a pull request May 17, 2024 that will close this issue
Enable test for #717 #4241
Merged
@soulomoon soulomoon reopened this May 17, 2024
@soulomoon soulomoon mentioned this issue May 17, 2024
Merged
soulomoon added a commit that referenced this issue May 18, 2024
@soulomoon
Enable test for #717 (#4241)
fb5506c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enable test for #717 haskell/haskell-language-server
5 participants
@jneira @hasufell @jacg @soulomoon @Anton-Latukha