-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Deno 1.34.3 panics when run with --v8-flags and no filename #19627
Labels
bug
Something isn't working correctly
Comments
crowlKats
pushed a commit
that referenced
this issue
Aug 13, 2023
…ags` is present in `deno run` (#20145) Fix #20022, fix #19627 (duplicate) #17333 upgraded clap from version 3.1 to version 4. clap version 3.2.0 (intentionally) broke a behavior that deno was relying on to make `deno run --v8-flags=--help` work without specifying a file, see clap-rs/clap#3793. The workaround was to make the script argument required _unless_ `--v8-flags` is present. This broke the expectation that all successfully parsed `run` commands have the script argument set, leading to the panic on `matches.remove_many::<String>("script_arg").unwrap()`. Clap, as far as I was able to find out, does not currently offer a neat solution to this problem. This PR adds logic to create and return a custom clap error when a parsed run command does not have the script argument. I added an appropriate test.
littledivy
pushed a commit
to littledivy/deno
that referenced
this issue
Aug 21, 2023
…ags` is present in `deno run` (denoland#20145) Fix denoland#20022, fix denoland#19627 (duplicate) denoland#17333 upgraded clap from version 3.1 to version 4. clap version 3.2.0 (intentionally) broke a behavior that deno was relying on to make `deno run --v8-flags=--help` work without specifying a file, see clap-rs/clap#3793. The workaround was to make the script argument required _unless_ `--v8-flags` is present. This broke the expectation that all successfully parsed `run` commands have the script argument set, leading to the panic on `matches.remove_many::<String>("script_arg").unwrap()`. Clap, as far as I was able to find out, does not currently offer a neat solution to this problem. This PR adds logic to create and return a custom clap error when a parsed run command does not have the script argument. I added an appropriate test.
littledivy
pushed a commit
that referenced
this issue
Aug 21, 2023
…ags` is present in `deno run` (#20145) Fix #20022, fix #19627 (duplicate) #17333 upgraded clap from version 3.1 to version 4. clap version 3.2.0 (intentionally) broke a behavior that deno was relying on to make `deno run --v8-flags=--help` work without specifying a file, see clap-rs/clap#3793. The workaround was to make the script argument required _unless_ `--v8-flags` is present. This broke the expectation that all successfully parsed `run` commands have the script argument set, leading to the panic on `matches.remove_many::<String>("script_arg").unwrap()`. Clap, as far as I was able to find out, does not currently offer a neat solution to this problem. This PR adds logic to create and return a custom clap error when a parsed run command does not have the script argument. I added an appropriate test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This produces the crash:
Looks like this line is the culprit:
deno/cli/args/flags.rs
Line 2908 in 6c6b20b
The text was updated successfully, but these errors were encountered: