Skip to content
This repository was archived by the owner on Aug 30, 2025. It is now read-only.

Find function references#516

Merged
scohen merged 4 commits into
mainfrom
find-function-references
Dec 14, 2023
Merged

Find function references#516
scohen merged 4 commits into
mainfrom
find-function-references

Conversation

@scohen

@scohen scohen commented Dec 12, 2023

Copy link
Copy Markdown
Collaborator

Find references: Functions

Implemented find references for functions.
Initially, very few references were found, and this was because we
were deduping the index by id. Removing this call allows us to see the
entire project's references, with the notable exception of imported
macros. I believe we'll need to augment the extractor with a
compilation tracer if we wish to see those. The bad news is lexical
takes around 12 seconds to compile, but around 3 seconds to index.

Implemented find references for functions.

Initially, very few references were found, and this was because we
were deduping the index by id. Removing this call allows us to see the
entire project's references, with the notable exception of imported
macros. I believe we'll need to augment the extractor with a
compilation tracer if we wish to see those. The bad news is lexical
takes around 12 seconds to compile, but around 3 seconds to index.
@scohen scohen closed this Dec 13, 2023
@scohen scohen reopened this Dec 13, 2023
@scottming

scottming commented Dec 13, 2023

Copy link
Copy Markdown
Collaborator

"range" is incorrect, I think we should highlight the ) character, Additionally, I think we need a few simple integration tests.

CleanShot 2023-12-13 at 21 13 43@2x

@scohen

scohen commented Dec 13, 2023

Copy link
Copy Markdown
Collaborator Author

Do you mean we should highlight the ) ? Presently it's not highlighted.

@scottming

Copy link
Copy Markdown
Collaborator

Yes, sorry, fixed the typo.

@scottming

scottming commented Dec 13, 2023

Copy link
Copy Markdown
Collaborator

Additionally, do you think that besides the definition range, the places where it’s referenced also need to include the parameters within? Wouldn’t this make it seem like there’s a bit too much noise?

@scohen

scohen commented Dec 13, 2023

Copy link
Copy Markdown
Collaborator Author

I think you brought this up before, and I resisted, I see your point about it being noisy, especially when passing an anonymous function as a parameter. I'd like to use the feature more before I decide if we should just highlight the name.

The extractors were slightly wonky when extracting ranges. They would
leave off the trailing paren in calls, and would capture the whole
function definition. This fixes both issues.

There are a couple of range issues that remain, but this is a marked improvement.
@scohen

scohen commented Dec 13, 2023

Copy link
Copy Markdown
Collaborator Author

@scottming I did tone down the definitions to only include the definition up to the do.

@scohen scohen force-pushed the find-function-references branch from c2472bb to 16ea0b3 Compare December 13, 2023 21:36
@scottming

Copy link
Copy Markdown
Collaborator

I think you brought this up before, and I resisted, I see your point about it being noisy,

At that time, you mentioned that after the function definition includes do, it's better handled as a whole using Fragment-related AST functions. Additionally, considering that each navigation has different requirements for the range, I have no objections to having more information at the indexer stage.

For example, in DocumentSymbol, there's a detail field; if it includes information about parameters, then the effect is quite good. This is the result in Lua:

CleanShot 2023-12-14 at 21 26 14@2x

especially when passing an anonymous function as a parameter.

Yes, it's not just with anonymous functions, the issue also arises when the argument is a list or contains a structure.

I'd like to use the feature more before I decide if we should just highlight the name.

Sure, I respect your opinion.

@scohen scohen merged commit bc3b622 into main Dec 14, 2023
@scohen scohen deleted the find-function-references branch December 14, 2023 16:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants