Hi! I'm one of the rustc developers, with a particular focus on developer experience. This often translates into working a lot on diagnostics to make compiler errors less verbose when irrelevant, more verbose when context is needed and generally make the compiler talk to humans as humans, and not compiler engineers.
I am looking for advice in interacting with git internals correctly, and to open a conversation on what git itself could do to improve the situation.
One of the things rustc does as part of its parser is identify conflict markers to provide appropriate output. The current output for the most verbose case looks like the following:
error: encountered diff 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
| ^^^^^^^ this marker concludes the conflict region
|
= note: conflict markers indicate that a merge was started but
could not be completed due to merge conflicts
to resolve a conflict, keep only the code you want and then
delete the lines containing conflict markers
= help: if you're having merge conflicts after pulling new code:
the top section is the remote code and the bottom section
is the code you already had
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>In an attempt to make the output simultaneously less verbose and more understandable and useful, I set out to make a change that would identify what the cause of the conflict markers was, if a git operation is indeed the culprit: https://github.com/rust-lang/rust/pull/150233
What this PR does is effectively: - `git rev-parse --git-path rebase-merge` to get the rebase metadata directory - use the `head-name` as the name for the branch being rebased - use `git branch --points-at onto` to try and find the branch name that we rebased onto - if the above yields no branch names, then *assume* we are in a rebase caused by `git pull`, use `git branch -r --points-at onto` to get the remote branch name (likely `origin/main`)
With all of this, the output for the `git rebase` case is:
error: encountered diff marker
--> src/main.rs:2:1
|
2 | <<<<<<< HEAD
| ^^^^^^^ between this marker and `=======` is the code from branch
`master` we're rebasing onto
3 | println!("Hello, main!");
4 | =======
| ------- between this marker and `>>>>>>>` is the code you had in
branch `branch-2` that you are rebasing
5 | println!("Hello, branch!");
6 | >>>>>>> e644375 (branch)
| ^^^^^^^ this marker concludes the conflict region
|
= note: conflict markers indicate that a merge was started but could
not be completed due to merge conflicts
to resolve a conflict, keep only the code you want and then
delete the lines containing conflict markers
= 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>and for the `git pull` case:
error: encountered diff marker
--> src/main.rs:2:1
|
2 | <<<<<<< HEAD
| ^^^^^^^ between this marker and `=======` is the code from the
remote branch `origin/main` we're rebasing onto
3 | println!("Hello, 1!");
4 | =======
| ------- between this marker and `>>>>>>>` is the code you had in
branch `main` that you are rebasing
5 | println!("Hello, 2!");
6 | >>>>>>> ebbeec7 (second)
| ^^^^^^^ this marker concludes the conflict region
|
= note: conflict markers indicate that a merge was started but could
not be completed due to merge conflicts
to resolve a conflict, keep only the code you want and then
delete the lines containing conflict markers
= 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>There are of course other cases, like a colleague who got this diagnostic after a `git stash pop` yesterday, or caused by `git merge`, which the mentioned PR does not address.
The reason to handle this in the parser is to avoid knock down errors, but the reason for going out of our way to check the git rebase state is because it is a *common* source of confusion for even experienced people, as anecdotally evidenced by the number of threads in different forums asking for clarification. On the rustc side, there's only so much we can do to help. When working on the change, I noticed that the git internal state held some useful and necessary information for git itself to display (like the branch name for the branch being rebased), but lacked others that would be incredibly useful for the UI (and me in this case, like the name of the branch being rebased onto, which would also clarify the user-driven cause of the rebase). By having to rely instead on `git branch --points-at`, I have to deal with both the case of multiple branches pointing at the same commit *and* pay the potentially large cost of scanning all branch tips in repositories with a large number of local and remote branches.
The questions I have are: - can I *avoid* `--points-at` in any way to identify what branch we're rebasing onto? - is there already a better way to identify if the rebase was triggered by `git rebase` or `git pull` (configured to rebase)? - if neither of the above has a "yes" answer, would git consider *adding* that information, both for third-parties as well as to extend its own UI?
As a somewhat separate concern, would people have a desire to try and eliminate or minimize the confusion caused by this by modifying the generated patch output to contain *some* of the information that the proposed rustc diagnostic includes? I have concerns that having this in the text output would be verbose, and could get annoying, particularly for those for whom the git workflow is already second nature, but I am convinced that there's a better default that can be found that strikes a balance between verbosity and understandability.
Kind regards, Esteban Küber
