git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* Improving merge of tricky conflicts
@ 2020-07-21 23:29 B. Stebler
  2020-07-22  5:50 ` Johannes Sixt
  0 siblings, 1 reply; 25+ messages in thread
From: B. Stebler @ 2020-07-21 23:29 UTC (permalink / raw)
  To: git

Hi,

I have been looking for a tool to display merge conflicts, that instead 
of showing the two versions of the conflicting section, would show the 
diff for that section in both conflicting commits.

The situation that would benefit a lot from this is when one branch has 
modified a large section of code (e.g. change in indentation), while the 
other branch has only a small change. In that case git will display the 
entire modified section twice, making it very hard to spot the difference.

Being able to see both commits diff would immediately make it clear how 
to apply the small change into the branch with the large edit.

I've looked around but couldn't find anything, but I'm pretty sure 
solutions exists. Anyone knows of an existing tool / script that does this?

Thank you,
B. Stebler


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-21 23:29 Improving merge of tricky conflicts B. Stebler
@ 2020-07-22  5:50 ` Johannes Sixt
  2020-07-22  7:45   ` Jeff King
                     ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Johannes Sixt @ 2020-07-22  5:50 UTC (permalink / raw)
  To: B. Stebler; +Cc: git

Am 22.07.20 um 01:29 schrieb B. Stebler:
> I have been looking for a tool to display merge conflicts, that instead
> of showing the two versions of the conflicting section, would show the
> diff for that section in both conflicting commits.

Perhaps you want to configure `merge.conflictStyle=diff3`? It does not
exactly show a diff, but it writes the base version of the conflicted
part in addition to "ours" and "theirs".

-- Hannes

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22  5:50 ` Johannes Sixt
@ 2020-07-22  7:45   ` Jeff King
  2020-07-22 17:26     ` Junio C Hamano
  2020-07-22 20:17   ` Sergey Organov
  2020-07-22 22:48   ` Bono Stebler
  2 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2020-07-22  7:45 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: B. Stebler, git

On Wed, Jul 22, 2020 at 07:50:08AM +0200, Johannes Sixt wrote:

> Am 22.07.20 um 01:29 schrieb B. Stebler:
> > I have been looking for a tool to display merge conflicts, that instead
> > of showing the two versions of the conflicting section, would show the
> > diff for that section in both conflicting commits.
> 
> Perhaps you want to configure `merge.conflictStyle=diff3`? It does not
> exactly show a diff, but it writes the base version of the conflicted
> part in addition to "ours" and "theirs".

Yeah, I find diff3 is usually sufficient. But the contents of the base,
"ours", and "theirs" sides are also available in the index:

  # diff between base (stage 1) and ours (stage 2)
  git diff :1:file :2:file

  # diff between base (stage 1) and theirs (stage 3)
  git diff :1:file :3:file

I thought we had added nice aliases for "ours" and "theirs" instead of
the hard-to-remember stage numbers, but I think we only did so for
things like "git checkout --ours", etc.

The big downside here, of course, is that it's showing the diff for the
whole file, not just one hunk (on the other hand, I often find the
trickiest conflicts are ones where the changes unexpectedly span
multiple hunks).

There's also git-mergetool, which uses the information in those stages
to feed content to other tools, which may show conflicts in more
advanced ways. I don't have opinions on any of the particular tools it
supports, but "git mergetool --tool-help" might be a good place to start
exploring.

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22  7:45   ` Jeff King
@ 2020-07-22 17:26     ` Junio C Hamano
  2020-07-23 18:25       ` Jeff King
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2020-07-22 17:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, B. Stebler, git

Jeff King <peff@peff.net> writes:

> On Wed, Jul 22, 2020 at 07:50:08AM +0200, Johannes Sixt wrote:
>
>> Am 22.07.20 um 01:29 schrieb B. Stebler:
>> > I have been looking for a tool to display merge conflicts, that instead
>> > of showing the two versions of the conflicting section, would show the
>> > diff for that section in both conflicting commits.
>> 
>> Perhaps you want to configure `merge.conflictStyle=diff3`? It does not
>> exactly show a diff, but it writes the base version of the conflicted
>> part in addition to "ours" and "theirs".
>
> Yeah, I find diff3 is usually sufficient. But the contents of the base,
> "ours", and "theirs" sides are also available in the index:
>
>   # diff between base (stage 1) and ours (stage 2)
>   git diff :1:file :2:file
>
>   # diff between base (stage 1) and theirs (stage 3)
>   git diff :1:file :3:file
>
> I thought we had added nice aliases for "ours" and "theirs" instead of
> the hard-to-remember stage numbers, but I think we only did so for
> things like "git checkout --ours", etc.
>
> The big downside here, of course, is that it's showing the diff for the
> whole file, not just one hunk (on the other hand, I often find the
> trickiest conflicts are ones where the changes unexpectedly span
> multiple hunks).

Yup, I often find myself comparing the base part (lines between |||
and ===) with our part (lines between <<< and |||) and their part
(lines between === and >>>) while looking at the diff3 output to see
what unique change each side did, in order to come up with a
conflict resolution.

I do this often enough to wonder if I should write a small "filter"
that I can pipe a whole "diff3" <<< ... ||| ... === ... >>> region
to and convert it into to diffs, but not often enough to motivate
me to actually write one ;-).

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22  5:50 ` Johannes Sixt
  2020-07-22  7:45   ` Jeff King
@ 2020-07-22 20:17   ` Sergey Organov
  2020-07-22 21:14     ` Junio C Hamano
  2020-07-22 22:48   ` Bono Stebler
  2 siblings, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-07-22 20:17 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: B. Stebler, git

