-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Improve suggestion for "missing function argument" on multiline call #144966
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
|
033e147
to
1eb4aef
Compare
LL | function_with_lots_of_arguments( | ||
LL | variable_name, | ||
LL ~ /* char */, | ||
LL ~ variable_name, |
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.
I don't understand why this line is labeled as ~
, as it is (well, should be) identical to the initial source. I tried to tweak the code a bit, but found nothing :/
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.
Because the suggestion I think is being constructed as "/* char */\n "
not "\n /* char */"
, so it's modifying an existing line (where the next arg already is) by adding the new arg suggestion in, then adding the new line with an existing suggestion which also being considered a modification.
You could modify this by appending the line to the previous arg, I think but it would require restructuring your logic.
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.
Seems fine to me
LL - p3, p4, p5, p6, p7, p8, | ||
LL - ); | ||
LL + foo(p1, /* Arc<T2> */, p3, p4, p5, p6, p7, p8); | ||
LL ~ foo( |
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.
This case is a bit unfortunate, though I don't know if we'd benefit from tweaking the heuristic to detect "fake" multiline args (i.e. when there's only one line break).
1eb4aef
to
1e271d6
Compare
@bors r+ rollup looks like a good change as-is. Maybe you can improve the heuristic slightly later as per errs' suggestion |
Rollup of 4 pull requests Successful merges: - #144966 ( Improve suggestion for "missing function argument" on multiline call) - #145111 (remove some unused private trait impls) - #145221 (Fix Cargo cross-compilation (take two)) - #145247 (Update `sysinfo` version to `0.37.0`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144966 - scrabsha:push-rozroqqmurvu, r=jdonszelmann Improve suggestion for "missing function argument" on multiline call `rustc` has a very neat suggestion when the argument count does not match, with a nice placeholder that shows where an argument may be missing. Unfortunately the suggestion is always single-line, even when the function call spans across multiple lines. With this PR, `rustc` tries to guess if the function call is multiline or not, and emits a multiline suggestion when required. r? `@jdonszelmann`
rustc
has a very neat suggestion when the argument count does not match, with a nice placeholder that shows where an argument may be missing. Unfortunately the suggestion is always single-line, even when the function call spans across multiple lines. With this PR,rustc
tries to guess if the function call is multiline or not, and emits a multiline suggestion when required.r? @jdonszelmann