fix(assert): Prevent arguments from requiring self #5836
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omertuc
changed the title
Please do. Generally, we recommend PRs be structured as
This
In this case, its going to be a little more obvious, so we might be able to let it slide (in my ideal world, I'd even break out that argument unpacking into its own refactor commit). |
|
Makes sense, will do that |
Use tuple destructuring to make the code clearer Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
omertuc
force-pushed
the
reqself
branch
2 times, most recently
from
e1265ba to
43fd3c0
Compare
|
Is this what you had in mind? |
Add a test that shows that clap doesn't complain when a flag requires itself. This test demonstrates existing broken behavior, ideally it should panic. It will be fixed in the next commit. Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
It's non-sensical for an argument to require itself, so it must be a mistake, and should be prevented. This is arguably a breaking change, but of the spacebar heating kind. Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
|
Thanks! Small commits like that make things very easy to review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's non-sensical for an argument to require itself, so it must be a mistake, and should be prevented.
This is arguably a breaking change, but of the spacebar heating kind.
Let me know if I should add any tests for this.