Johannes Sixt <j6t@kdbg.org> writes:

> Am 22.07.20 um 01:29 schrieb B. Stebler:
>> I have been looking for a tool to display merge conflicts, that instead
>> of showing the two versions of the conflicting section, would show the
>> diff for that section in both conflicting commits.
>
> Perhaps you want to configure `merge.conflictStyle=diff3`?

Is there 'git merge' command-line option for that? I failed to find one.

Thanks,
-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22 20:17   ` Sergey Organov
@ 2020-07-22 21:14     ` Junio C Hamano
  2020-07-22 21:20       ` Sergey Organov
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2020-07-22 21:14 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Johannes Sixt, B. Stebler, git

Sergey Organov <sorganov@gmail.com> writes:

> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Am 22.07.20 um 01:29 schrieb B. Stebler:
>>> I have been looking for a tool to display merge conflicts, that instead
>>> of showing the two versions of the conflicting section, would show the
>>> diff for that section in both conflicting commits.
>>
>> Perhaps you want to configure `merge.conflictStyle=diff3`?
>
> Is there 'git merge' command-line option for that? I failed to find one.

There isn't, unless you count

    $ git -c merge.conflictstyle=diff3 merge side-branch

as a "command line option" (which may technically is).

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22 21:14     ` Junio C Hamano
@ 2020-07-22 21:20       ` Sergey Organov
  2020-07-23 18:26         ` Jeff King
  0 siblings, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-07-22 21:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, B. Stebler, git

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> Am 22.07.20 um 01:29 schrieb B. Stebler:
>>>> I have been looking for a tool to display merge conflicts, that instead
>>>> of showing the two versions of the conflicting section, would show the
>>>> diff for that section in both conflicting commits.
>>>
>>> Perhaps you want to configure `merge.conflictStyle=diff3`?
>>
>> Is there 'git merge' command-line option for that? I failed to find one.
>
> There isn't, unless you count
>
>     $ git -c merge.conflictstyle=diff3 merge side-branch
>
> as a "command line option" (which may technically is).

Yeah, I thought of maybe making an alias for that, so apparently I do
count it as command line option, thanks!

-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22  5:50 ` Johannes Sixt
  2020-07-22  7:45   ` Jeff King
  2020-07-22 20:17   ` Sergey Organov
@ 2020-07-22 22:48   ` Bono Stebler
  2 siblings, 0 replies; 25+ messages in thread
From: Bono Stebler @ 2020-07-22 22:48 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git

Thank you for your reply, and everyone else in the thread. I've played 
with diff3 and while it is slightly better in some cases, I feel there 
is a much better solution. To illustrate:

Commit 1: https://i.snipboard.io/Ssm7M9.jpg
Commit 2: https://i.snipboard.io/l2pqdT.jpg
Diff3: https://i.snipboard.io/teb1RS.jpg

What I would actually want: https://i.snipboard.io/8dOHgV.jpg

After looking at it and some research, I've managed to mostly automate 
it. You need to manually set the filename, which isn't as convenient as 
having the details directly in the file, but it beats rummaging in 
commits manually by a lot:

# Patch failed at 0001 smol commit

FILENAME="conflicting.txt"

THEIRS=$(head -1 .git/rebase-apply/0001 | awk '{ print $2 }')

OURS=$(git rev-parse HEAD)

THEIRS_DIFF=$(git -c color.ui=always show -U0 $THEIRS $FILENAME | tail 
-n +12)

OURS_DIFF=$(git -c color.ui=always show -U0 $OURS $FILENAME | tail -n +12)

printf "<<<<<<<<\n$OURS_DIFF\n========\n$THEIRS_DIFF\n>>>>>>>>\n"

Voilà, feel free to use that or suggest improvements, I am by no means a 
bash or git wizard!

Cheers,
B

On 22/07/2020 07:50, Johannes Sixt wrote:
> Am 22.07.20 um 01:29 schrieb B. Stebler:
>> I have been looking for a tool to display merge conflicts, that instead
>> of showing the two versions of the conflicting section, would show the
>> diff for that section in both conflicting commits.
> Perhaps you want to configure `merge.conflictStyle=diff3`? It does not
> exactly show a diff, but it writes the base version of the conflicted
> part in addition to "ours" and "theirs".
>
> -- Hannes

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22 17:26     ` Junio C Hamano
@ 2020-07-23 18:25       ` Jeff King
  2020-07-24  1:19         ` Junio C Hamano
  2021-01-16  2:50         ` Martin von Zweigbergk
  0 siblings, 2 replies; 25+ messages in thread
From: Jeff King @ 2020-07-23 18:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, B. Stebler, git

On Wed, Jul 22, 2020 at 10:26:04AM -0700, Junio C Hamano wrote:

