-
Notifications
You must be signed in to change notification settings - Fork 63
Source range for pipe value used as unlabelled arg #6787
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
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
CodSpeed Instrumentation Performance ReportMerging #6787 will not alter performanceComparing Summary
|
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.
Looks good. One suggestion, if you like.
.clone() | ||
.map(|val| Arg::new(val, pipe_value_source_range)); | ||
let Some(unlabeled) = args.unlabeled.take().or(default_unlabeled) else { | ||
let unlabelled = args.kw_args.unlabeled.take().or_else(|| args.pipe_value.take()); |
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.
We already have Args::unlabeled_kw_arg_unconverted()
that computes the unlabeled arg from all its possible inputs. To reduce duplication, or at least keep it together, could we maybe make another flavored function over there in Args
that takes ownership, instead of reaching into it from here?
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Fixes #6613
Extracted from #6644 to just fix the source range without touching the kwarg shorthand (left for post 1.0).