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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html