> > The big downside here, of course, is that it's showing the diff for the
> > whole file, not just one hunk (on the other hand, I often find the
> > trickiest conflicts are ones where the changes unexpectedly span
> > multiple hunks).
> 
> Yup, I often find myself comparing the base part (lines between |||
> and ===) with our part (lines between <<< and |||) and their part
> (lines between === and >>>) while looking at the diff3 output to see
> what unique change each side did, in order to come up with a
> conflict resolution.
> 
> I do this often enough to wonder if I should write a small "filter"
> that I can pipe a whole "diff3" <<< ... ||| ... === ... >>> region
> to and convert it into to diffs, but not often enough to motivate
> me to actually write one ;-).

I would definitely have found that useful before (usually when one side
made a tiny one-line change and the other side deleted or drastically
changed a huge chunk).

It might even be possible to stuff it into xdiff's fill_conflict_hunk().
We have all of the data there, and xdiff in theory can make diffs. :) It
might be easier to prototype it as an external filter, though.

Something like the script below seems to work; you can run whole files
through it, or do something like ":10,20r!perl foo.pl" in vim to filter
a snippet. I won't be at all surprised if somebody more familiar with
vim tells me that you can already do something way better than this
(I've always found vimdiff pretty confusing).

-- >8 --
#!/usr/bin/perl

use File::Temp;
use strict;

my (@base, @ours, @theirs);
my $state;

sub flush {
  print @ours;
  print "|||||||\n";
  show_diff(base => \@base, ours => \@ours);
  print "|||||||\n";
  show_diff(base => \@base, theirs => \@theirs);
  print "=======\n";
  print @theirs;
  @ours = @base = @theirs = ();
}

sub show_diff {
  my ($pre_name, $pre_data, $post_name, $post_data) = @_;

  my $pre = File::Temp->new;
  print $pre @$pre_data;
  $pre->flush;

  my $post = File::Temp->new;
  print $post @$post_data;
  $post->flush;

  open(my $diff, '-|', qw(diff -u), $pre->filename, $post->filename);
  # throw away file header, which just mentions tempfiles, and replace
  # it with our own
  <$diff>; <$diff>;
  print "--- $pre_name\n";
  print "+++ $post_name\n";
  while (<$diff>) {
    print;
  }
}

sub state_none {
  if (/^<{7}/) { $state = \&state_ours }
  print
}

sub state_ours {
  if (/^\|{7}/) { $state = \&state_base }
  else { push @ours, $_ }
}

sub state_base {
  if (/^={7}/) { $state = \&state_theirs }
  else { push @base, $_ }
}

sub state_theirs {
  if (/^>{7}/) { flush(); print; $state = \&state_none }
  else { push @theirs, $_ }
}

$state = \&state_none;
while (<>) {
  $state->();
}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-22 21:20       ` Sergey Organov
@ 2020-07-23 18:26         ` Jeff King
  2020-07-23 19:11           ` Sergey Organov
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2020-07-23 18:26 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Junio C Hamano, Johannes Sixt, B. Stebler, git

On Thu, Jul 23, 2020 at 12:20:46AM +0300, Sergey Organov wrote:

> >> Is there 'git merge' command-line option for that? I failed to find one.
> >
> > There isn't, unless you count
> >
> >     $ git -c merge.conflictstyle=diff3 merge side-branch
> >
> > as a "command line option" (which may technically is).
> 
> Yeah, I thought of maybe making an alias for that, so apparently I do
> count it as command line option, thanks!

You can also do it after "git merge" aborts with conflicts by running:

  git checkout --conflict=diff3 my-file

but do note that it will check out from the index, overwriting any
resolution you've already done in that file.

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-23 18:26         ` Jeff King
@ 2020-07-23 19:11           ` Sergey Organov
  2020-07-23 21:39             ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-07-23 19:11 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Johannes Sixt, B. Stebler, git

Jeff King <peff@peff.net> writes:

> On Thu, Jul 23, 2020 at 12:20:46AM +0300, Sergey Organov wrote:
>
>> >> Is there 'git merge' command-line option for that? I failed to find one.
>> >
>> > There isn't, unless you count
>> >
>> >     $ git -c merge.conflictstyle=diff3 merge side-branch
>> >
>> > as a "command line option" (which may technically is).
>> 
>> Yeah, I thought of maybe making an alias for that, so apparently I do
>> count it as command line option, thanks!
>
> You can also do it after "git merge" aborts with conflicts by running:
>
>   git checkout --conflict=diff3 my-file
>
> but do note that it will check out from the index, overwriting any
> resolution you've already done in that file.

Really nice trick, thanks for sharing!

Though now it gets really odd "git merge" itself doesn't have this
option.

-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-23 19:11           ` Sergey Organov
@ 2020-07-23 21:39             ` Junio C Hamano
  2020-07-24  5:15               ` Jacob Keller
  2020-07-24  6:53               ` Sergey Organov
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-07-23 21:39 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Jeff King, Johannes Sixt, B. Stebler, git

Sergey Organov <sorganov@gmail.com> writes:

>> You can also do it after "git merge" aborts with conflicts by running:
>>
>>   git checkout --conflict=diff3 my-file
>>
>> but do note that it will check out from the index, overwriting any
>> resolution you've already done in that file.
>
> Though now it gets really odd "git merge" itself doesn't have this
> option.

A command line option is cumbersome that you have to type it every
time, so configuration variable makes 100% more sense than an option
to "git merge".  

If your merge used the merge (as opposed to diff3) style, and seeing
that the resulting conflict is not easy to review and you wish you
used diff3 style instead, it is way too late for any option to "git
merge" to help you.

But having an option to "git checkout" lets you move forward from
that state, so it also makes 100% more sense than an option to "git
merge".

So, it is not odd at all.  Just compare between merge and diff3,
think which one would often help you, configure to use it by default,
*and* at a rare occasion where the chosen default does not work for
you, let "checkout" help you.  The thing is, unless you first attempt
to "git merge", you won't know what shape of conflict you would get,
so you cannot choose the right conflict style command line option,
even if one were available.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-23 18:25       ` Jeff King
@ 2020-07-24  1:19         ` Junio C Hamano
  2020-07-24 19:48           ` Jeff King
  2021-01-16  2:50         ` Martin von Zweigbergk
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2020-07-24  1:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, B. Stebler, git

Jeff King <peff@peff.net> writes:

> sub flush {
>   print @ours;
>   print "|||||||\n";
>   show_diff(base => \@base, ours => \@ours);
>   print "|||||||\n";
>   show_diff(base => \@base, theirs => \@theirs);
>   print "=======\n";

Before this gets called, "<<<<<<<\n" from the original has been
emitted to the output, so this shows

	<<<<<<<
	version from ours
	|||||||
	output from diff -u base ours
	|||||||
	output from diff -u base theirs
	=======
	version from theirs

   print @theirs;
>   @ours = @base = @theirs = ();
> }

Unfortunately, there is no ">>>>>>>" shown with this code, as $_
seems to get clobbered before the sub returns, so ...

> sub state_theirs {
>   if (/^>{7}/) { flush(); print; $state = \&state_none }
>   else { push @theirs, $_ }
> }

... that "print" does not do what we want it to do.

Localizing the $_ upfront in the show_diff sub should probably be
sufficient.  That is ...

> sub show_diff {
>   my ($pre_name, $pre_data, $post_name, $post_data) = @_;

+  local ($_);

... here.

>
>   my $pre = File::Temp->new;
>   print $pre @$pre_data;

I am debating myself if I want to see the base version between these
two extra diffs.  Perhaps there is no need, as either one of these
two extra diffs should be sufficient to see what was in the base
version.

Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-23 21:39             ` Junio C Hamano
@ 2020-07-24  5:15               ` Jacob Keller
  2020-07-24  6:01                 ` Junio C Hamano
  2020-07-24  6:53               ` Sergey Organov
  1 sibling, 1 reply; 25+ messages in thread
From: Jacob Keller @ 2020-07-24  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Sergey Organov, Jeff King, Johannes Sixt, B. Stebler, Git mailing list

On Thu, Jul 23, 2020 at 2:43 PM Junio C Hamano <gitster@pobox.com> wrote:
> If your merge used the merge (as opposed to diff3) style, and seeing
> that the resulting conflict is not easy to review and you wish you
> used diff3 style instead, it is way too late for any option to "git
> merge" to help you.
>
> But having an option to "git checkout" lets you move forward from
> that state, so it also makes 100% more sense than an option to "git
> merge".
>

Perhaps the issue is just that it's not discoverable easily because
it's a different command.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-24  5:15               ` Jacob Keller
@ 2020-07-24  6:01                 ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-07-24  6:01 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Sergey Organov, Jeff King, Johannes Sixt, B. Stebler, Git mailing list

Jacob Keller <jacob.keller@gmail.com> writes:

> On Thu, Jul 23, 2020 at 2:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>> If your merge used the merge (as opposed to diff3) style, and seeing
>> that the resulting conflict is not easy to review and you wish you
>> used diff3 style instead, it is way too late for any option to "git
>> merge" to help you.
>>
>> But having an option to "git checkout" lets you move forward from
>> that state, so it also makes 100% more sense than an option to "git
>> merge".
>
> Perhaps the issue is just that it's not discoverable easily because
> it's a different command.

That may be true.

By the way, "git checkout --conflict=<style>" is a short-hand for
"git checkout --merge --conflict=<style>" and it is quite useful not
just in the said case of "oops, I cannot read this conflict with the
'merge' style---let's switch to 'diff3' style".  After starting to
attempt resolving the conflict, sometimes I become unsure if the
resolution I've been working on is correct, and want to start from
scratch.  In such a case, even without switching the style to a
different one, "git checkout --merge" to discard the changes I made
in the working tree file and reproduce the auto-merged state with
conflict markers, is a good tool in your toolbox to know (being able
to give a different conflict style is merely a natural extension of
the feature).

Perhaps in Documentation/git-merge.txt there can be a mention of

	When you really screwed up your resolution, you could use
	"git checkout --merge -- $paths" to revert selected paths
	back to the state just before the auto-merge gave up and
	asked your help in resolving.  The --conflict=<style> option
	can be used instead of the "--merge" option in the command
	to use a different conflict marking style.  See
	git-checkout[1] for details.

or something along that line.  People who are unware of "checkout -m"
in such a situation may run "git reset --hard" and redo the whole
merge from scratch, but you do not have to discard the resolution
you made in other paths successfully only to redo a few files that
you botched.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-23 21:39             ` Junio C Hamano
  2020-07-24  5:15               ` Jacob Keller
@ 2020-07-24  6:53               ` Sergey Organov
  2020-07-24 20:30                 ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-07-24  6:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Sixt, B. Stebler, git

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> You can also do it after "git merge" aborts with conflicts by running:
>>>
>>>   git checkout --conflict=diff3 my-file
>>>
>>> but do note that it will check out from the index, overwriting any
>>> resolution you've already done in that file.
>>
>> Though now it gets really odd "git merge" itself doesn't have this
>> option.
>
> A command line option is cumbersome that you have to type it every
> time, so configuration variable makes 100% more sense than an option
> to "git merge".

Fortunately Git already supports configuration variable, that we both
agree is a nice thing to have, so I won't have to type the option every
time, no.

> If your merge used the merge (as opposed to diff3) style, and seeing
> that the resulting conflict is not easy to review and you wish you
> used diff3 style instead, it is way too late for any option to "git
> merge" to help you.

  $ git merge --abort
  $ git merge --conflict=diff3 side-branch

or, say, entirely imaginary:

  $ git merge --redo --conflict=diff3 side-branch -- my-file

if merge had --redo option and path limiting support, that could be
handy for other reasons as well, as I have already pointed elsewhere and
you disagreed, but still.

>
> But having an option to "git checkout" lets you move forward from
> that state, so it also makes 100% more sense than an option to "git
> merge".

Actually, "git checkout" is not the place where I'd expect to find this
feature in the first place, so to me it's rather already 99%
illogical.

If "git merge" is what gave me the original result, it's some "git
merge" variant that I'd expect to give me modified result as well.

Yeah, I can see how Git machinery is an excuse for "git checkout" to end
up being used for this feature, but only after I learned it is.

> So, it is not odd at all.  Just compare between merge and diff3,
> think which one would often help you, configure to use it by default,
> *and* at a rare occasion where the chosen default does not work for
> you, let "checkout" help you.  The thing is, unless you first attempt
> to "git merge", you won't know what shape of conflict you would get,
> so you cannot choose the right conflict style command line option,
> even if one were available.

What looks odd to me is that for "git merge" I need to use:

   $ git -c merge.conflictstyle=diff3 merge side-branch

as you yourself pointed, whereas for "git checkout" I can use:

   $ git checkout --conflict=diff3 my-file

even though it could have been:

   $ git -c merge.conflictstyle=diff3 checkout -m my-file

as far as I can tell. A minor inconsistency, but still.

If I got your arguments right, you think that having short-cut option
for "git checkout" makes sense, while having similar one for "git merge"
doesn't, whereas my argument is that either both make sense, or both
don't.

Anyway, it's nice we can still do these things, one way or another.

Thanks,
-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-24  1:19         ` Junio C Hamano
@ 2020-07-24 19:48           ` Jeff King
  2020-07-24 20:18             ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2020-07-24 19:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, B. Stebler, git

On Thu, Jul 23, 2020 at 06:19:19PM -0700, Junio C Hamano wrote:

> Unfortunately, there is no ">>>>>>>" shown with this code, as $_
> seems to get clobbered before the sub returns, so ...

Ah yeah, thanks for pointing it out (as you might guess I was mainly
looking at the diff output to see if it was broken ;) ).

Your suggested fix is probably what I'd do (I really wish perl localized
$_ automatically when entering new blocks, but I suspect they would
never do so because of compatibility).

> >   my $pre = File::Temp->new;
> >   print $pre @$pre_data;
> 
> I am debating myself if I want to see the base version between these
> two extra diffs.  Perhaps there is no need, as either one of these
> two extra diffs should be sufficient to see what was in the base
> version.

Yeah, I didn't give too much thought to the output, but was afraid of
making it too long and hard to read. In addition to whether to show the
base, I also wondered whether we could omit the "---/+++" header for
each diff. It's mostly boilerplate, and the signal of "hey, this is the
diff between 'base' and 'ours'" could be done on the "|||||||" line
(which also could potentially use a different character to be more
distinct).

I was also tempted to remove the hunk header since the line numbers are
fairly useless. But it is possible for the diff to have multiple hunks,
and you'd still want to show that.

My big challenge now will be remembering to try to use this next time I
run into a conflict that could be helped by it. :)

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-24 19:48           ` Jeff King
@ 2020-07-24 20:18             ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-07-24 20:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, B. Stebler, git

Jeff King <peff@peff.net> writes:

> My big challenge now will be remembering to try to use this next time I
> run into a conflict that could be helped by it. :)

