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

For position named arguments lint, underline both curly brackets when inserting and don't add $ to formatting names in messages #99907

Closed
PrestonFrom opened this issue Jul 29, 2022 · 1 comment · Fixed by #99958
Assignees

Comments

@PrestonFrom
Copy link
Contributor

PrestonFrom commented Jul 29, 2022

A comment asked why we only underline the closing curly brace when inserting (https://github.com/rust-lang/rust/pull/99660/files/1b2e05e212f9e2d50fb35817a80136018fbea4ba#r932952838) a named argument. This was originally because that's where it gets inserted -- but on reflection, this is potentially confusing. So underline both.

Also, in the lint message, the formatting argument names are emitted with a dollar sign.
this formatting argument uses named argument precision$ by position
This is also potentially confusing because the actual name does not contain it.

@PrestonFrom
Copy link
Contributor Author

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 2, 2022
…-errors

Improve position named arguments lint underline and formatting names

For named arguments used as implicit position arguments, underline both
the opening curly brace and either:
* if there is formatting, the next character (which will either be the
  closing curl brace or the `:` denoting the start of formatting args)
* if there is no formatting, the entire arg span (important if there is
  whitespace like `{  }`)

This should make it more obvious where the named argument should be.

Additionally, in the lint message, emit the formatting argument names
without a dollar sign to avoid potentially confusion.

Fixes rust-lang#99907
@bors bors closed this as completed in d0ea440 Aug 3, 2022
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 a pull request may close this issue.

1 participant