-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
[WIP] Better flag parsing #1200
Conversation
There are benefits to caring about the ordering e.g. $ deno gist.ts --title X --allow-net
# vs
$ deno --allow-net gist.ts --title X For example you can complain about typos e.g. |
@hayd that’s a good point (that should be discussed in the code comment) My argument against it: V8 has all these flags which are silently extracted from argv.. it works. By being less strict we allow the user code to take on that responsibility. |
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 - thanks!
Changes since v0.1.12: - First pass at running subprocesses (denoland#1156) - Improve flag parsing (denoland#1200) - Improve fetch() (denoland#1194 denoland#1188 denoland#1102) - Support shebang (denoland#1197)
I really dislike this change. If you want to pass args to v8 or deno, pass them before the filename. That's the standard. Anything else is going to be surprising.
Perhaps the solution is to work out a way to be more explicit here. Require all args before the script to be consumed either by deno or v8. That way we can reject typos in v8 args too. That way there's nothing to do in userland! It seems clear there's a cost. What's the benefit of this ambiguity? |
Your dislike is because you don't get errors when args are misspelled? Or are there other reasons? I do a lot of command-line editing, and I dislike having to carefully place arguments. It's very nice to do "up up -D enter" and get debug output. I also think this behavior is easy to explain - whereas telling people where to place arguments is more complex. |
I prefer errors be loud and not happen at runtime, so I think catching typos is valuable. If deno adds a arg, or there is a v8 arg you don't know about/added, it's going to be confusing that it's not available in deno.args (and that it's modified v8 somehow). up ctrl+a alt+f -D is a few more keystrokes BUT I'd argue generally a user is more likely to debug their own code rather than deno's core. I wonder if you're optimizing development rather than majority use case (which is, of course, fine at the moment). An alternative is to alias deno and ddeno resp. to up ctrl+a d which is the same number keystrokes 😄 |
I mean, -D is just an example, it could as well be "up up --allow-net enter" or "up up -r enter" I think this is the least surprising behavior. |
I still don't buy this argument, but I wonder if a way to move forward here is to pass --version and --help along if a script is passed. i.e. cc @lucascaro |
Based on @ry's patch: https://gist.github.com/ry/158e4d3ca54a57f4926bd0a39fd5a844
Fixes #1198
This is a bit of brute force approach because we're parsing arguments one-by-one using
getopts.parse
and storing unrecognized arguments to be later passed todeno.args
. Any ideas how to that do better are welcome.There is still work to do, because at the moment V8 swallows arguments passed after
--
.