Yup, I installed it in ~/bin so that I can pipe diff3 result to it
for the next time.  Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-24  6:53               ` Sergey Organov
@ 2020-07-24 20:30                 ` Junio C Hamano
  2020-07-24 22:11                   ` Sergey Organov
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2020-07-24 20:30 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Jeff King, Johannes Sixt, B. Stebler, git

Sergey Organov <sorganov@gmail.com> writes:

>> If your merge used the merge (as opposed to diff3) style, and seeing
>> that the resulting conflict is not easy to review and you wish you
>> used diff3 style instead, it is way too late for any option to "git
>> merge" to help you.
>
>   $ git merge --abort
>   $ git merge --conflict=diff3 side-branch
>
> or, say, entirely imaginary:
>
>   $ git merge --redo --conflict=diff3 side-branch -- my-file
>
> if merge had --redo option and path limiting support, that could be
> handy for other reasons as well, as I have already pointed elsewhere and
> you disagreed, but still.

You are ignoring the case where you may have successfully resolved
conflicts in some paths before you noticed conflicts in some
particular files are hard to see in one style and wish if you used
the other style, I think.

Surely, you can reset everything away and redo it from scratch,
which is what all of the above is, but then you would need a way to
stash away the successful half resolution so far before discarding
them.  Compared to that, "ouch, I screwed up and want a freshly
conflicted state back for these paths" would allow you revert only
the botched paths without discarding the work you have already done.

> Actually, "git checkout" is not the place where I'd expect to find this
> feature in the first place, so to me it's rather already 99%
> illogical.

One half of the "checkout" (which now exists as a synonym "restore")
is to update the working tree files out of various sources, and
"conflicted stages in the index" is one of them, so it entirely is
natural and logical home for the feature.

The documentation needs updating to help you and others feel it
natural, I would think.  This seems to be mostly the matter of
better education.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-24 20:30                 ` Junio C Hamano
@ 2020-07-24 22:11                   ` Sergey Organov
  2020-07-24 23:04                     ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Sergey Organov @ 2020-07-24 22:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Sixt, B. Stebler, git

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> If your merge used the merge (as opposed to diff3) style, and seeing
>>> that the resulting conflict is not easy to review and you wish you
>>> used diff3 style instead, it is way too late for any option to "git
>>> merge" to help you.
>>
>>   $ git merge --abort
>>   $ git merge --conflict=diff3 side-branch
>>
>> or, say, entirely imaginary:
>>
>>   $ git merge --redo --conflict=diff3 side-branch -- my-file
>>
>> if merge had --redo option and path limiting support, that could be
>> handy for other reasons as well, as I have already pointed elsewhere and
>> you disagreed, but still.
>
> You are ignoring the case where you may have successfully resolved
> conflicts in some paths before you noticed conflicts in some
> particular files are hard to see in one style and wish if you used
> the other style, I think.

I believe I'm not, or if I do, then you are ignoring the opposite case
that you may need to redo entire merge with different merge style
output.

In other words, I only aimed at decreasing 100% down to at most 80% in
your claim:

>>> so configuration variable makes 100% more sense than an option
>>> to "git merge".

because you can only claim 100% if nobody ever needs to redo the merge
from scratch, that is obviously not the case.

> Surely, you can reset everything away and redo it from scratch,
> which is what all of the above is,

No, only first two commands are reset and redo everything from scratch,
whereas the third command supposedly only affects 'my-file' path:

  $ git merge --redo --conflict=diff3 side-branch -- my-file

Anyway, my primary point was that I still might wish to do exactly reset
and start from scratch, and then I miss --conflict option, that in turn
makes its existence less than 100% less sense than configuration
variable, my estimation being about 80%.

> but then you would need a way to stash away the successful half
> resolution so far before discarding them. Compared to that, "ouch, I
> screwed up and want a freshly conflicted state back for these paths"
> would allow you revert only the botched paths without discarding the
> work you have already done.
>
>> Actually, "git checkout" is not the place where I'd expect to find this
>> feature in the first place, so to me it's rather already 99%
>> illogical.
>
> One half of the "checkout" (which now exists as a synonym "restore")
> is to update the working tree files out of various sources, and
> "conflicted stages in the index" is one of them, so it entirely is
> natural and logical home for the feature.

I believe I already agreed it makes sense the feature ends up being
there, and I perfectly understand this /after/ I learned it's there, but
I'm still afraid I'd not figure out to look for it there in the first
place.

> The documentation needs updating to help you and others feel it
> natural, I would think.  This seems to be mostly the matter of
> better education.

Yeah, it's education that helps most when things get unintuitive enough.

Better documentation always helps indeed, and description of --conflict
option in "man git-merge" would be the best place to put a reference to
"git checkout -m" (or should it rather be "git restore -m" nowadays?)
that you've suggested elsewhere in this thread ;-)

Thanks,
-- Sergey

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-24 22:11                   ` Sergey Organov
@ 2020-07-24 23:04                     ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2020-07-24 23:04 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Jeff King, Johannes Sixt, B. Stebler, git

