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

Improve error output of “Detect diff markers in the parser” #113826

Closed
rben01 opened this issue Jul 18, 2023 · 1 comment · Fixed by #126125
Closed

Improve error output of “Detect diff markers in the parser” #113826

rben01 opened this issue Jul 18, 2023 · 1 comment · Fixed by #126125
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rben01
Copy link

rben01 commented Jul 18, 2023

Copied from my comment in #106242:

I'd love this functionality but I'm not a huge fan of the error messages themselves.

error: encountered diff marker
  --> $DIR/enum-2.rs:3:1
   |
LL | <<<<<<< HEAD
   | ^^^^^^^ after this is the code before the merge
LL |         x: u8,
LL | |||||||
   | -------
LL |         z: (),
LL | =======
   | -------
LL |         y: i8,
LL | >>>>>>> branch
   | ^^^^^^^ above this are the incoming code changes
   |
   = help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
   = help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
   = note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

First, I believe these markers are called conflict markers, not diff markers. (IIUC diff markers are e.g., +++ file.rs.)

Second, I think the intermediate markers | and = should also be explained.

Third, I find the help: tips a bit verbose — especially considering that they'll likely line wrap in most terminals — and perhaps redundant given the content of the ^^^^^^^ notes. It might be better to stick the help: info in rustc --explain E#### instead of showing every time this error is encountered. Not sure where I stand on the note: tip; it's also a bit long, and its presence might lead people away from rustc --explain, which is probably undesirable. (The git documentation also has a lot of extraneous info that most users wouldn't need to solve this error; they just need to know which is old and which is new, <<<<<<< or >>>>>>>.)

Finally, I think the words “above” and “after” are a bit ambiguous; there's nearly an entire file above and below the outer conflict markers. (What's important is what's between conflict markers.) Furthermore they aren't quite accurate when there's a ||||||| section, since that section is below the <<<<<<< but not part of “our” code.

I think the following error message would be overall clearer and more accurate:

error: git conflict markers in source code
  --> $DIR/enum-2.rs:3:1
   |
LL | <<<<<<< HEAD
   | ^^^^^^^ between this marker and `|||||||` is the code that we're merging into
   |
LL | ||||||| 4fa0b83
   | ^^^^^^^ between this marker and `=======` is the base code (what the two refs diverged from)
   |
LL | =======
   | ^^^^^^^ between this marker and `>>>>>>>` is the incoming code
   |
LL | >>>>>>> branch
   | ^^^^^^^ this marker concludes the conflict region
   |
   = help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
   = help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers

error: aborting due to previous error

or, if not using diff3,

error: git conflict markers in source code
  --> $DIR/enum-2.rs:3:1
   |
LL | <<<<<<< HEAD
   | ^^^^^^^ between this marker and `=======` is the code that we're merging into
   |
LL | =======
   | ^^^^^^^ between this marker and `>>>>>>>` is the incoming code
   |
LL | >>>>>>> branch
   | ^^^^^^^ this marker concludes the conflict region
   |
   = help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
   = help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers

error: aborting due to previous error
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 18, 2023
@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-papercut Diagnostics: An error or lint that needs small tweaks. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 18, 2023
@dev-ardi
Copy link
Contributor

@rustbot claim

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 21, 2024
Improve conflict marker recovery

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
closes rust-lang#113826
r? `@estebank` since you reviewed rust-lang#115413
cc: `@rben01` since you opened up the issue in the first place
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 21, 2024
Improve conflict marker recovery

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
closes rust-lang#113826
r? ``@estebank`` since you reviewed rust-lang#115413
cc: ``@rben01`` since you opened up the issue in the first place
@bors bors closed this as completed in 73cc4ec Jun 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 21, 2024
Rollup merge of rust-lang#126125 - dev-ardi:conflict-markers, r=estebank

Improve conflict marker recovery

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
closes rust-lang#113826
r? ```@estebank``` since you reviewed rust-lang#115413
cc: ```@rben01``` since you opened up the issue in the first place
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 D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants