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

Add another way to resolve merge conflicts with Meld #3109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add another way to resolve merge conflicts with Meld #3109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* Added completions for [Nushell](https://nushell.sh) to `jj util completion`

* Another way to resolve conflicts with Meld, in 3 tabs, adding to the
experimental `meld-3` tool.
amiryal marked this conversation as resolved.
Show resolved Hide resolved

### Fixed bugs

* On Windows, symlinks in the repo are now materialized as regular files in the
Expand Down
8 changes: 8 additions & 0 deletions cli/src/config/merge_tools.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ merge-args = ["$left", "$base", "$right", "-o", "$output", "--auto-merge"]
program="meld"
# If using this as a template, note that `$output` is repeated twice below
edit-args = ["$left", "$output", "$right", "-o", "$output"]
# With the normal Meld merge conflict resolution setup, changes from `$base` to
# either side are not highlighted. Here we offer an alternative that opens the
# original two sides compared to the base in two more tabs.
merge-args = ["--diff", "$left", "$output", "$right", "--diff", "$base", "$left", "--diff", "$base", "$right"]
Copy link
Collaborator

@ilyagr ilyagr Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Have you tried making the first of these a proper merge tab (with or without auto-merge)? I'm guessing it doesn't work?

If it worked, there wouldn't be a need to pre-populate the output pane.

Copy link
Collaborator

@ilyagr ilyagr Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I actually find this merge view with jj's conflict presentation in the middle confusing; the way jj currently shows merge conflict becomes misleading in the central tab.

image

Of the two other tabs, only one is actually useful in this case of rebase:

image

It really makes it seem like "impl Input..." is the base unless you think about it carefully.

From this, you can tell that this commit renames an identifier.

The other one is not very useful (for a rebase merge conflict, at least):

image

There's this weird fact that even though it's clear what the files are in the two extra tabs, each tab is confusing on its own.

I think I'm still OK with using this as an "experimental" merge view, unless we come up with something better, but I'm thinking about putting more warnings in the description.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The problem I have with Meld’s 3-pane merging mode is that, when there is a conflict (an area marked with red background), it stops highlighting the character-level changes and leaves you an opaque wall of code in which to hunt for changes, as in a Find The Differences game:

image
meld left base right -o output --auto-merge

This is not what I signed up for when I launched a GUI app.

If you know how to read JJ’s conflict markers, then at least you get some line-level hints with merge-tool-edits-conflict-markers = true. This is the first tab of meld-3:

image
meld left output right

I presume I don’t need to explain the conflict markers here. Just remember that the part with +++++++ will be identical to one of the sides, the context ( ) and removed (-) lines in the %%%%%%% part come from the base, and the added (+) lines would be from the side that was not chosen for +++++++. Older versions of Meld would have made it more obvious, as they did not have the red background for conflicts, and identical parts used to be not highlighted.

Going back to my pain point, the lack of character-level highlighting in 3-pane mode, this is what the other two tabs are for. They let you see the changes each side had made, highlighted nicely. Normally, only the “local” (right) side will be interesting, because these are the changes we would like to apply. It is perfectly normal in my workflow to ignore the middle tab, though sometimes I do like to peek into what the “other” (left) side was doing.

Side note: Using the 3-pane mode without --auto-merge should be reserved for pathological cases where automatic merging does the wrong thing:

image
meld left base right -o output

I hope I covered all of your concerns. Now I will take it unto myself to distil it down to a few succinct words to include in the TOML comment and in the docs.

Copy link
Collaborator

@ilyagr ilyagr Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The problem I have with Meld’s 3-pane merging mode is that, when there is a conflict (an area marked with red background), it stops highlighting the character-level changes and leaves you an opaque wall of code in which to hunt for changes, as in a Find The Differences game:

I know this problem, and it makes sense to me that showing the two extra tabs helps with it.

However, it would make more sense to me if the first tab (3-pane one) would just show the normal Meld merge view, with --auto-merge:

image

It seems less confusing than the view your current setup shows as the first tab. On the other hand, I'm not sure whether Meld can show merge tabs and diff tabs at the same time.

If what I suggest were possible, the only difference between the meld and meld-3 merge tools would be the two extra tabs.

If it's not possible, we can go with what you have now and hope to improve it later.

A somewhat out-there option, especially if Meld cannot show merge tabs and diff tabs at the same time, would be to teach jj to produce different kinds of conflict markers. For example, I think that Git-style conflict markers would be less confusing in this UI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, I know I said I'm thinking about putting more warnings in the description, but your text LGTM at the moment. I think you improved it a bit since I last looked (👍 ), but it was quite clear before as well. We can always improve it later if needed. It'd take me effort to try to add warnings without making it less clear, and it's quite possible that I'd conclude that it's better as is if I tried.

# The output pane in the first tab must be prepopulated with _something_. The
# conflict markers were chosen here, rather than simply the base, because they
# contain enough information to let you skip the other tabs for simple cases.
merge-tool-edits-conflict-markers = true

[merge-tools.vimdiff]
program = "vim"
Expand Down
10 changes: 10 additions & 0 deletions docs/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,16 @@ the conflict is done, `jj` assumes that the conflict was only partially resolved
and parses the conflict markers to get the new state of the conflict. The
conflict is considered fully resolved when there are no conflict markers left.

### Experimental 3-tab conflict resolution

The `"meld-3"` merge editor opens 3 tabs in Meld. The first tab is a 3-pane
view of the left and right sides with an editing pane in the middle. The
editing pane in the middle of the first tab contains [enough
information](conflicts.md#conflict-markers) on its own to resolve conflicts.
The next two tabs are 2-pane views for reference, each showing a diff from the
base to one of the sides. Of the two reference tabs, the last tab (the right
side compared to base) is generally the more useful one.

## Commit Signing

`jj` can be configured to sign and verify the commits it creates using either
Expand Down