Sergey Organov <sorganov@gmail.com> writes:

> Anyway, my primary point was that I still might wish to do exactly reset
> and start from scratch, and then I miss --conflict option, that in turn
> makes its existence less than 100% less sense than configuration
> variable, my estimation being about 80%.

As with other uses of "update working tree files from elsewhere" use
of "checkout", "--merge/--conflict=<style>" checkout does take
pathspec to specify what paths are to be updated.  Nothing stops you
to give "." (everything under the sun) pathspec there, so I do not
really see the point of insisting "I want to start and redo from
scratch".  

Unless the reason is "because that is the way I am used to", that
is.  Surely, we can start and redo from scratch any operation, not
limited to merges, by removing the entire repository and cloning it
again from scratch.  I am trying to give our users tools to help
them not get into that habit and instead always make forward
progress without discarding work that has already been done.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2020-07-23 18:25       ` Jeff King
  2020-07-24  1:19         ` Junio C Hamano
@ 2021-01-16  2:50         ` Martin von Zweigbergk
  2021-01-21 14:28           ` Jeff King
  1 sibling, 1 reply; 25+ messages in thread
From: Martin von Zweigbergk @ 2021-01-16  2:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Johannes Sixt, B. Stebler, git

On Thu, Jul 23, 2020 at 8:27 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Jul 22, 2020 at 10:26:04AM -0700, Junio C Hamano wrote:
>
> > > The big downside here, of course, is that it's showing the diff for the
> > > whole file, not just one hunk (on the other hand, I often find the
> > > trickiest conflicts are ones where the changes unexpectedly span
> > > multiple hunks).
> >
> > Yup, I often find myself comparing the base part (lines between |||
> > and ===) with our part (lines between <<< and |||) and their part
> > (lines between === and >>>) while looking at the diff3 output to see
> > what unique change each side did, in order to come up with a
> > conflict resolution.
> >
> > I do this often enough to wonder if I should write a small "filter"
> > that I can pipe a whole "diff3" <<< ... ||| ... === ... >>> region
> > to and convert it into to diffs, but not often enough to motivate
> > me to actually write one ;-).
>
> I would definitely have found that useful before (usually when one side
> made a tiny one-line change and the other side deleted or drastically
> changed a huge chunk).

