git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Luis Gutierrez <luisgutz@gmail.com>,
	John Keeping <john@keeping.me.uk>,
	git@vger.kernel.org
Subject: Re: git-mergetool reverse file ordering
Date: Wed, 17 Aug 2016 14:18:28 -0700	[thread overview]
Message-ID: <20160817211828.GB14619@gmail.com> (raw)
In-Reply-To: <1c089d6b-0acc-a5da-91bf-1887b6eaedbb@kdbg.org>


Hi Luis and Hannes,

On Wed, Aug 17, 2016 at 09:35:56AM +0200, Johannes Sixt wrote:
> Am 17.08.2016 um 08:46 schrieb David Aguilar:
> > The only thing that using diff-files doesn't address is the
> > rerere support in mergetool where it processes the files in
> > the order specified by "git rerere remaining".  This is why I
> > initially thought we needed a generic sort-like command.
> 
> I see. This is actually an important code path. How about this code
> structure:
> 
> if test $# -eq 0
> then
> 	cd_to_toplevel
> 
> 	if test -e "$GIT_DIR/MERGE_RR"
> 	then
> 		set -- $(git rerere remaining)
> 	fi
> fi
> files=$(git diff-files --name-only --diff-filter=U -- "$@")
> 

Beautiful.

> This does not require an enhancement of rerere-remaining and still captures
> all three cases that currently go through separate branches. (Throw in some
> version of --ignore-submodules= if necessary, but I guess it is not.)
> 
> We do have a problem if there are file names with spaces, but it is not a
> new problem.

Thanks for the heads-up about file names with spaces.  We set,

IFS='
'

in git-mergetool--lib.sh so file names with spaces should be ok.
Naturally, we won't be able to support paths with embedded
newlines, but that's not a new problem ;-)

We should probably also set core.quotePath=false when calling
diff-files so that git doesn't try to quote "unusual" paths,
e.g.

	git -c core.quotePath=false diff-files ...

Lastly, for anyone that's curious, I was wondering why we were
passing "-u" to "sort", and why we won't need to use "uniq" in
the new version.

The reason is that "ls-files -u" lists the different index
stages separately, and thus it reports duplicate paths.

"diff-files" with "--diff-filter=U" does not do that, so that's
another benefit to be gained from this change.

I think we've touched all the bases now.

Luis, I hope that makes sense.  Let us know if any of this is
unclear.


ciao,
-- 
David

      reply	other threads:[~2016-08-17 21:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-27 10:14 git-mergetool reverse file ordering Luis Gutierrez
2016-08-14  3:42 ` David Aguilar
2016-08-14 10:38   ` John Keeping
2016-08-15 20:19     ` Luis Gutierrez
2016-08-17  1:25       ` David Aguilar
2016-08-17  6:05         ` Johannes Sixt
2016-08-17  6:10           ` Johannes Sixt
2016-08-17  6:46             ` David Aguilar
2016-08-17  7:35               ` Johannes Sixt
2016-08-17 21:18                 ` David Aguilar [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=20160817211828.GB14619@gmail.com \
    --to=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=john@keeping.me.uk \
    --cc=luisgutz@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).