git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Luis Gutierrez <luisgutz@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: git-mergetool reverse file ordering
Date: Sat, 13 Aug 2016 20:42:21 -0700	[thread overview]
Message-ID: <20160814034221.GB21057@gmail.com> (raw)
In-Reply-To: <CAD8hE_yzNZDPkxRy8s4Fy2_dZN5ppWzLM_2xc01C-VAdR1Pj_g@mail.gmail.com>

On Wed, Jul 27, 2016 at 11:14:28AM +0100, Luis Gutierrez wrote:
> Hi,
> 
> Attached is a potential patch for reversing the order on which
> git-mergetool presents the files to merge.
> 
> Currently, when running git-mergetool, it performs a sort of the files
> to merge by alphabetical ordering. When working on C, this has the
> annoying effect of presenting the merge for a .c* files before the
> header files; which is always a bit harder to do. Reading the header
> first to figure out what the other dude changed is usually preferred.
> 
> The attach patch reverse the order (-r flag to sort) so *.h* are
> merged before  *.c* files
> 
> PS, given the simplicity of the patch, I have not tested it.
> 
> Regards
> 
> Luis

Thanks for the sug, this is an interesting idea and I definitely
see why we would want something like this...

> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index bf86270..cce3b0d 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -453,10 +453,10 @@ then
>  	then
>  		files=$(git rerere remaining)
>  	else
> -		files=$(git ls-files -u | sed -e 's/^[^	]*	//' | sort -u)
> +		files=$(git ls-files -u | sed -e 's/^[^	]*	//' | sort -u -r)
>  	fi
>  else
> -	files=$(git ls-files -u -- "$@" | sed -e 's/^[^	]*	//' | sort -u)
> +	files=$(git ls-files -u -- "$@" | sed -e 's/^[^	]*	//' | sort -u -r)
>  fi
>  
>  if test -z "$files"

While we won't take this patch as-is (please see
Documentation/SubmittingPatches for details about the patch
submission process), I am interested in the use case you've
described.

This use case makes me wonder whether the sorting we do here is
something that should be opened up a bit so that the it's not
quite so set in stone.

For example, an extension to the approach taken by this patch
would be to have `mergetool.reverseOrder` git config boolean
option that would tell us whether or not to use the "-r" flag
when calling sort.

But, IMO that is too rigid, and only addresses this narrow use
case.  What if users want a case-insensitive sort, or some other
preferred ordering?

We can address these concerns, and your use case, by opening it
up. Something like,

	sort=$(git config mergetool.sort || echo sort -u)

That preserves the existing behavior, and it opens it up so that
we can accomplish the same result as this patch by doing:

	git config mergetool.sort "sort -u -r"

Then, if someone later writes a nicer C/C++-specific sort that
sorts in the natural order but also keeps .h files before .c,
.cpp, etc. files with the same basename, then they could do:

	git config mergetool.sort crescent-fresh-sort

...and it'll be totally crescent.

Thoughts?  Would you be interested in helping work up a patch
for this idea?  At a minimum we should also write a test case in
t/t7610-mergetool.sh to verify that it works as advertised.

Let me know if you have any questions.

cheers,
-- 
David

  reply	other threads:[~2016-08-14  9:16 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 [this message]
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

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=20160814034221.GB21057@gmail.com \
    --to=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --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).