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

Make spans for tuple patterns in E0023 more precise #88123

Merged
merged 7 commits into from
Aug 27, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Aug 17, 2021

As suggested in #86307. Closes #86307.

r? @estebank

@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Aug 17, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2021
@rust-log-analyzer

This comment has been minimized.

@camelid camelid force-pushed the tup-pat-precise-spans branch from 15b1b2c to da25af2 Compare August 17, 2021 21:28
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 19, 2021
@rust-log-analyzer

This comment has been minimized.

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 21, 2021
@rust-log-analyzer

This comment has been minimized.

@camelid camelid force-pushed the tup-pat-precise-spans branch from 4ffe312 to 08ceac8 Compare August 21, 2021 23:15
@rust-log-analyzer

This comment has been minimized.

Comment on lines +65 to +68
help: a tuple struct with a similar name exists
|
LL | Z1(_, _) => {}
| ~~
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that we should suggest this only if the number of fields matches or if nothing else is being suggested (but that seems out of scope for this PR).

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Have to say, love how this is shaping up 😍

@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 22, 2021
@inquisitivecrystal inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021
* Highlight the whole pattern if it has no fields
* Highlight the whole definition if it has no fields
* Only highlight the pattern name if the pattern is multi-line
* Determine whether a pattern is multi-line based on distance from name
  to last field, rather than first field
@camelid camelid removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 26, 2021
@camelid
Copy link
Member Author

camelid commented Aug 26, 2021

r=me after removing the secondary span label with no text in the pattern for cases where there's more than 0 fields, and changing the 0 field case to point at the whole pattern (like it used to).

Ok, done! I'll let you approve it so you can check that the new output matches what you were expecting :)

@camelid
Copy link
Member Author

camelid commented Aug 26, 2021

From the commit message:

  • Determine whether a pattern is multi-line based on distance from name
    to last field, rather than first field

I made this change so that code like the following that uses "visual indentation" is understood as multi-line:

M(1,
  2,
  3)

@estebank
Copy link
Contributor

I made this change so that code like the following that uses "visual indentation" is understood as multi-line:

M(1,
  2,
  3)

Personally I would lean towards hiding the secondary span label under M simply because the code will be visible, but I'm not bothered by it :)

@bors r+

@bors
Copy link
Contributor

bors commented Aug 26, 2021

📌 Commit 8a6501d has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 26, 2021
…tebank

Make spans for tuple patterns in E0023 more precise

As suggested in rust-lang#86307. Closes rust-lang#86307.

r? `@estebank`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 26, 2021
…tebank

Make spans for tuple patterns in E0023 more precise

As suggested in rust-lang#86307. Closes rust-lang#86307.

r? ``@estebank``
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 26, 2021
…tebank

Make spans for tuple patterns in E0023 more precise

As suggested in rust-lang#86307. Closes rust-lang#86307.

r? ```@estebank```
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 27, 2021
…arth

Rollup of 11 pull requests

Successful merges:

 - rust-lang#87832 (Fix debugger stepping behavior with `match` expressions)
 - rust-lang#88123 (Make spans for tuple patterns in E0023 more precise)
 - rust-lang#88215 (Reland rust-lang#83738: "rustdoc: Don't load all extern crates unconditionally")
 - rust-lang#88216 (Don't stabilize creation of TryReserveError instances)
 - rust-lang#88270 (Handle type ascription type ops in NLL HRTB diagnostics)
 - rust-lang#88289 (Fixes for LLVM change 0f45c16)
 - rust-lang#88320 (type_implements_trait consider obligation failure on overflow)
 - rust-lang#88332 (Add argument types tait tests)
 - rust-lang#88340 (Add `c_size_t` and `c_ssize_t` to `std::os::raw`.)
 - rust-lang#88346 (Revert "Add type of a let tait test impl trait straight in let")
 - rust-lang#88348 (Add field types tait tests)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8aa46e5 into rust-lang:master Aug 27, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 27, 2021
@camelid camelid deleted the tup-pat-precise-spans branch August 27, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E0023 should have smaller, more targeted spans
7 participants