git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Chris Torek via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Chris Torek <chris.torek@gmail.com>
Subject: Re: [PATCH] git-mv: improve error message for conflicted file
Date: Sat, 18 Jul 2020 10:46:25 -0700	[thread overview]
Message-ID: <xmqq8sfgd8cu.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <CABPp-BGDp_SjJKvi+XVd6KvRLA5PVsK4xBLPvBxAimDft+0M9g@mail.gmail.com> (Elijah Newren's message of "Fri, 17 Jul 2020 19:00:22 -0700")

Elijah Newren <newren@gmail.com> writes:

> Or, even better, make ce_stage(ce) not be an error; see
> https://lore.kernel.org/git/xmqqk1ozb6qy.fsf@gitster-ct.c.googlers.com/.

Hmph, I actually am not convinced with that one, even though I know
I wrote it, I do not think I had thought things through before I
did.  It may make sense in a narrow special case where there is a
single entry at a higher stage to rename it to the destination path
and drop it down to stage #0, but I do not think of a good behaviour
for more general cases.

What should happen, for example, if two or more entries at higher
stages exist for the path being moved?  Renaming all of them to the
destination path without changing their stage number may make more
sense [*1*].  At least, that solution still lets the user to choose
object at which stage [*2*] to use in the final resolution.

Now, if we define that "git mv src dst", when 'src' is a conflicted
path, moves the working tree file src to dst at the same time moves
all the higher stage entries for 'src' to 'dst' while retaining
their stage number, what does the degenerate case of having only one
such entry look like?  You start from the state where you have
$that_path at stage #3, "git mv $that_path $over_there" would put
you in the state where you have the same contents at $over_there and
at stage #3.  If you want to just take "their" contents as the
resolution, "git add $over_there" after doing that "git mv" would
let you record the resolution, so it does not look too bad.

It would also be a handy way to recover from a mistake made by
"directory level rename" heuristics.  Instead of resolving the
content-level conflicts at the wrong path and then moving that
resolved result to the right path, you can first correct the wrong
path by moving the conflicted whole to the right path and then
perform the content-level conflict resolution.  The advantage of the
latter is obvious.  You have to do two things (rename and edit) and
with the former way of doing 'edit' first and then 'rename', after
resolving the conflict and adding the result at the wrong path, "git
ls-files -u" or "git status" no longer help you remember that the
path still needs to be moved.  Instead, you can move first and "git
ls-files -u" would still remember that even after the move you still
need to deal with content-level conflicts.

Anyway, I think the "separate missing entry and conflicted entry
when issuing an error message" is a strict improvement.


[Footnotes]

*1* Of course, the "D/F conflicts are issue only among the entries
    at the same stage" and other usual rules apply when we check if
    the move of these higher stage entries is possible.  I am not
    sure if the low-level API functions like rename_index_entry_at()
    are prepared to deal with higher stage entries, though, so this
    may be a nontrivial amount of new work.  It is unclear to me if
    it is worth doing.

*2* Actually, "the object at stage #1 for conflicted path P" is a
    wrong thing to say.  The index is designed to hold multiple
    stage #1 entries at the same path so that it can express the
    case where there are more than one common ancestors, and
    multiple stage #3 entries to express an octopus merge in
    progress.  The notation to name the object in the index punts
    and does not let you say "git diff :1.0:path :1.1:path" to
    compare the first and the second common ancestor versions of
    path, though it probably should if we wanted to be consistent.


  reply	other threads:[~2020-07-18 17:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-17 23:24 [PATCH] git-mv: improve error message for conflicted file Chris Torek via GitGitGadget
2020-07-17 23:47 ` Eric Sunshine
2020-07-18  1:35   ` Chris Torek
2020-07-18  6:55     ` Eric Sunshine
2020-07-18  0:07 ` Junio C Hamano
2020-07-18  2:00   ` Elijah Newren
2020-07-18 17:46     ` Junio C Hamano [this message]
2020-07-19  1:48       ` Elijah Newren
2020-07-19  6:16         ` Junio C Hamano
2020-07-20  6:17 ` [PATCH v2] " Chris Torek via GitGitGadget
2020-07-20 18:28   ` Junio C Hamano

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=xmqq8sfgd8cu.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=chris.torek@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).