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

Fixes signature help edge case #152

Merged
merged 2 commits into from
Dec 12, 2023
Merged

Conversation

ncordon
Copy link
Collaborator

@ncordon ncordon commented Dec 8, 2023

What

Fixes the case of getting the overlay with the information about the first argument for functions.

How

RETURN apoc.do.when( gets parsed as:

      statements
     /    |    \     
statement (    EOF
    /  \
RETURN  expression

rather than:

      statements
     /    |    \     
statement (    EOF
    /  \
RETURN  functionInvocation

so we need to treat that case differently because we cannot modify the relative priority of functionInvocation vs expression

RETURN apoc.do.when(x gets parsed correctly in contrast (when we've started the first argument), because the parser has enough information to recognize it is a function invocation.

When our final character is (, just go to the preceeding statement and traverse it to find whether it ends on a expression. If it does, that's our function name.

Copy link
Collaborator

@OskarDamkjaer OskarDamkjaer left a comment

Choose a reason for hiding this comment

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

I worry a little bit about false positives with this approach (since expressions are so flexible), but I can't think of any compelling examples or any other solution barring looking into changing the error recovery so I think it makes sense to go ahead

packages/language-support/src/signatureHelp.ts Outdated Show resolved Hide resolved
packages/language-support/src/tests/signatureHelp.test.ts Outdated Show resolved Hide resolved
packages/language-support/src/signatureHelp.ts Outdated Show resolved Hide resolved
@ncordon ncordon merged commit 816122b into main Dec 12, 2023
4 checks passed
@ncordon ncordon deleted the fixes-signature-help-edge-case branch December 12, 2023 16:04
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