-
-
Notifications
You must be signed in to change notification settings - Fork 820
feat: add UX to cycle through completions in the REPL #2463
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
Conversation
Signed-off-by: Snehil Shah <[email protected]>
lib/node_modules/@stdlib/repl/test/integration/test.completer_engine.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/repl/test/integration/test.completer_engine.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Athan <[email protected]>
Co-authored-by: Athan <[email protected]> Signed-off-by: Snehil Shah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very slick and works great, thanks so much @Snehil-Shah for your work on this!
One small issue: The current behavior of the tab completions is a little unintuitive when there are too many completions to be rendered in the terminal. Those won't be visible but one can then still navigate to them with the right arrow keys.
Signed-off-by: Snehil Shah <[email protected]>
@Planeshifter I updated the code, now one can only traverse what they see. And as we also support interactive terminal window sizes, they can always resize. Here's a video demonstrating this: resize.mp4 |
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
@Snehil-Shah Pulled down the latest and played around. Works nicely. Thanks for working on this. One comment: currently, when I enter What I wonder is whether we should eliminate the need for repeatedly hitting TAB in order to see the list of potential matches. In terms of UX, one thing I do when seeing completion possibilities is use the list to help guide my spelling, having immediate feedback that I've misspelled something when the list disappears due to a spelling mistake. In which case, I wonder if hitting TAB and triggering a completion list should trigger a "completion mode" in which the list of completions is continually displayed and updated as I enter new characters, without needing to explicitly hit TAB after every entered character. Not sure how feasible this suggestion is. |
@Snehil-Shah Yes, this is what I was thinking. Thanks for the updates. The only other small niggle is that, when I type |
@kgryte The new UX doesn't make the completions disappear due to an exact match. Turns out it's a bug in the I will investigate this and fix it in a separate patch PR. |
Signed-off-by: Snehil Shah <[email protected]>
Also, wrote the test for this and the specific case you mentioned of an exact match. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did another look through the PR and played around with the latest changes; looks all good to me!
lib/node_modules/@stdlib/repl/test/integration/test.completer_engine.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/repl/test/integration/test.completer_engine.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's go ahead and get this in. Any remaining bug fixes can be addressed in follow-up PRs. Thanks, @Snehil-Shah, for contributing, and @Planeshifter for reviewing!
Subtask of #1845
Description
This pull request:
Related Issues
This pull request:
Questions
No.
Other
Recording of new behavior:
completer.mp4
Checklist
@stdlib-js/reviewers