Re: git-mergetool reverse file ordering

[David Aguilar <davvid@gmail.com>]

On Wed, Aug 17, 2016 at 08:10:46AM +0200, Johannes Sixt wrote:
> Am 17.08.2016 um 08:05 schrieb Johannes Sixt:
> > Am 17.08.2016 um 03:25 schrieb David Aguilar:
> > > Hmm, I do like the idea of reusing the diff orderFile, but a
> > > mechanism for sorting arbitrary inputs based on the orderFile
> > > isn't currently exposed in a way that mergetool could use it.
> > 
> > Instead of using 'git ls-files -u | sed ... | sort -u' you could use
> > 
> >   git diff-files --name-status -O... | sed -ne '/^U/s/^..//p'
> 
> Or even better:
> 
>     git diff-files --name-only --diff-filter=U -O...

Nice!


> > > But, that sort is honestly kinda crude.  It can't implement the
> > > interesting case where we want bar.h to sort before bar.c but
> > > not foo.h.
> > > 

Note: I had a subtle typo above -- I meant to describe this desired
order:
	bar.h
	bar.c
	foo.h
	foo.c

which is not possible with an orderFile that has only:

	*.h
	*.c

But...

> > > If we did the sort option, we could have both.
> > 
> > I don't think that we need a new filter when the diff machinery is
> > capable of re-ordering files. We should enhance the diff machinery instead.
> > 

Reading the code more thoroughly, I fully agree.

Switching to a diff-files invocation lets us reuse the
diff.orderFile machinery and does not require exposing any
additional helpers.

While it would be *nice* if we had a way to sort foo.h before
foo.c and after bar.c, that's just a pie-in-the-sky idea.
Consistency is king.


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.

That code path does not currently do any sorting (which may
explain why we didn't notice it if we were just looking for
"sort" invocations) but maybe that's an edge case that we don't
need to address initially (if at all).

Would it be okay for the rerere code path to not honor the
orderFile?  IMO if we documented the behavior then it would be
acceptable, and perhaps can be improved as a follow-up.

For the basic support the implementation becomes really
simple: switch to using diff-files instead of ls-files.

If we did want consistency in the "git rerere remaining" code
path, would it be acceptable to teach "rerere" about
"-O<orderfile>" so that we could drive it from mergetool?

I only suggest an option, and not config support, because I
would be hesitant to make "rerere remaining" unconditionally
honor the orderFile since that feature is diff-centric, while
"rerere remaining" is a different beast altogether.  Adding a
flag is a middle-ground that would allow mergetool to drive it
while not suddently changing rerere's behavior.


The patches could then be:

1. switch to diff-files, add tests, and document how
   diff.orderFile affects mergetool.

2. Teach mergetool about the "-O<orderFile>" flag so that it can
   override the configuration, and add tests.  It could be
   argued that this should be squashed into (1).

3. (optional) teach "rerere remaining" to honor the
   -O<orderfile> flag and teach mergetool to supply the option.

Sound good?


ciao,
-- 
David