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

chore: upgrade clap to v4 #17333

Merged
merged 24 commits into from
Mar 26, 2023
Merged

chore: upgrade clap to v4 #17333

merged 24 commits into from
Mar 26, 2023

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented Jan 10, 2023

Blocked on clap-rs/clap#4769

@crowlKats crowlKats marked this pull request as ready for review March 20, 2023 17:40
cli/mitata.rs Outdated Show resolved Hide resolved
cli/mitata.rs Outdated Show resolved Hide resolved
cli/mitata.rs Outdated Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crowlKats crowlKats merged commit 33362b8 into denoland:main Mar 26, 2023
@crowlKats crowlKats deleted the clap_4 branch March 28, 2023 14:58
mmastrac pushed a commit that referenced this pull request Mar 31, 2023
crowlKats pushed a commit that referenced this pull request 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 pull request 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 pull request 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants