git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [RFC/WIP PATCH 0/3] Modifications to handling of non-textual file merge conflicts
Date: Thu, 9 Aug 2018 11:51:24 -0700	[thread overview]
Message-ID: <CABPp-BE23fzBPuR94Ukrn4Va_Aw2wsZ0DjjxRxbSYJLmCrvP6w@mail.gmail.com> (raw)
In-Reply-To: <xmqq600jctr1.fsf@gitster-ct.c.googlers.com>

On Thu, Aug 9, 2018 at 10:52 AM Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > 1) Representative example: A modify/delete conflict; the path in question
> > in the working tree would have conflict information at the top of the file
> > followed by the normal file contents; thus it could be of the form:
> >
> >     <<<<<<<< HEAD
> >     Conflict hint: This block of text was not part of the original
> >     branch; it serves instead to hint about non-textual conflicts:
> >       MODIFY/DELETE: path foo modified in HEAD and deleted in BRANCH
> >     ========
> >     Conflict hint: This block of text was not part of the original
> >     branch; it serves instead to hint about non-textual conflicts:
> >       MODIFY/DELETE: path foo modified in HEAD and deleted in BRANCH
> >     >>>>>>>> BRANCH
> >     Lorem ipsum dolor sit amet, consectetuer sadipscing elitr,
> >     sed diam nonumy eirmod tempor invidunt ut labore et dolore
> >     magna aliquyam erat, sed diam voluptua. At vero eos et
> >     accusam et justo duo dolores et ea rebum. Stet clita kasd
> >     gubergren, no sea takimata sanctus est Lorem ipsum dolor
> >     sit amet.
> >
> > Alternative ideas for handling the explanatory text here are welcome.
>
> In a modify/delete conflict, we currently do not leave any in-file
> clue, so smudging the modified side like this might be a change that
> helps those who "grep e '<<<<<<<'" to find the set of paths that
> need to be examined.

Yes, that's one of the things I'm hoping for with this change.

>  I personally do not feel it would be all that
> useful, as "ls-files -u" is how I'd learn about these paths.

"ls-files -u" is really nice; I love it and try to show it to others,
but keep getting surprised by how many people are surprised to learn
of its existence.

Further, "ls-files -u" may be insufficient even for those who know
about it, for two reasons (and admittedly, I care a lot more about the
second than the first):

* There are more conflict types than the number of permutations of
stages.  For example, how do you know if a file was from a
rename/delete or a modify/delete conflict?  And if from a
rename/delete, how do you determine the original filename?  The index
doesn't store this information.  Granted those two conflict types are
at least similar, but other stage combinations might be more
confusing.  For example, if all three stages are present, is the
conflict in question an edit/edit, a rename/add/delete, or a D/F
conflict where the file came from a rename?

* Future feature: Thomas Rast' proposed remerge-diff[1] (which I want
to resurrect and remove the edge/corner cases from).  This worked by
creating what I call an auto-merged tree, where you just accept all
conflicts and commit.  Then you diff the merge commit to what the
auto-merge tree was to see how the user edited conflicts to create the
merge commit.  Problem is, for the auto-merge tree we won't have an
index anymore so how do we represent conflicts that aren't edit/edit
of normal text files?  My proposal here is an answer for that; I'm
open to others, but it's the best I came up with.

[1] https://public-inbox.org/git/cover.1409860234.git.tr@thomasrast.ch/

(I'll also note here that others have requested the ability to create
an as-merged-as-possibly tree including conflicts for other purposes;
see "API suggestion" of
https://public-inbox.org/git/CAFAcib-2fxiVxtVWcbvafY3-Br7Y70HMiHFZoT0VfK6JU0Dp9A@mail.gmail.com/
and my response noting the various difficulties with non-textual
conflicts.  So it may not be just the remerge-diff ability here.)

> What I would want to see when faced to a modify/delete conflict is
> how the modification side changed the contents, as the change, or
> its moral equivalent, would need to be ported to other locations in
> the context of the deleting side.  But I am not sure if it makes
> sense to attempt to somehow include "diff HEAD...MERGE_HEAD" (or the
> other way around) in the file to show the contents change on the
> modification side.

That's a good point; I'm not sure how to include that either.
Something to think about.

> > 2) Representative example: A binary edit/edit conflict.  In this case,
> > it would be inappropriate to put the conflict markers inside the
> > binary file.  Instead, we create another file (e.g. path~CONFLICTS)
> > and put conflict markers in it:
> >
> >     <<<<<<<< HEAD
> >     Conflict hint: This block of text was not part of the original
> >     branch; it serves instead to hint about non-textual conflicts:
> >       BINARY conflict: path foo modified in both branches
> >     ========
> >     Conflict hint: This block of text was not part of the original
> >     branch; it serves instead to hint about non-textual conflicts:
> >       BINARY conflict: path foo modified in both branches
> >     >>>>>>>> BRANCH
> >
> > This file would also be added to the index at stage 1 (so that 'git merge
> > --abort' would clean this file out instead of leaving it around untracked,
> > and also because 'git status' would report "deleted in both" which seems
> > reasonable).
> >
> > This type of example could apply for each of the following types of
> > conflicts:
> >   * binary edit/edit
> >   * any of the conflicts from type 1 when binary files are involved
> >   * symlink edit/edit (or add/add)
> >   * symlink/submodule
> >   * symlink/directory
> >   * directory/submodule
> >   * submodule/submodule
> >
> > It could also apply to the following new corner case conflict types from
> > directory rename detection:
> >   * N-way colliding paths (N>=2) due to directory renames
> >   * directory rename split; half renamed to one directory and half to another
>
> Hmph, I am starting to wonder if it may be easier to access if
> instead you did not touch any working tree file to do any of the
> above, and instead write a single file in $GIT_DIR/ to explain what
> kind of conflicts these paths are involved in.  That would probably
> give a better and easier-to-read summary than "ls-files -u" output.

That's interesting, and would be an alternate way to help some users.
I think it's less discoverable than sticking the info in files in the
working tree for the user to see (particularly for the users who are
allergic to git commands and just stay in their IDE as much as
possible), but there might be something interesting here.

However, I don't see how this would cover the remerge-diff usecase (or
more general create-as-merged-as-possible-tree-with-conflicts-in-it
usecases) at all, which was my biggest motivation for these changes.
Is there a good solution there?

> Or do we have _enough_ information in the "ls-files -u" already to
> infer "Ah, we are in symlink edit/edit conflict.", etc.?  If so,
> perhaps "git status" can be extended to show what kind of conflict
> these paths are in by reading the higher-stage index entries (and
> lack of stages thereof, when dealing with a conflict with deletion
> involved)?

We have enough information in _some_ cases to determine which type of
conflict it is from the index (e.g. because we don't detect renames
for symlinks or submodules and thus have fewer conflict types possible
involving those).  However, we again run into problems (at least for
binaries) about there being more conflict types than permutations of
stage entries, and for all conflicts, we run into the same problem of
not having an index at all for the remerge-diff capability.

      reply	other threads:[~2018-08-09 18:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-06 22:44 Two RFC/WIP series for facilitating merge conflict resolution Elijah Newren
2018-08-06 22:45 ` [RFC/WIP PATCH 0/1] Simplify handling of directory/file conflicts Elijah Newren
2018-08-06 22:45   ` [RFC/WIP PATCH 1/1] merge-recursive: make file/directory conflicts easier to resolve Elijah Newren
2018-08-09 17:36     ` Junio C Hamano
2018-08-09 19:26       ` Elijah Newren
2018-08-09 20:54         ` Junio C Hamano
2018-08-09 21:27           ` Elijah Newren
2018-08-06 22:47 ` [RFC/WIP PATCH 0/3] Modifications to handling of non-textual file merge conflicts Elijah Newren
2018-08-06 22:47   ` [RFC/WIP PATCH 1/3] rerere: avoid buffer overrun Elijah Newren
2018-08-06 22:47   ` [RFC/WIP PATCH 2/3] merge-recursive: fix handling of submodules in modify/delete conflicts Elijah Newren
2018-08-06 22:47   ` [RFC/WIP PATCH 3/3] merge-recursive: provide more conflict hints for non-textual conflicts Elijah Newren
2018-08-09 17:52   ` [RFC/WIP PATCH 0/3] Modifications to handling of non-textual file merge conflicts Junio C Hamano
2018-08-09 18:51     ` Elijah Newren [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BE23fzBPuR94Ukrn4Va_Aw2wsZ0DjjxRxbSYJLmCrvP6w@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [RFC/WIP PATCH 0/3] Modifications to handling of non-textual file merge conflicts' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).