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.

Sign up for GitHub

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

Jump to bottom

Improve error output when encountering git conflict markers #115413

Closed
wants to merge 2 commits into from
Closed

Improve error output when encountering git conflict markers #115413

wants to merge 2 commits into from

Conversation

atsuzaki
Copy link
Contributor

Applied suggestions in #113826 with minor modifications.

The current output is as follows:

error: encountered git conflict marker
  --> $DIR/enum-2.rs:3:1
   |
LL | <<<<<<< HEAD
   | ^^^^^^^ between this marker and `|||||||` is the code that we're merging into
LL |         x: u8,
LL | |||||||
   | ------- between this marker and `=======` is the base code (what the two refs diverged from)
LL |         z: (),
LL | =======
   | ------- between this marker and `>>>>>>>` is the incoming code
LL |         y: i8,
LL | >>>>>>> branch
   | ^^^^^^^
   |
   = 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
   = note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

w/o diff3

error: encountered git conflict marker
  --> $DIR/enum.rs:2:1
   |
LL | <<<<<<< HEAD
   | ^^^^^^^ between this marker and `=======` is the code that we're merging into
LL |     Foo(u8),
LL | =======
   | ------- between this marker and `>>>>>>>` is the incoming code
LL |     Bar(i8),
LL | >>>>>>> branch
   | ^^^^^^^
   |
   = 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
   = note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

r? @estebank

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 31, 2023
@atsuzaki atsuzaki changed the title Conflict marker Improve error output when encountering git conflict markers Aug 31, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
estebank
estebank reviewed Sep 8, 2023
View reviewed changes
compiler/rustc_parse/src/parser/diagnostics.rs
Comment on lines -2762 to -2767
"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",
"conflict markers indicate that a merge was started but could not be completed due to merge conflicts",
);
err.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",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm slightly concerned about removing these two notes, because I find them to be a significant part of what makes the error useful. Would you be open with adding them back in addition to the new ones? Maybe marking some as note (their title is a big less colorful).

estebank
estebank reviewed Sep 8, 2023
View reviewed changes
compiler/rustc_parse/src/parser/diagnostics.rs
spans.push(span);
end = Some(span);
break;
}
self.bump();
}
let mut err = self.struct_span_err(spans, "encountered diff marker");
err.span_label(start, "after this is the code before the merge");
let mut err = self.struct_span_err(spans, "encountered git conflict marker");
Copy link
Contributor

Choose a reason for hiding this comment

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

git is not the only application that uses these kinds of conflict markers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm that's true. What about the "for more information, visit the git documentation" note at the end, should that perhaps be reworded to something more along the lines of "if you're using git check out this documentation"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me (although I feel that git's docs are useful even if you're not using git specifically).

@estebank estebank 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 Sep 8, 2023
@bors
Copy link
Contributor

bors commented Oct 29, 2023

☔ The latest upstream changes (presumably #116889) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@atsuzaki

ping from triage - can you post your status on this PR? This PR has not received an update in a few months.

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

Or if you're not going to continue, please close it. Thank you!

@atsuzaki atsuzaki closed this Feb 16, 2024
@dev-ardi dev-ardi mentioned this pull request Jun 7, 2024
Merged
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 21, 2024
@workingjubilee
Rollup merge of rust-lang#126125 - dev-ardi:conflict-markers, r=estebank
b1d0604 
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 pull request Jun 21, 2024
@matthiaskrgr
Rollup merge of rust-lang#126125 - dev-ardi:conflict-markers, r=estebank
6c4f668 
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 pull request Jun 21, 2024
@matthiaskrgr
Rollup merge of rust-lang#126125 - dev-ardi:conflict-markers, r=estebank
73cc4ec 
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
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2024
@rust-timer
Unrolled build for rust-lang#126125
906c52f 
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
Reviewers

@estebank estebank estebank left review comments

Assignees

@estebank estebank

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@atsuzaki @rust-log-analyzer @bors @JohnCSimon @estebank @rustbot