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

Can conflict markers indicate what revision each version originates from? #1176

Open
dbarnett opened this issue Feb 2, 2023 · 7 comments
Open
Labels
enhancement New feature or request

Comments

@dbarnett
Copy link
Collaborator

dbarnett commented Feb 2, 2023

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 dbarnett added the enhancement New feature or request label Feb 2, 2023
@martinvonz
Copy link
Member

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
>>>>>>>

@dbarnett
Copy link
Collaborator Author

dbarnett commented Mar 2, 2023

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.

martinvonz added a commit that referenced this issue Jul 5, 2023
I don't think we'll want to record a label for each term, because such
labels would get stale, and it seems hard to make them make sense
after transferring a remote to another repo. I think we'll probably
want to infer labels on demand instead (#1176).
martinvonz added a commit that referenced this issue Jul 5, 2023
I don't think we'll want to record a label for each term, because such
labels would get stale, and it seems hard to make them make sense
after transferring a remote to another repo. I think we'll probably
want to infer labels on demand instead (#1176).
@joyously
Copy link

joyously commented Sep 1, 2023

There is an issue in difftastic that is very related, and it talks about a config option in git: Wilfred/difftastic#565

@ilyagr
Copy link
Collaborator

ilyagr commented Mar 21, 2024

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
Copy link
Collaborator

ilyagr commented Apr 5, 2024

Here's what I have so far for this: #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).

@poliorcetics
Copy link
Collaborator

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

@ilyagr ilyagr removed their assignment May 2, 2024
@ilyagr
Copy link
Collaborator

ilyagr commented May 2, 2024

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

ilyagr added a commit to ilyagr/jj that referenced this issue May 6, 2024
For example, 

```
<<<<<<< Conflict 1 of 3
+++++++ Contents of side #1
left 3.1
left 3.2
left 3.3
%%%%%%% Changes from base to side #2
-line 3
+right 3.1
>>>>>>>
```

or

```
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
-line 3
+right 3.1
+++++++ Contents of side #2
left 3.1
left 3.2
left 3.3
>>>>>>>
```

Currently, there is no way to disable these, this is TODO for a future
PR. Other TODOs for future PRs: make these labels configurable. After
that, we could support a `diff3/git`-like conflict format as well, in
principle.

Counting conflicts helps with knowing whether you fixed all the
conflicts while you are in the editor.

While labeling "side #1", etc, does not tell you the commit id or
description as requested in jj-vcs#1176, I still think it's an improvement.
Most importantly, I hope this will make `jj`'s conflict format less
scary-looking for new users.

I've used this for a bit, and I like it. Without the labels, I would see
that the two conflicts have a different order of conflict markers, but I
wouldn't be able to remember what that means. For longer diffs, it can
be tricky for me to quickly tell that it's a diff as opposed to one of
the sides. This also creates some hope of being able to navigate a
conflict with more than 2 sides.

Another not-so-secret goal for this is explained in
jj-vcs#3109 (comment). The
idea is a little weird, but I *think* it could be helpful, and I'd like
to experiment with it.
ilyagr added a commit that referenced this issue May 6, 2024
For example, 

```
<<<<<<< Conflict 1 of 3
+++++++ Contents of side #1
left 3.1
left 3.2
left 3.3
%%%%%%% Changes from base to side #2
-line 3
+right 3.1
>>>>>>>
```

or

```
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
-line 3
+right 3.1
+++++++ Contents of side #2
left 3.1
left 3.2
left 3.3
>>>>>>>
```

Currently, there is no way to disable these, this is TODO for a future
PR. Other TODOs for future PRs: make these labels configurable. After
that, we could support a `diff3/git`-like conflict format as well, in
principle.

Counting conflicts helps with knowing whether you fixed all the
conflicts while you are in the editor.

While labeling "side #1", etc, does not tell you the commit id or
description as requested in #1176, I still think it's an improvement.
Most importantly, I hope this will make `jj`'s conflict format less
scary-looking for new users.

I've used this for a bit, and I like it. Without the labels, I would see
that the two conflicts have a different order of conflict markers, but I
wouldn't be able to remember what that means. For longer diffs, it can
be tricky for me to quickly tell that it's a diff as opposed to one of
the sides. This also creates some hope of being able to navigate a
conflict with more than 2 sides.

Another not-so-secret goal for this is explained in
#3109 (comment). The
idea is a little weird, but I *think* it could be helpful, and I'd like
to experiment with it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants