git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Andreen <harv@ruin.nu>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] Make XDF_NEED_MINIMAL default in blame.
Date: Thu, 20 Mar 2014 22:21:04 +0100	[thread overview]
Message-ID: <3088608.Qolv0cJMME@river> (raw)
In-Reply-To: <xmqq8us4y4ym.fsf@gitster.dls.corp.google.com>

On Thursday, March 20, 2014 01:45:21 PM Junio C Hamano wrote:
> There hasn't been any argument for this patch, either.
> 
> It is not like we are still in year 2007; timing result in a small
> project like Git itself is not a good enough argument to change a
> well established default at this late in the game, especially when
> there are ways like command line options for users to specify their
> preferred settings.

The main reason why I submitted it is because the current behavior is very 
unintuitive when you split big files into smaller ones. Like the one that was 
done in 2cf565c.

If you do:

 $ git checkout 2cf565c
 $ git blame -C -C -C -M Documentation/git-ls-files.txt

then blame will not be able to detect any moves, even though big chunks have 
been moved without whitespace changes. Worse is that if you do

 $ git gui blame Documentation/git-ls-files.txt

then that won't see any movies either, and there is no way to make git gui 
blame use --minimal.

I spent a lot of time at $work trying to code-review a big split like this, 
wondering why "git gui blame" and "git blame -C -C -C -M" couldn't detect 
moves when complete functions had been moved with no changes (not even 
whitespace). So by making it default it will hopefully help someone else from 
losing time investigating this.

It wasn't until I did a bisect on git, finding 582aa00 and later googling for 
XDF_NEED_MINIMAL to find 059a500, that I found out about --minimal. Which is 
not documented anywhere.

I guess it can be argued for just documenting --minimal and either patching 
"git gui blame" to use it, or make it configurable. However I think the 
current default is really confusing, and the reason for changing it back in 
2007 was for performance, so that was what I tried to focus on, hopefully 
showing that there aren't any noticeable differences.

/Michael

  reply	other threads:[~2014-03-20 21:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-20 20:18 [PATCH] Make XDF_NEED_MINIMAL default in blame Michael Andreen
2014-03-20 20:45 ` Junio C Hamano
2014-03-20 21:21   ` Michael Andreen [this message]
  -- strict thread matches above, loose matches on Subject: below --
2014-03-16 10:43 Michael Andreen
2014-03-16 12:12 ` Thomas Rast
2014-03-16 12:32   ` Michael Andreen

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=3088608.Qolv0cJMME@river \
    --to=harv@ruin.nu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).