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

Fix REPL type display #1610

Merged
merged 3 commits into from
Nov 11, 2023
Merged

Fix REPL type display #1610

merged 3 commits into from
Nov 11, 2023

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented Nov 7, 2023

Make sure it again prints the type on one line, by using prettyTextLine instead of prettyText to format the type.

Fixes #1597.

Make sure it again prints the type on one line, by using
`prettyTextLine` instead of `prettyText` to format the type.

Fixes #1597.
@byorgey byorgey added the CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. label Nov 7, 2023
@byorgey byorgey requested a review from xsebek November 7, 2023 17:16
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

I wonder what happens if the type is too long, should there be some take maxTypeWidth followed by ...?

@byorgey
Copy link
Member Author

byorgey commented Nov 11, 2023

Hmm, good question. The type just gets cut off:

repl-type-long

I will see whether there's a quick fix for putting a ... at the end. One problem is we'd like it to be dynamic based on the size of the window.

@byorgey
Copy link
Member Author

byorgey commented Nov 11, 2023

@xsebek How about this? It's not ideal the way it cuts off the type textually (note below how a -> gets cut off in the middle so we see -...), but figuring out how to cut it off semantically (so e.g. we always put the ... at some kind of natural break) feels like a project for another time.

repl-type-long2

@xsebek
Copy link
Member

xsebek commented Nov 11, 2023

Looks good! 👍

I think this will be more than enough context for REPL, anything beyond that should really be done in editor with LSP. 🙂

@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Nov 11, 2023
@mergify mergify bot merged commit 3592b4e into main Nov 11, 2023
@mergify mergify bot deleted the fix/repl-type-display branch November 11, 2023 20:47
byorgey added a commit that referenced this pull request Feb 12, 2024
Update to use the `lsp-2.4` API.  Closes #1350.

Initially I hoped that any `lsp-2.x` would work.  However, the `SeverDefinition` record changed in `2.2` so I initially set that as a lower bound.  But then it turns out that `2.4` changed which module it is importing `Rope` from; since we work with ropes in the `Hover` module it matters since we have to import the matching module.  Updating to the new `Rope` type also required some changes as the API provided by the new `Rope.Mixed` module is a bit different than the old module, so we would not even be able to easily put in CPP to conditionally depend on the right rope module depending on the `lsp` version.  Finally, this means dropping support for GHC 9.0 since `lsp-2.4` does not support it.

Along the way I also fixed a minor issue related to showing type information returned by the LSP server, so that it uses `prettyTypeLine` to display the type on a single line (in my editor, when the type does not use a single line it gets cut off).  For comparison see also #1610. 

This refactoring was a big pain because a lot of things (names of types and constructors, locations of exports, etc...) changed from 1.x to 2.x, but there was not much in the way of documenting what had changed. =(   I am pretty sure that all functionality has been preserved but I would appreciate independent confirmation.

This is also a prerequisite for updating other dependencies such as the `base` version (I will open a follow-up PR soon) since the old `lsp-1.x` versions do not allow many newer versions of various dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type displayed in UR corner of REPL is incomplete
2 participants