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

Add inlay hint support #342

Merged
merged 2 commits into from
Jun 23, 2023
Merged

Add inlay hint support #342

merged 2 commits into from
Jun 23, 2023

Conversation

alcarney
Copy link
Collaborator

Description (e.g. "Related to ...", etc.)

  • This adds support for textDocument/inlayHint and inlayHint/resolve requests from v3.17 of the spec.
  • Adds an example server that uses inlay hint to display the binary representation of a number inline in editors that support it.

Note: This requires a fix to lsprotocol in order work correctly (microsoft/lsprotocol#219)

Screenshot_20230618_005141

Code review checklist (for code reviewer to complete)

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Commit messages are meaningful (see this for details)
  • Tests have been included and/or updated, as appropriate
  • Docstrings have been included and/or updated, as appropriate
  • Standalone docs have been updated accordingly
  • CONTRIBUTORS.md was updated, as appropriate
  • Changelog has been updated, as needed (see CHANGELOG.md)

@alcarney alcarney requested a review from tombh June 18, 2023 00:00
tombh
tombh previously approved these changes Jun 18, 2023
@tombh
Copy link
Collaborator

tombh commented Jun 18, 2023

Amazing to see these tests doing their magic, hats off.

So just waiting on that Pyodide test and the lsprotocol fix then?

@alcarney
Copy link
Collaborator Author

alcarney commented Jun 19, 2023

that Pyodide test

It's that flaky one we saw before... so it looks like I'll have to revisit the pyodide test runner before long

@tombh
Copy link
Collaborator

tombh commented Jun 20, 2023

Ohh yes, right I see. Ok, what are you thinking then? Some pinning?

@alcarney
Copy link
Collaborator Author

Ok, what are you thinking then? Some pinning?

At least a lower version bound on typing-extensions but at this point it's probably useful to see if we can get the installation logs out of the pyodide runtime so we can see what's going on.

lsprotocol 2023.0.0a2 is out which should mean this is ready to go - I've set the version in setup.cfg in line with the discussions in #331

@alcarney alcarney requested a review from tombh June 22, 2023 18:10
@tombh
Copy link
Collaborator

tombh commented Jun 23, 2023

At least a lower version bound on typing-extensions but at this point it's probably useful to see if we can get the installation logs out of the pyodide runtime so we can see what's going on.

Sure ✨

lsprotocol 2023.0.0a2 is out which should mean this is ready to go - I've set the version in setup.cfg in line with the discussions in #331

Greta, and I think I can just close fix: Pin lsprotocol depdency #345 because I'd have to update it to the later version that you need here. A git blame and these comments and issue references are good enough papertrail I think.

Awesome work again Alex 🤓

@tombh tombh merged commit 3dcd0a6 into openlawlibrary:master Jun 23, 2023
13 checks passed
@tombh
Copy link
Collaborator

tombh commented Jun 23, 2023

Oh, BTW, I guess you don't need a Pygls release? But might as well do one anyway? My only hesitancy is the lsprotocol pin, I'll mention it in #345.

@alcarney alcarney deleted the inlay-hints branch June 23, 2023 19:08
@alcarney
Copy link
Collaborator Author

alcarney commented Jun 23, 2023

A new release would be nice... though do we want to push and get support for all of v3.17 in first? It would also give a bit of time for the new release of lsprotocol to propagate

@tombh
Copy link
Collaborator

tombh commented Jun 23, 2023

I must admit I didn't even think Pygls even supported all of 3.16. I guess we get a lot for free now by depending on lsprotocol? Here's the changelog from the LSP website:

Specify how clients will handle stale requests.
Add support for a completion item label details.
Add support for workspace symbol resolve request.
Add support for label details and insert text mode on completion items.
Add support for shared values on CompletionItemList.
Add support for HTML tags in Markdown.
Add support for collapsed text in folding.
Add support for trigger kinds on code action requests.
Add the following support to semantic tokens:
    server cancelable
    augmentation of syntax tokens
Add support to negotiate the position encoding.
Add support for relative patterns in file watchers.
Add support for type hierarchies
Add support for inline values.
Add support for inlay hints.
Add support for notebook documents.
Add support for diagnostic pull model.

What are the main things you think would be good to get in Pygls?

@alcarney
Copy link
Collaborator Author

I didn't even think Pygls even supported all of 3.16

Well... I'm assuming we do! 😅 It would be interesting to know if we have any gaps.

What are the main things you think would be good to get in Pygls?

We do get a lot for free now, I think somewhere between 1/2 - 2/3 of that list is just new fields on types coming from lsprotocol.

Work for us to do is the new methods

  • Add support for type hierarchies
  • Add support for inline values.
  • Add support for inlay hints. (done!)
  • Add support for workspace symbol resolve request.
  • Add support for diagnostic pull model.

And these

  • Add support for notebook documents. - We already have an issue for that in (Add support for notebooks #311)
  • Add support to negotiate the position encoding.

I would imagine we can do nearly all of that apart from Add support to negotiate the position encoding. since, if I'm honest, I wouldn't know where to start with that one!

@tombh
Copy link
Collaborator

tombh commented Jun 26, 2023

Well... I'm assuming we do! sweat_smile It would be interesting to know if we have any gaps.

It was just an assumption. I've come across gaps over the months, and perhaps I mistakenly assumed they were for older versions, whereas in fact they're legitimate gaps in the recent 3.17 spec.

We do get a lot for free now, I think somewhere between 1/2 - 2/3 of that list is just new fields on types coming from lsprotocol.

😏

Great, so I've made a dedicated issue for the remaining 3.17 features #346

I would imagine we can do nearly all of that apart from Add support to negotiate the position encoding. since, if I'm honest, I wouldn't know where to start with that one!

I actually am in a good position do that one because of all the detailed work on #304

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.

None yet

2 participants