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

Improve OnHover for Def/Let, App and Const #986

Merged
merged 10 commits into from Jan 25, 2023
Merged

Improve OnHover for Def/Let, App and Const #986

merged 10 commits into from Jan 25, 2023

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Jan 8, 2023 •
edited

@xsebek xsebek requested a review from kostmo January 8, 2023 23:47
@restyled-io restyled-io bot mentioned this pull request Jan 8, 2023
Merged
@xsebek xsebek requested a review from byorgey January 8, 2023 23:51
@xsebek xsebek mentioned this pull request Jan 9, 2023
Open
byorgey
byorgey approved these changes Jan 9, 2023
View reviewed changes
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
src/Swarm/Language/LSP/Hover.hs Show resolved Hide resolved
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
@xsebek
Copy link
Member Author

xsebek commented Jan 13, 2023 •
edited

I have rebased on the AST with types (#991) in the branch hover-types.

@byorgey and @kostmo feel free to test it. 🙂

The results look quite nice:

Local variables

let def
image image
bind lambda
image image

Tuples

Tuple Individual element (needed fix)
Screenshot from 2023-01-13 20-56-56 Screenshot from 2023-01-13 20-57-03

Literal types

All literals and constants get a type header.

Different generic and specific Same types
image image
Text Direction
image image

Delay

image

@xsebek xsebek mentioned this pull request Jan 15, 2023
Closed
2 tasks
kostmo
kostmo approved these changes Jan 17, 2023
View reviewed changes
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
src/Swarm/Language/LSP/Hover.hs Outdated Show resolved Hide resolved
@xsebek
Improve OnHover for Def/Let, App and Const
e31e191 
* add signature to const
* add signature to var
* add signature to let/def
* unfold function application
* hide internal force applications
* fix indenting for multiline text

The typing is not really correct and only works for top-level variables.
That still gives us all defined variable types in call sites, which I hope
will be useful.

Just don't overwrite the names.
@xsebek
Fix compilation with new TModule
9ef13cc
@xsebek
Use new type annotations
6106779
@xsebek
Improve TVar and SLam
d52c44a
@xsebek
Refactor and pretty print tuples
7eee79a
@xsebek
Get rid of length
cd1d072 
turns out it was never needed
@xsebek
Fix bound variable type
232babf
@xsebek
Pretty delay documentation
d24a855
@xsebek xsebek requested review from byorgey and kostmo January 25, 2023 00:33
@xsebek
Copy link
Member Author

xsebek commented Jan 25, 2023 •
edited

@kostmo please take a look at this again, I have changed quite a lot after rebasing on the AST types. 😉

EDIT: The test failure is just an import problem with doctest, I will fix it soon.

kostmo
kostmo approved these changes Jan 25, 2023
View reviewed changes
Copy link
Member

@kostmo kostmo left a comment

Choose a reason for hiding this comment

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

Very nice! 🚢

byorgey
byorgey approved these changes Jan 25, 2023
View reviewed changes
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

Nice!!

The formatting for function application explanations looks weird in emacs:

func-app-hover

Not sure what's going on there. But I don't think it should hold up merging this.

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Jan 25, 2023
@xsebek
Copy link
Member Author

xsebek commented Jan 25, 2023

@byorgey it seems emacs has a different take on markdown. 😕 It should be easy to tweak by adding newlines, but I don't have emacs set up. 😅

@byorgey
Copy link
Member

byorgey commented Jan 25, 2023

@xsebek no worries, maybe I can poke at it a bit.

@xsebek
Copy link
Member Author

xsebek commented Jan 25, 2023

It seems emacs can render triple backtick code in lists, but if it turns out to be an issue, I can restore my original code.

At some point, I had an extra parameter for "depth" that used single backticks in lists. It did not have nice syntax highlighting but it might render better for emacs.

It was making the code a bit messy, so it's best avoided if possible. 😅

@mergify mergify bot merged commit 652ba55 into main Jan 25, 2023
@mergify mergify bot deleted the unfold-app branch January 25, 2023 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostmo kostmo kostmo approved these changes

@byorgey byorgey byorgey approved these changes

Assignees
No one assigned
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unfold function application in LSP Hover
3 participants
@xsebek @byorgey @kostmo