FYI, I added something similar to Mercurial recently. Instead of two
diffs, it shows one snapshot and one diff. See
https://phab.mercurial-scm.org/D9551 for details. I've used it for a
few weeks and it seems to be working pretty well. The drawback is
mostly when you want to keep the side with the diff and ignore the
other side, since you'll then have to drop the lines prefixed with "-"
and then enter column-selection mode or something and delete the first
character on each remaining line.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2021-01-16  2:50         ` Martin von Zweigbergk
@ 2021-01-21 14:28           ` Jeff King
  2021-01-21 20:30             ` Martin von Zweigbergk
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2021-01-21 14:28 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Junio C Hamano, Johannes Sixt, B. Stebler, git

On Fri, Jan 15, 2021 at 04:50:08PM -1000, Martin von Zweigbergk wrote:

> > > I do this often enough to wonder if I should write a small "filter"
> > > that I can pipe a whole "diff3" <<< ... ||| ... === ... >>> region
> > > to and convert it into to diffs, but not often enough to motivate
> > > me to actually write one ;-).
> >
> > I would definitely have found that useful before (usually when one side
> > made a tiny one-line change and the other side deleted or drastically
> > changed a huge chunk).
> 
> FYI, I added something similar to Mercurial recently. Instead of two
> diffs, it shows one snapshot and one diff. See
> https://phab.mercurial-scm.org/D9551 for details. I've used it for a
> few weeks and it seems to be working pretty well. The drawback is
> mostly when you want to keep the side with the diff and ignore the
> other side, since you'll then have to drop the lines prefixed with "-"
> and then enter column-selection mode or something and delete the first
> character on each remaining line.

I've used the script I posted earlier in the thread several times in the
last 6 months or so, by replacing the conflict markers in the file I'm
resolving with the new output (basically "%!magic-diff3" in vim).

It is helpful. My biggest complaint is cleaning up the diff from the
marker after viewing it. In most cases where it's helpful, one side made
a large change (say, deleting or moving a big chunk of code) and the
other made a small one (tweaking one line in the moved chunk). The small
diff is useful, but the big one is not. And then after having viewed it,
I have to remove the whole big diff in my editor.

(It sounds like yours _replaces_ the conflict marker with the diff,
which is why you have to edit the diff. Mine is showing it in addition,
so you have to delete the diff).

I think rather than thinking of these as expanded conflict markers, it
would probably be a more useful workflow to just look at the diff in a
separate command (so just show the conflicts, not everything else, and
just show the diff). I suspect it could be made pretty nice with some
simple editor support (e.g., open a new buffer in the editor showing the
diff for just the current hunk, or even the current _half_ of the hunk
you're on).

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2021-01-21 14:28           ` Jeff King
@ 2021-01-21 20:30             ` Martin von Zweigbergk
  2021-01-21 21:08               ` Jeff King
  0 siblings, 1 reply; 25+ messages in thread
