Skip to content

Remove inlay hints in favor of VS Code's built-in support #16

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

Merged
merged 2 commits into from
Jul 4, 2022

Conversation

xeger
Copy link
Contributor

@xeger xeger commented Jul 3, 2022

This effectively prevents ruby-syntax-tree/syntax_tree#103 from happening. It also lets us drop a setting (Code just uses the built-in colors, fonts, etc for inlay hints).

Open questions:

  1. Do we bother to retain the homebrew hinting code for awhile? I'm guessing not; with only 66 installs I'm thinking this extension is still in its early days.
  2. Should we perform a version check of stree at startup? It's probably not necessary (it will advertise its capabilities directly to the client) ... though I would like to do Provide a clearer error message when the stree exe cannot be spawned #13 at some point and give better guidance when no stree or no ruby can be found.

Weird, Unrelated Issue

While testing this, I noticed an oddity (not caused by this PR): the formatting only works if the file exists on disk at the time I ask to format it. For example, if the editor is a real file:

image

Whereas, if the editor is an unsaved buffer using the Ruby language mode:

image

Any idea what's up with that?!

@kddnewton
Copy link
Member

Oh thank god this is so much cleaner. I wouldn't bother with version checking yet, this thing is not being used a ton yet.

I have no idea what's going on with the bug you mentioned. Maybe it has to do with the activation events here? https://github.com/ruby-syntax-tree/vscode-syntax-tree/blob/main/package.json#L20-L26 Maybe we should be listening for something else as well?

@kddnewton kddnewton merged commit f591859 into ruby-syntax-tree:main Jul 4, 2022
@xeger
Copy link
Contributor Author

xeger commented Jul 10, 2022

The activation hypothesis is a good one. I'll take a look if/when I delve into the double activation thing I'm seeing in the tests.

@xeger xeger deleted the inlayHint branch July 22, 2022 14:44
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.

2 participants