jj icon indicating copy to clipboard operation
jj copied to clipboard

Can conflict markers indicate what revision each version originates from?

Open dbarnett opened this issue 2 years ago • 8 comments

When you have a conflict and materialize the conflict markers into the working copy, can the markers give some hint of origin for each version of the changes like revision IDs?

For instance the conflicts in a file will look like

<<<<<<<
%%%%%%%
some version
+++++++
other version
>>>>>>>

but without context on where these versions originated from, it can be hard to figure out how to resolve the changes. Could there be any kind of hint IDs/descriptions maybe after the markers on the same line to give context?

dbarnett avatar Feb 02 '23 02:02 dbarnett

I'm surprised it took so long before someone asked for this. The situation is different from most VCSs because we allow conflicts to be manipulated. Let's look at an example:

# Initial log
B
|
| C
|/
A
# Rebase C onto B:
C conflict (B + (C - A))
|
B
|
A

If you now look at the conflict in C, what would you like to see? Maybe something this:

<<<<<<<
%%%%%%% A -> C
-A
+C
+++++++ B
B
>>>>>>>

Or maybe this:

<<<<<<<
%%%%%%% changes in rebased commit (C)
-A
+C
+++++++ rebase destination (B)
B
>>>>>>>

Let's say you now rebase B and C onto another commit - D:

C conflict (D + (B - A) + (C - A))
|
B conflict (D + (B - A))
|
D
|
A

What should we say about the conflict in C now? Maybe something like this would be ideal:

<<<<<<<
%%%%%%% changes in parent commit
-A
+B
%%%%%%% changes in rebased commit (C)
-A
+C
+++++++ rebase destination (D)
D
>>>>>>>

Keep in mind that conflicts may be exchanged. That's disallowed today, but we should make it possible if the remote somehow says it supports it, or if the user tells us that they want to push the conflict. That means that any information we embed in the conflict itself might not make sense in the remote repo, since that repo doesn't necessarily know about the same commits.

Also note that conflicts can be moved between commits (e.g. jj restore -r abc123 path/to/file/with/conflict). I'm not sure what descriptions to attach to the markers in such cases.

I'm leaning towards not recording any extra information in conflicts about where they came from. I think it's better to instead try to come up with a description when we materialize the conflict. An easy first step would be to say if any of the sides match the state in a parent commit. That would make the markers in the initial example look something like this:

<<<<<<<
%%%%%%% changes to apply
-A
+C
+++++++ changes already in parent B
B
>>>>>>>

The last example might be like this:

<<<<<<<
%%%%%%% changes already in parent B
-A
+B
%%%%%%% changes to apply
-A
+C
+++++++ changes already in parent B
D
>>>>>>>

For a merge commit, we might present it like this:

<<<<<<<
%%%%%%% changes from C
-A
+C
+++++++ changes already in parent B
B
>>>>>>>

martinvonz avatar Feb 10 '23 00:02 martinvonz

Yeah, that sounds like a good start.

I don't have a great mental model yet for what it means to conflicts around and divorce them from their source, but it might be helpful to record op info into the conflicts in those cases so you could say "restored from X" or "fetched from remote ABC". Anyway I think that could be layered on later if needed.

dbarnett avatar Mar 02 '23 18:03 dbarnett

There is an issue in difftastic that is very related, and it talks about a config option in git: https://github.com/Wilfred/difftastic/issues/565

joyously avatar Sep 01 '23 14:09 joyously

I think the simplest thing to implement would be:

<<<<<<<
%%%%%%% changes from base to side 1
-A
+C
+++++++ content of side 2
B
>>>>>>>

This would also extend to many-sided conflicts (with "base" replaced by "base A", "base B", etc). I think this would already be helpful for users who see our conflict format for the first time.

We could make this gradually more intelligent, e.g. comparing whether one of the sides comes from the parent commit. This seems closely related to an intelligent jj blame (possibly beyond the intelligence of a first version of a jj blame). I'm not sure whether or not jj blame could be intelligent enough for this be without storing extra metadata, but either way it's a more general question than just for describing conflicts.

ilyagr avatar Mar 21 '24 23:03 ilyagr

Here's what I have so far for this: https://github.com/martinvonz/jj/pull/3459.

I don't think I'll be able to add much more information than that anytime soon, but maybe eventually. In the near future, the main thing I'd add on top of that PR would be to make it configurable what kind of conflict markers you get (with or without labels, for example).

ilyagr avatar Apr 05 '24 07:04 ilyagr

with or without labels, for example

Probably unnecessary ? I don't think git offers the option to hide labels and I never saw anyone or any tool having issues with labels

poliorcetics avatar Apr 05 '24 12:04 poliorcetics

I unassigned myself; I'm not sure if I intend to take this much further than #3459 in the near future.

ilyagr avatar May 02 '24 23:05 ilyagr

Perhaps we can add back the labels removed in commit 651a3cb. After this issue was created, we have started recording conflicts at the root-tree level instead at individual paths (#1624). It might make sense to record a string describing the first conflict tree and one string for each tree diff. That might be okay even when copying only part of a diff because it can be up to each caller to set the description. For example, jj squash -i --from k --into l might attach a description like "Partial changes from " or something like that. I'm not sure this will work well but I think it's worth considering.

martinvonz avatar May 26 '25 03:05 martinvonz