-
Notifications
You must be signed in to change notification settings - Fork 199
Improve REPL directive support #348
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
- use . on column 0 as directive prefix - use `directives` object properties for genericity - accept non ambiguous directive abbreviations - simplify `handle_directive` and `handle_cmd`
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.
Left some comments.
I think the first prompt should also be updated from:
QuickJS-ng - Type "\h" for help
to:
QuickJS-ng - Type ".help" for help
std.loadScript(filename); | ||
return true; | ||
} | ||
if (a[0] !== '\\' && a[0] !== '.') |
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.
If we are switching to dot directives, it feeels weird to get .quit
in the docs, for example, but for \quit
to also work.
Also, \quit()
and \quit123
also work, which is arguably wrong.
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.
I agree .quit()
and \quit123
should be rejected. Will do.
I switched to .
directives because they are more readable and that's what node uses, I kept backslash for muscle memory, but I should probably remove it at some point.
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.
👍
repl.js
Outdated
"\\clear clear the terminal\n" + | ||
"\\q exit\n"); | ||
var sel = (n) => n ? "*": " "; | ||
std.puts(".help this help\n" + |
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.
Document load
?
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.
Yes, definitely. Will do.
Yes. Good point. |
- reject invalid directive with extra characters - document ".load"
directives
object properties for genericityhandle_directive
andhandle_cmd