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

[FE-6195] Disable JS Completions #495

Merged
merged 4 commits into from
Dec 10, 2024
Merged

[FE-6195] Disable JS Completions #495

merged 4 commits into from
Dec 10, 2024

Conversation

henryfauna
Copy link
Contributor

@henryfauna henryfauna commented Dec 9, 2024

Ticket(s): FE-6195

Problem

Setting completer: undefined when we set up the repl means we default to JS completions. This is why you can get things like:

Screenshot 2024-12-09 at 10 42 11 AM

setTimeout is of course not a function in Fauna. Previously the shell would parse the expression and catch the appropriate error code to avoid logging these sorts of errors. See here:

const res = esprima.parseScript(`(${cmd})`);

Solution

Disable JS completions as these don't make sense here.

Testing

Behold typing Time. does not blow up:

Screenshot 2024-12-09 at 10 58 01 AM

ignoreUndefined: true,
preview: argv.apiVersion !== "10",
// TODO: integrate with fql-analyzer for completions
completer: argv.apiVersion === "10" ? () => [] : undefined,
completer: () => [],
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, we technically do lose functionality here vs the old CLI:
iTerm2 2024-12-09 12 00 19

I'm not sure I care, given that FQL v4 is on the deprecation path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah your right - let me take a closer look

Copy link
Collaborator

Choose a reason for hiding this comment

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

the way the old code works is by making a nodeJS sub-vm with a context object that only contains the names defined in v4 syntax. so, setTimeout isn't a suggestion because setTimeout isn't defined as a global var in the sub-vm that evaluates completions.

https://nodejs.org/api/vm.html#scriptruninnewcontextcontextobject-options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I think v4 completions should be back -

Screenshot 2024-12-09 at 4 40 42 PM

Copy link
Collaborator

Choose a reason for hiding this comment

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

awesome, thank you for looking into it!

@henryfauna henryfauna merged commit 5951ab7 into v3 Dec 10, 2024
4 checks passed
@henryfauna henryfauna deleted the FE-6195 branch December 10, 2024 17:36
@mwilde345 mwilde345 mentioned this pull request Dec 10, 2024
@cleve-fauna cleve-fauna mentioned this pull request Dec 13, 2024
@mwilde345 mwilde345 mentioned this pull request Dec 18, 2024
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.

4 participants