From: Martin von Zweigbergk @ 2021-01-21 20:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Johannes Sixt, B. Stebler, git

On Thu, Jan 21, 2021 at 4:28 AM Jeff King <peff@peff.net> wrote:
>
> On Fri, Jan 15, 2021 at 04:50:08PM -1000, Martin von Zweigbergk wrote:
>
> > > > I do this often enough to wonder if I should write a small "filter"
> > > > that I can pipe a whole "diff3" <<< ... ||| ... === ... >>> region
> > > > to and convert it into to diffs, but not often enough to motivate
> > > > me to actually write one ;-).
> > >
> > > I would definitely have found that useful before (usually when one side
> > > made a tiny one-line change and the other side deleted or drastically
> > > changed a huge chunk).
> >
> > FYI, I added something similar to Mercurial recently. Instead of two
> > diffs, it shows one snapshot and one diff. See
> > https://phab.mercurial-scm.org/D9551 for details. I've used it for a
> > few weeks and it seems to be working pretty well. The drawback is
> > mostly when you want to keep the side with the diff and ignore the
> > other side, since you'll then have to drop the lines prefixed with "-"
> > and then enter column-selection mode or something and delete the first
> > character on each remaining line.
>
> I've used the script I posted earlier in the thread several times in the
> last 6 months or so, by replacing the conflict markers in the file I'm
> resolving with the new output (basically "%!magic-diff3" in vim).
>
> It is helpful. My biggest complaint is cleaning up the diff from the
> marker after viewing it. In most cases where it's helpful, one side made
> a large change (say, deleting or moving a big chunk of code) and the
> other made a small one (tweaking one line in the moved chunk). The small
> diff is useful, but the big one is not. And then after having viewed it,
> I have to remove the whole big diff in my editor.
>
> (It sounds like yours _replaces_ the conflict marker with the diff,
> which is why you have to edit the diff. Mine is showing it in addition,
> so you have to delete the diff).

