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

Detect Merge Conflict Text #36879

Closed
Havvy opened this issue Oct 1, 2016 · 11 comments
Closed

Detect Merge Conflict Text #36879

Havvy opened this issue Oct 1, 2016 · 11 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST

Comments

@Havvy
Copy link
Contributor

Havvy commented Oct 1, 2016

I saw a tweet about git merge conflict text in source code, and wondered what the Rust compiler does. As such, I created a play.rust-lang.org sample.

It currently detects more than two equals in a row, doesn't know what it means, and bails. Specifically, it looks like this:

rustc 1.12.0 (3191fbae9 2016-09-23)
error: expected one of `!`, `+`, `->`, `::`, `>`, or `as`, found `==`
 --> <anon>:4:1
  |
4 | =======
  | ^^

error: aborting due to previous error

I propose that when this happens, the compiler should look for the git merge conflict issues, and if it finds it, give a specific error message related to what is actually happening.

The error message could show the start and end of the merge sections, and actually give searchable text for what this strange textual artifact is for those who've never seen it before.

For an example of how it'd look:

error: Merge conflict detected.
 --> <anon>:2:1
  |
2 |<<<<<<< HEAD
  |^^^^^^^^^^^^ Note: Beginning of commit HEAD here.
  |
4 | =======
  | ^^^^^^^ Note: Divider between commits HEAD and ab890fb here.
  |
5 |>>>>>>> ab890fb
  |^^^^^^^^^^^^^^^ Note: End of commit ab890fb here.

error: aborting due to previous error

The only con I can think of is that we're specifically caring about the semantics of one tool that isn't really related to Rust. But at the same time, git is widely used.

@8573
Copy link

8573 commented Oct 1, 2016

The only con I can think of is that we're specifically caring about the semantics of one tool that isn't really related to Rust.

Perhaps it could use this diagnostic iff a Git repository is detected?

@solson
Copy link
Member

solson commented Oct 1, 2016

Or do other merge tools use similar marks?

@8573
Copy link

8573 commented Oct 1, 2016

Hm, I do seem to recall that <<<...===....>>> notation predating Git.

@8573
Copy link

8573 commented Oct 1, 2016

Looks like it dates back to RCS, or even earlier.

@Havvy Havvy changed the title Detect Git Merge Conflict Text Detect Merge Conflict Text Oct 1, 2016
@Havvy
Copy link
Contributor Author

Havvy commented Oct 1, 2016

Alright, removed the "Git" part from this.

@8573
Copy link

8573 commented Oct 1, 2016

Still, I think trying to detect a VCS (lazily and memoizedly) would be useful for giving the user more context.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Oct 1, 2016

Still, I think trying to detect a VCS (lazily and memoizedly) would be useful for giving the user more context.

Why? Since the format is apparently not specific to one VCS, the compiler can just say "merge conflict" and perhaps drop the term "commit". The user (if they are in a position to resolve the merge conflict) will presumably know which VCS they are using. Not to mention that a merge conflict marker remains a merge conflict marker when it lives outside a VCS directory — for example, because the source code was downloaded from crates.io. Add to that the work needed to implement this feature and the danger of the detection being overly brittle, and it's a clear 👎 for me.

@TimNN
Copy link
Contributor

TimNN commented Oct 1, 2016

In my personal opinion a functionality like this does not belong in the rust compiler. The current error message already identifies one of the three locations, which I think should be enough.

@arielb1
Copy link
Contributor

arielb1 commented Oct 1, 2016

GCC returns the following error:

test.c:1:1: error: version control conflict marker in file
 <<<<<<< HEAD
 ^~~~~~~

@8573
Copy link

8573 commented Oct 1, 2016

Still, I think trying to detect a VCS (lazily and memoizedly) would be useful for giving the user more context.
Why?

While I still think it could be useful, I've failed to think of any feature for which it would be necessary, save one: giving the user VCS-specific tips or URLs of documentation re resolving merge conflicts — and of course rustc could instead (in e.g. its --explain text) merely give the user a list of such and have them pick.

I am and have been quite okay with a generic "merge conflict detected" message, although looking at GCC's above, I think I would prefer "merge conflict marker detected", as it doesn't necessarily mean there's an actual merge conflict.

@Manishearth Manishearth added A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST labels Oct 5, 2016
@jfirebaugh
Copy link
Contributor

Duplicate of #32059.

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 A-parser Area: The parsing of Rust source code to an AST
Projects
None yet
Development

No branches or pull requests

8 participants