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

Feature: hover for variables #309

Closed

Conversation

kyklish
Copy link
Contributor

@kyklish kyklish commented Dec 26, 2022

Closes #289.

Now, working only for global variables (vars inside functions are without hover).

Notifying @mark-wiemer

@mark-wiemer
Copy link
Member

Hey @kyklish , is there a reason this is still a draft?

@kyklish
Copy link
Contributor Author

kyklish commented Jan 17, 2023

Hey @kyklish , is there a reason this is still a draft?

It's quick Copy& Paste code. I did not quite understand yet, why hover not shown for local variables and do we need it at all for local variables.

I did not test it, like I should.

@kyklish kyklish marked this pull request as ready for review January 20, 2023 15:41
mark-wiemer
mark-wiemer previously approved these changes Jan 29, 2023
@mark-wiemer mark-wiemer self-requested a review January 29, 2023 19:52
@mark-wiemer mark-wiemer dismissed their stale review January 29, 2023 19:52

mistaken approval

Copy link
Member

@mark-wiemer mark-wiemer left a comment

Choose a reason for hiding this comment

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

Let's also try to add the internal/external logic we have in formatter. Let's get the list of variables from the document cache and then send it to a helper function that takes a list of variables and a variable name and returns the first variable that matches the provided name. We'll call the helper with one file at a time to avoid performance issues with larger projects.

Similarly for ahkHoverProvider.ts, let's try to add unit tests to the return value of the modified function.

src/parser/parser.ts Show resolved Hide resolved
src/parser/parser.ts Show resolved Hide resolved
@kyklish kyklish marked this pull request as draft February 11, 2023 12:54
kyklish and others added 2 commits February 19, 2023 14:02
Co-authored-by: Mark Wiemer <7833360+mark-wiemer@users.noreply.github.com>
Co-authored-by: Mark Wiemer <7833360+mark-wiemer@users.noreply.github.com>
@rlitwiller89
Copy link

Would it be possible to do this for labels too?

@kyklish kyklish changed the title Hover for variables Feature: Hover for variables Apr 1, 2023
@kyklish kyklish changed the title Feature: Hover for variables Feature: hover for variables Apr 1, 2023
@mark-wiemer mark-wiemer closed this Feb 3, 2024
@kyklish kyklish deleted the feature/hover-for-variable branch April 21, 2024 07:38
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.

Add doc comments to variables and labels
3 participants