Yes, that's correct. It replaces the base and one side of the conflict
marker by a diff (and leaves the other side as a snapshot).

> I think rather than thinking of these as expanded conflict markers, it
> would probably be a more useful workflow to just look at the diff in a
> separate command (so just show the conflicts, not everything else, and
> just show the diff). I suspect it could be made pretty nice with some
> simple editor support (e.g., open a new buffer in the editor showing the
> diff for just the current hunk, or even the current _half_ of the hunk
> you're on).

At some point it seems better to delegate to a proper merge tool. You
said that you use vim, so I'm a little surprised that you use conflict
markers instead of using vimdiff. I don't use vim and I've never
really used vimdiff. I still use conflict markers, mostly out of
habit, but also because I usually run in a tmux session on a remote
machine. I feel like I should try to switch to meld.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Improving merge of tricky conflicts
  2021-01-21 20:30             ` Martin von Zweigbergk
@ 2021-01-21 21:08               ` Jeff King
  0 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2021-01-21 21:08 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Junio C Hamano, Johannes Sixt, B. Stebler, git

On Thu, Jan 21, 2021 at 10:30:36AM -1000, Martin von Zweigbergk wrote:

> > I think rather than thinking of these as expanded conflict markers, it
> > would probably be a more useful workflow to just look at the diff in a
> > separate command (so just show the conflicts, not everything else, and
> > just show the diff). I suspect it could be made pretty nice with some
> > simple editor support (e.g., open a new buffer in the editor showing the
> > diff for just the current hunk, or even the current _half_ of the hunk
> > you're on).
> 
> At some point it seems better to delegate to a proper merge tool. You
> said that you use vim, so I'm a little surprised that you use conflict
> markers instead of using vimdiff. I don't use vim and I've never
> really used vimdiff. I still use conflict markers, mostly out of
> habit, but also because I usually run in a tmux session on a remote
> machine. I feel like I should try to switch to meld.

Yeah, I think your first sentence might be the most important takeaway. ;)

I have tried using vimdiff in the past, but didn't really like it. My
recollection is that it was clunky to navigate, and I could fix most
conflicts much faster just by looking at them. But I have never been a
heavy user of the multi-window multi-buffer stuff in vim. My "open a new
buffer in the editor" is probably a lie; for me it is more like "open a
new terminal and run a command at the shell". :)

-Peff

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2021-01-21 21:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21 23:29 Improving merge of tricky conflicts B. Stebler
2020-07-22  5:50 ` Johannes Sixt
2020-07-22  7:45   ` Jeff King
2020-07-22 17:26     ` Junio C Hamano
2020-07-23 18:25       ` Jeff King
2020-07-24  1:19         ` Junio C Hamano
2020-07-24 19:48           ` Jeff King
2020-07-24 20:18             ` Junio C Hamano
2021-01-16  2:50         ` Martin von Zweigbergk
2021-01-21 14:28           ` Jeff King
2021-01-21 20:30             ` Martin von Zweigbergk
2021-01-21 21:08               ` Jeff King
2020-07-22 20:17   ` Sergey Organov
2020-07-22 21:14     ` Junio C Hamano
2020-07-22 21:20       ` Sergey Organov
2020-07-23 18:26         ` Jeff King
2020-07-23 19:11           ` Sergey Organov
2020-07-23 21:39             ` Junio C Hamano
2020-07-24  5:15               ` Jacob Keller
2020-07-24  6:01                 ` Junio C Hamano
2020-07-24  6:53               ` Sergey Organov
2020-07-24 20:30                 ` Junio C Hamano
2020-07-24 22:11                   ` Sergey Organov
2020-07-24 23:04                     ` Junio C Hamano
2020-07-22 22:48   ` Bono Stebler

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git