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

REPL aggressively uses wrong autocomplete on ENTER #42040

Closed
spazmodius opened this issue Feb 17, 2022 · 8 comments
Closed

REPL aggressively uses wrong autocomplete on ENTER #42040

spazmodius opened this issue Feb 17, 2022 · 8 comments
Assignees
Labels
repl Issues and PRs related to the REPL subsystem.

Comments

@spazmodius
Copy link

Version

v16.13.0

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

repl

What steps will reproduce the bug?

When typing into the REPL, autocomplete often predicts identifiers or keywords even when the typed characters are valid. When I hit ENTER (no TAB to accept), then the REPL chooses to use its prediction rather than what I actually typed.

For example, typing

image

then hitting ENTER

image

It's infuriating.

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

I expect to be able to hit TAB to accept the prediction. If I continue to type something else then autocomplete should revise its prediction. If I hit enter, autocomplete should realize its services are no longer needed.

What do you see instead?

> var add = x => y => x+y

followed by ENTER results in

> var add = x => y => x+yield
...

Additional information

No response

@VoltrexKeyva VoltrexKeyva added the repl Issues and PRs related to the REPL subsystem. label Feb 17, 2022
@benjamingr
Copy link
Member

I am able to reproduce this in both v16 latest and master. I think the ask here is reasonable and this isn't really a hard fix but I'm not sure if it's intentional.

@nodejs/repl @BridgeAR wdyt?

@benjamingr
Copy link
Member

@meixg since you committed there in the past and it doesn't look like a hard fix, want to take a look at this too?

@meixg
Copy link
Member

meixg commented Feb 18, 2022

@meixg since you committed there in the past and it doesn't look like a hard fix, want to take a look at this too?

Yes, with pleasure :)

@meixg
Copy link
Member

meixg commented Feb 19, 2022

I made a PR may fix this: #42053


But there may be two more things we can improve:

  1. The preview of var add = x => y => x+y should be just y, not yield . The problem here is that we currently implement autocomplete based on regular expressions, so the context information in var add = x => y => x+y is lost. Maybe we should use a parser repl: #41690 REPL gives wrong autocomplete on literals #41883 (comment).
  2. There is a 500ms delay after pressing ESCAPE: https://github.com/nodejs/node/blob/master/lib/internal/readline/emitKeypressEvents.js#L67, if we press another key after ESCAPE within 500ms, the ESCAPE will be ignored.
    Visual feedback (PR above) can help a lot, but I wonder if we can find a way to remove this delay completely.

@spazmodius
Copy link
Author

Thanks for working on this, @meixg . But I want to clarify that the issue I raise here is not related to "preview" (the advance evaluation of the line I'm still typing), but rather "autocomplete", particularly preferring it's own suggestion without confirmation from me.

I like autocomplete, when it doesn't get in my way.

Preview, however, is... uncompelling.

@meixg
Copy link
Member

meixg commented Feb 19, 2022

Thanks for working on this, @meixg . But I want to clarify that the issue I raise here is not related to "preview" (the advance evaluation of the line I'm still typing), but rather "autocomplete", particularly preferring it's own suggestion without confirmation from me.

I like autocomplete, when it doesn't get in my way.

Preview, however, is... uncompelling.

I know what you mean, see 80913e6, when you press ENTER, is actually the 'preview' part that is used.

With the PR above, it will behave more like the Chrome DevTools console.

@spazmodius
Copy link
Author

I agree Chrome Devtools has a good usability model to emulate. I'll give this observation, then leave it in your capable hands:

In Devtools, when I'm at the point of having an outstanding suggestion, my first ENTER accepts the suggestion, and a second ENTER submits the line. In Node REPL, a single ENTER does both of these.

@meixg
Copy link
Member

meixg commented Feb 21, 2022

I agree Chrome Devtools has a good usability model to emulate. I'll give this observation, then leave it in your capable hands:

In Devtools, when I'm at the point of having an outstanding suggestion, my first ENTER accepts the suggestion, and a second ENTER submits the line. In Node REPL, a single ENTER does both of these.

Yeah, there is a different behavior too, I can try to improve that.

Trott pushed a commit that referenced this issue Feb 23, 2022
Fix: #42040

PR-URL: #42053
Fixes: #42040
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
sxa pushed a commit to sxa/node that referenced this issue Mar 7, 2022
Fix: nodejs#42040

PR-URL: nodejs#42053
Fixes: nodejs#42040
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 21, 2022
Fix: #42040

PR-URL: #42053
Fixes: #42040
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Fix: #42040

PR-URL: #42053
Fixes: #42040
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Fix: #42040

PR-URL: #42053
Fixes: #42040
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Fix: #42040

PR-URL: #42053
Fixes: #42040
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants