Re: git-mergetool reverse file ordering

From: David Aguilar <davvid@gmail.com>
Date: Aug 17, 2016, 21:18 UTC
Message-ID: <20160817211828.GB14619@gmail.com>
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:

Show 20 quoted lines

> 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.

Show 6 quoted lines

> 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