git blame --follow

git blame --follow

[Wolfgang Rohdewald]

git log --follow filename

shows the full history, while

git blame --follow filename

blames everything to the latest commit (which was
a file rename)

it would be nice if --follow were also supported by
git blame. Actually git blame accepts --follow without
an error message, it just ignores it. git help blame
does not mention --follow

using latest git from master
git version 1.7.4.1.234.ga3513

-- 
Wolfgang

Re: git blame --follow

[Junio C Hamano]

Wolfgang Rohdewald <wolfgang@rohdewald.de> writes:

> git log --follow filename
>
> shows the full history, while
>
> git blame --follow filename
>
> blames everything to the latest commit (which was
> a file rename)

Huh?

    $ git checkout master^0
    $ git mv COPYING RENAMING
    $ git commit -m renamed
    $ git blame --follow RENAMING

gives everything blamed to 075b845a COPYING (but that is probably by
accident, see below).  FYI,

    $ git blame RENAMING

should also blame everything to the same commit and the same COPYING
file. If you get a different behaviour out of the above command sequence,
there is something else going on.

I didn't know "blame" even accepted "--follow".  It is entirely out of the
scope of its design to take "--follow" option, as the "blame" command
itself has its own and real "follow" logic that is enabled by default
(i.e. it follows a whole file rename without any option), instead of the
checkbox "--follow" hack that is in "git log" family of commands.  You
cannot even turn it off.

Perhaps the behaviour you saw comes from the internal revision traversal
machinery blame uses getting confused by the use of --follow, but I am too
lazy to check (and I don't think it is worth to check, as the command
should follow by default).

We should just teach "blame" to ignore --follow option from the command
line, without even complaining.

Re: git blame --follow

[Wolfgang Rohdewald]

On Dienstag 15 März 2011, Junio C Hamano wrote:
> Wolfgang Rohdewald <wolfgang@rohdewald.de> writes:
> > git log --follow filename
> > 
> > shows the full history, while
> > 
> > git blame --follow filename
> > 
> > blames everything to the latest commit (which was
> > a file rename)
> 
> Huh?
> 
>     $ git checkout master^0
>     $ git mv COPYING RENAMING
>     $ git commit -m renamed
>     $ git blame --follow RENAMING
> 
> gives everything blamed to 075b845a COPYING (but that is
> probably by accident, see below).  FYI,
> 
>     $ git blame RENAMING
> 
> should also blame everything to the same commit and the same
> COPYING file. If you get a different behaviour out of the
> above command sequence, there is something else going on.

I get the same - except that only most is attributed to 075b845a,
some is attributed to 703601d6. But git blame and git
blame --follow return the same output with your example

> I didn't know "blame" even accepted "--follow".  It is
> entirely out of the scope of its design to take "--follow"
> option, as the "blame" command itself has its own and real
> "follow" logic that is enabled by default (i.e. it follows a
> whole file rename without any option)

so if I rename a parent directory of myfile and do git blame 
myfile, blaming should ignore the renaming. This works
with the git repo

git mv xdiff xxx
git ci -m'mv xdiff xxx'
cd xxx
git blame xemit.c

But with my repository (which I cannot share),
this does not happen. git blame attributes everything to 
the renaming commit. If I checkout the commit before, git
blame shows everything correctly.

So there must be something special with my repo. How could
I debug that? BTW the renaming happened in svn, it is the
last svn commit for this file before I imported this svn
repo into git (like described in the Pro Git book)

And - for directories below the renamed one git log --follow 
cannot cross this barrier either but if the "follow" logic
is different I suppose this is not related

-- 
Wolfgang

Re: git blame --follow

[Jakub Narebski]

Wolfgang Rohdewald <wolfgang@rohdewald.de> writes:

> But with my repository (which I cannot share),
> this does not happen. git blame attributes everything to 
> the renaming commit. If I checkout the commit before, git
> blame shows everything correctly.

"git blame" manpage says:

       -M|<num>|
              Detect  moved  or copied lines within a file. [...]
              [...]
              <num> is optional [...]

       -C|<num>|
              In  addition  to  -M,  detect lines moved or copied from other files
              that were modified in the same commit.

Sidenote: I wonder why we use '-M|<num>|' and not '-M[<num>]' here.

So you probably want to run "git blame -C -C <file>", not "git blame <file>".
Note that it is the same option name to turn on rename detection as is
used for for "git diff".

Note that "git gui blame" shows _two_ commits for each line: one is
result of "git blame" (and it would show commit that did code
movement, or renamed file), the other is result of "git blame -C -C -w",
which would show commit that changed line, disregarding whitespace-only
change (like e.g. reindent caused by extracting code into separate
function).

I wonder if it would be possible to generate result of "git blame"
and of "git blame -C -C -w" in a single run (with --porcelain or
--incremental)...

[...]
> And - for directories below the renamed one git log --follow 
> cannot cross this barrier either but if the "follow" logic
> is different I suppose this is not related

The implementation of "git log --follow <file>" is a bolted-on hack,
and it doesn't work in all cases; for example it doesn't cross
boundaries of subtree merge, IIRC.  Try "git log --follow gitweb/gitweb.perl"
in git repository itself...

HTH
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: git blame --follow

[Wolfgang Rohdewald]

On Donnerstag 17 März 2011, Jakub Narebski wrote:
> So you probably want to run "git blame -C -C <file>", not "git
> blame <file>".

that does the trick - I only tried "git blame -C"

-- 
Wolfgang

Re: git blame --follow

[Jakub Narebski]

Dnia czwartek 17. marca 2011 11:16, Wolfgang Rohdewald napisał:
> On Donnerstag 17 März 2011, Jakub Narebski wrote:

> > So you probably want to run "git blame -C -C <file>", not "git
> > blame <file>".
> 
> that does the trick - I only tried "git blame -C"

Hmmm... it is described in git-blame(1) manpage, but you have to read
it carefully.


"git blame" synopsis states:

 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] [-L n,m]
             [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
             [<rev> | --contents <file> | --reverse <rev>] [--] <file>

The important thing is to notice '[-C] [-C] [-C]' here.


In the description of '-C' option we have:

  -C|<num>|::
        In addition to `-M`, detect lines moved or copied from other
        files that were modified in the same commit.  This is
        useful when you reorganize your program and move code
        around across files.  When this option is given twice,
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        the command additionally looks for copies from other
        files in the commit that creates the file. When this
        option is given three times, the command additionally
        looks for copies from other files in any commit.

I have underlined important part.
-- 
Jakub Narebski
Poland

git blame --follow

[norbert.nemec]

Hi there,

'git blame --follow' seems to be undocumented. The exact behavior is not 
clear to me. Perhaps an alias for some combination of '-C' and '-M'? It 
seems not be be fully consistent with 'git log --follow'.

Could someone clarify? Did I miss something?

Greetings,
Norbert

Re: git blame --follow

[Jeff King]

On Thu, Sep 06, 2012 at 09:02:17AM +0200, norbert.nemec wrote:

> 'git blame --follow' seems to be undocumented. The exact behavior is
> not clear to me. Perhaps an alias for some combination of '-C' and
> '-M'? It seems not be be fully consistent with 'git log --follow'.
> 
> Could someone clarify? Did I miss something?

I don't think it was ever intended to do anything; the only reason it is
not rejected outright is that "blame" piggy-backs on the regular
revision option parser used by "log" and others.

What would you expect it to do?

I can't think of a sane behavior for "blame --follow". The follow code
is about tweaking path-limiting during traversal, but blame does not use
pathspecs. It tracks content, and the "-C" option already instructs it to
look across file boundaries.

-Peff

Re: git blame --follow

[norbert.nemec]

Thanks for the explanation.

I actually do not have any clear opinion what it should do. Just that 
the current situation is confusing when experimenting and trying to 
understand the behavior of git blame and git log: an intuitive option 
that is accepted but ignored.

The option should either be rejected or do *something* documented and 
useful. Ideally, it should result in behavior that matches 'git log 
--follow' as closely as possible. So maybe, it should be a synonym for a 
certain number of "-C" options?

Greetings,
Norbert



Am 06.09.12 11:58, schrieb Jeff King:
> On Thu, Sep 06, 2012 at 09:02:17AM +0200, norbert.nemec wrote:
>
>> 'git blame --follow' seems to be undocumented. The exact behavior is
>> not clear to me. Perhaps an alias for some combination of '-C' and
>> '-M'? It seems not be be fully consistent with 'git log --follow'.
>>
>> Could someone clarify? Did I miss something?
>
> I don't think it was ever intended to do anything; the only reason it is
> not rejected outright is that "blame" piggy-backs on the regular
> revision option parser used by "log" and others.
>
> What would you expect it to do?
>
> I can't think of a sane behavior for "blame --follow". The follow code
> is about tweaking path-limiting during traversal, but blame does not use
> pathspecs. It tracks content, and the "-C" option already instructs it to
> look across file boundaries.
>
> -Peff
>

Re: git blame --follow

[Jeff King]

On Thu, Sep 06, 2012 at 12:12:42PM +0200, norbert.nemec wrote:

> The option should either be rejected or do *something* documented and
> useful. Ideally, it should result in behavior that matches 'git log
> --follow' as closely as possible. So maybe, it should be a synonym
> for a certain number of "-C" options?

But I don't see how it would match "git log --follow", as that is a
fundamentally different operation that makes no sense in the context of
blame (why would you be adjusting the pathspec? There is no pathspec). A
synonym for "-C" would just confuse things more, as log also has "-C"
and it is not a synonym there.

So if anything, I'd say to simply reject it. It's not documented, and it
never did anything useful. Patches welcome.

-Peff

[PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Drew Northup]

Make note that while the --follow option is accepted by git blame it does
nothing.

Signed-off-by: Drew Northup <n1xim.email@gmail.com>
---
 Documentation/git-blame.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 7ee9236..7465bd8 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] [-L n,m]
-	    [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>]
+	    [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>] [--follow]
 	    [<rev> | --contents <file> | --reverse <rev>] [--] <file>
 
 DESCRIPTION
@@ -78,6 +78,9 @@ include::blame-options.txt[]
 	abbreviated object name, use <n>+1 digits. Note that 1 column
 	is used for a caret to mark the boundary commit.
 
+--follow::
+        NO-OP accepted due to using the option parser also used by
+        'git log'
 
 THE PORCELAIN FORMAT
 --------------------
-- 
1.7.12.rc0.54.g9e2116a

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Junio C Hamano]

Drew Northup <n1xim.email@gmail.com> writes:

> Make note that while the --follow option is accepted by git blame it does
> nothing.
>
> Signed-off-by: Drew Northup <n1xim.email@gmail.com>
> ---
>  Documentation/git-blame.txt | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
> index 7ee9236..7465bd8 100644
> --- a/Documentation/git-blame.txt
> +++ b/Documentation/git-blame.txt
> @@ -9,7 +9,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] [-L n,m]
> -	    [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>]
> +	    [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>] [--follow]
>  	    [<rev> | --contents <file> | --reverse <rev>] [--] <file>
>  
>  DESCRIPTION
> @@ -78,6 +78,9 @@ include::blame-options.txt[]
>  	abbreviated object name, use <n>+1 digits. Note that 1 column
>  	is used for a caret to mark the boundary commit.
>  
> +--follow::
> +        NO-OP accepted due to using the option parser also used by
> +        'git log'

This is triply questionable.  If it is a useless NO-OP, it shouldn't
be advertised in the SYNOPSIS section to begin with.

Your "--follow is a no-op for blame" is technically correct, but I
think the only ones that can appreciate that technical correctness
are those like you who know why we can get away by having a
seemingly no-op "--follow" option without losing functionality.

"git blame" follows the whole-file renames correctly [*1*], without
any -M/-C options.  There is no _need_ to use the "keep one global
filename to follow, and switch to a different filename when it
disappears" hack the "--follow" code uses.  That is the reason why
"blame" pays no attention to the "--follow" option.  You know that,
and that is why you think it is a sane thing to describe it in
technically correct way.

But I think most of the readers of the documentation are not aware
of that true reason why it can be a "No-op".  Worse yet, they may
have heard of the "--follow" option that the "log" command has from
any of the numerous misguided web pages, and are led to believe that
the "--follow" option is a true feature, not a checkbox hack.

If readers come from that background, thinking "--follow" is the way
to tell Git to follow renames, what message does your description
send them?  I would read it as "git blame accepts --follow from the
command line, but it is a no-op.  There is no way to make it follow
renames."

That is a totally wrong message to send.  You failed to teach the
reader that there is no need to do anything special to tell the
command to follow per-line origin across renames.

So if anything, I would phrase it this way instead:

    --follow::
          This option is accepted but silently ignored.  "git blame"
	  follows per-line origin across renames without any special
	  options, and there is no reason to use this option.

It does not matter to the reader why it is accepted by the parser at
the mechanical level (your description of the parser being shared
with the log family).  What matters to the readers is that it is
accepted as a courtesy (as opposed to being rejected as an error),
but it is unnecessary to give it in the first place.

If you followed the logic along, you would agree that it is a crime
to list it in the SYNOPSIS section.


[Footnote]

*1* Unlike "--follow" checkbox hack, which follows renames correctly
only in a strictly linear history, "blame" maintains the filename
being tracked per history traversal path and will follow a history
like this:

    ----A----B
         \    \
          C----D

where you originally had your file as fileA, one side of the fork
renamed it to fileB while the other side of the fork renamed it to
fileC, and a merge coalesced it to fileD.  "git blame fileD" will
find the line-level origin across all these renames.

Try this:

    git init
    printf "%s\n" a a a a a >fileA
    git add fileA
    git commit -m A
    git branch side
    printf "%s\n" a b b a a >fileB
    git add fileB
    rm fileA
    git commit -a -m B
    git checkout side
    printf "%s\n" a a c c a >fileC
    git add fileC
    rm fileA
    git commit -a -m C
    git merge master
    git rm -f fileA fileB fileC
    printf "%s\n" a b d c a >fileD
    git add fileD
    git commit -a -m D

and then in the resulting history

    git blame fileD

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Jeff King]

On Tue, Sep 18, 2012 at 09:38:32PM -0700, Junio C Hamano wrote:

> That is a totally wrong message to send.  You failed to teach the
> reader that there is no need to do anything special to tell the
> command to follow per-line origin across renames.
> 
> So if anything, I would phrase it this way instead:
> 
>     --follow::
>           This option is accepted but silently ignored.  "git blame"
> 	  follows per-line origin across renames without any special
> 	  options, and there is no reason to use this option.

I think that is much better than Drew's text. But I really wonder if the
right solution is to simply disallow --follow. It does not do anything,
and it is not documented. There is no special reason to think that it
would do anything, except by people who try it. So perhaps that is the
right time to say "no, this is not a valid option".

Like this (totally untested) patch:

diff --git a/builtin/blame.c b/builtin/blame.c
index 0e102bf..412d6dd 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2365,6 +2365,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 			ctx.argv[0] = "--children";
 			reverse = 1;
 		}
+		else if (!strcmp(ctx.argv[0], "--follow")) {
+			error("unknown option `--follow`");
+			usage_with_options(blame_opt_usage, options);
+		}
 		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:

-Peff

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Sep 18, 2012 at 09:38:32PM -0700, Junio C Hamano wrote:
>
>> That is a totally wrong message to send.  You failed to teach the
>> reader that there is no need to do anything special to tell the
>> command to follow per-line origin across renames.
>> 
>> So if anything, I would phrase it this way instead:
>> 
>>     --follow::
>>           This option is accepted but silently ignored.  "git blame"
>> 	  follows per-line origin across renames without any special
>> 	  options, and there is no reason to use this option.
>
> I think that is much better than Drew's text. But I really wonder if the
> right solution is to simply disallow --follow. It does not do anything,
> and it is not documented. There is no special reason to think that it
> would do anything, except by people who try it. So perhaps that is the
> right time to say "no, this is not a valid option".
>
> Like this (totally untested) patch:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 0e102bf..412d6dd 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2365,6 +2365,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  			ctx.argv[0] = "--children";
>  			reverse = 1;
>  		}
> +		else if (!strcmp(ctx.argv[0], "--follow")) {
> +			error("unknown option `--follow`");
> +			usage_with_options(blame_opt_usage, options);
> +		}
>  		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
>  	}
>  parse_done:

This patch would not hurt existing users very much; blame is an
unlikely thing to run in scripts, and it is easy to remove the
misguided --follow from them.

So I am in general OK with it, but if we are to go that route, we
should make sure that the documentation makes it clear that blame
follows whole-file renames without any special instruction before
doing so.  Otherwise, it again will send the same wrong message to
people who try to use the "--follow" from their experience with
"log", no?

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Jeff King]

On Wed, Sep 19, 2012 at 12:36:56PM -0700, Junio C Hamano wrote:

> > Like this (totally untested) patch:
> >
> > diff --git a/builtin/blame.c b/builtin/blame.c
> > index 0e102bf..412d6dd 100644
> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -2365,6 +2365,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
> >  			ctx.argv[0] = "--children";
> >  			reverse = 1;
> >  		}
> > +		else if (!strcmp(ctx.argv[0], "--follow")) {
> > +			error("unknown option `--follow`");
> > +			usage_with_options(blame_opt_usage, options);
> > +		}
> >  		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
> >  	}
> >  parse_done:
> 
> This patch would not hurt existing users very much; blame is an
> unlikely thing to run in scripts, and it is easy to remove the
> misguided --follow from them.

I would not worry about such users. I am of the opinion that their
scripts are buggy for calling a useless and undocumented option that
just happened to not complain.

> So I am in general OK with it, but if we are to go that route, we
> should make sure that the documentation makes it clear that blame
> follows whole-file renames without any special instruction before
> doing so.  Otherwise, it again will send the same wrong message to
> people who try to use the "--follow" from their experience with
> "log", no?

I guess it depends on your perspective. I can see the argument that
blame is already doing what --follow would ask for, and thus it is a
no-op. I think of it more as --follow is nonsensical for blame. But I
do not think either is wrong per se, and there is no reason not to help
people who come to git thinking the former. So yes, I think
documentation in either case is probably a good thing.

I am a little lukewarm on my patch if only because of the precedent it
sets.  There are a trillion options that revision.c parses that are not
necessarily meaningful or implemented for sub-commands that piggy-back
on its option parser. I'm not sure we want to get into manually
detecting and disallowing each one in every caller.

-Peff

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Kevin Ballard]

On Sep 19, 2012, at 12:42 PM, Jeff King <peff@peff.net> wrote:

>> So I am in general OK with it, but if we are to go that route, we
>> should make sure that the documentation makes it clear that blame
>> follows whole-file renames without any special instruction before
>> doing so.  Otherwise, it again will send the same wrong message to
>> people who try to use the "--follow" from their experience with
>> "log", no?
> 
> I guess it depends on your perspective. I can see the argument that
> blame is already doing what --follow would ask for, and thus it is a
> no-op. I think of it more as --follow is nonsensical for blame. But I
> do not think either is wrong per se, and there is no reason not to help
> people who come to git thinking the former. So yes, I think
> documentation in either case is probably a good thing.
> 
> I am a little lukewarm on my patch if only because of the precedent it
> sets.  There are a trillion options that revision.c parses that are not
> necessarily meaningful or implemented for sub-commands that piggy-back
> on its option parser. I'm not sure we want to get into manually
> detecting and disallowing each one in every caller.

I tend to agree with your final sentiment there. But the point that
users may not realize that blame already follows is also valid. Perhaps
we should catch --follow, as in your patch, but instead of saying that
it's an unknown argument, just print out a helpful message saying blame
already follows renames (and then continue with the blame anyway, so
as to not set a precedent to abort on unknown-but-currently-accepted
flags).

-Kevin

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Jeff King]

On Wed, Sep 19, 2012 at 01:31:50PM -0700, Kevin Ballard wrote:

> > I am a little lukewarm on my patch if only because of the precedent it
> > sets.  There are a trillion options that revision.c parses that are not
> > necessarily meaningful or implemented for sub-commands that piggy-back
> > on its option parser. I'm not sure we want to get into manually
> > detecting and disallowing each one in every caller.
> 
> I tend to agree with your final sentiment there. But the point that
> users may not realize that blame already follows is also valid. Perhaps
> we should catch --follow, as in your patch, but instead of saying that
> it's an unknown argument, just print out a helpful message saying blame
> already follows renames (and then continue with the blame anyway, so
> as to not set a precedent to abort on unknown-but-currently-accepted
> flags).

Sure, that would probably make sense. Care to roll a patch with
suggested wording?

-Peff

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Kevin Ballard]

On Sep 19, 2012, at 1:37 PM, Jeff King <peff@peff.net> wrote:

> On Wed, Sep 19, 2012 at 01:31:50PM -0700, Kevin Ballard wrote:
> 
>>> I am a little lukewarm on my patch if only because of the precedent it
>>> sets.  There are a trillion options that revision.c parses that are not
>>> necessarily meaningful or implemented for sub-commands that piggy-back
>>> on its option parser. I'm not sure we want to get into manually
>>> detecting and disallowing each one in every caller.
>> 
>> I tend to agree with your final sentiment there. But the point that
>> users may not realize that blame already follows is also valid. Perhaps
>> we should catch --follow, as in your patch, but instead of saying that
>> it's an unknown argument, just print out a helpful message saying blame
>> already follows renames (and then continue with the blame anyway, so
>> as to not set a precedent to abort on unknown-but-currently-accepted
>> flags).
> 
> Sure, that would probably make sense. Care to roll a patch with
> suggested wording?

Sadly, no. I'm not in a position to contribute to GPL code anymore, based
on my current job (I'd have to jump through some hoops to get the ok
to expose myself to that potential legal liability).

-Kevin

Re: [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I guess it depends on your perspective. I can see the argument that
> blame is already doing what --follow would ask for, and thus it is a
> no-op. I think of it more as --follow is nonsensical for blame.

Is "--follow" a nonsense in the context of blame?  I am not so sure.

I think it all boils down to this question:

	Does "blame" that does not follow rename make sense?

When you think about -M (allows us to keep track of the origin of
lines inside a single file when they are moved around) and -C
(allows us to keep track of the origin of lines that migrate from
another file), "follow across whole-file rename" is another optional
mode of operation in the same class to tell the command to pay more
processing cost to buy better precision.  When you know what you are
interested in happened entirely inside a file that was never
renamed, "blame -M" without "-C" and without whole-file rename
tracking is a sensible way to set that trade-off, even though we
currently do not have a way to say "--no-follow".

Eventually we would review and accept a patch to fix "--follow" by
somebody and that patch will make "--follow" truly follows renames
by keeping track of a single pathspec used to limit the changes per
ancestry traversal path, instead of switching one global one, which
is the current hack does.

Once that happens, what "--follow" does will match exactly what the
current "blame" internally (and unconditionally) does. The current
"blame" pays no attention to "--follow" because it wants to do the
right thing without letting the broken "--follow" logic to take over
and do a half-hearted job at following renames.

So if we were to do something special for "--follow" inside blame, I
think the right thing to do is probably to silently ignore, and in
addition, accept "--no-follow" and disable the whole-file rename
tracking logic.  When a true "--follow" comes along, we may be able
to rip out the whole-file rename tracking logic from "blame" and let
the version of "--follow" implemented correctly for the "log"
family.

Re* [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Sep 19, 2012 at 01:31:50PM -0700, Kevin Ballard wrote:
>
>> > I am a little lukewarm on my patch if only because of the precedent it
>> > sets.  There are a trillion options that revision.c parses that are not
>> > necessarily meaningful or implemented for sub-commands that piggy-back
>> > on its option parser. I'm not sure we want to get into manually
>> > detecting and disallowing each one in every caller.
>> 
>> I tend to agree with your final sentiment there. But the point that
>> users may not realize that blame already follows is also valid. Perhaps
>> we should catch --follow, as in your patch, but instead of saying that
>> it's an unknown argument, just print out a helpful message saying blame
>> already follows renames (and then continue with the blame anyway, so
>> as to not set a precedent to abort on unknown-but-currently-accepted
>> flags).
>
> Sure, that would probably make sense. Care to roll a patch with
> suggested wording?

Let's do this for now instead.  That would make it clear to people
who (rightly or wrongly) think the "--follow" option should do
something that we already do so, and explain the output that they
see when they do give the "--follow" option to the command.

I may do a "--no-follow" patch as a follow-up, or I may not,
depending on the mood and workload.


 Documentation/git-blame.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git c/Documentation/git-blame.txt w/Documentation/git-blame.txt
index 7ee9236..809823e 100644
--- c/Documentation/git-blame.txt
+++ w/Documentation/git-blame.txt
@@ -20,6 +20,12 @@ last modified the line. Optionally, start annotating from the given revision.
 
 The command can also limit the range of lines annotated.
 
+The origin of lines is automatically followed across whole-file
+renames (currently there is no option to turn the rename-following
+off). To follow lines moved from one file to another, or to follow
+lines that were copied and pasted from another file, etc., see the
+`-C` and `-M` options.
+
 The report does not tell you anything about lines which have been deleted or
 replaced; you need to use a tool such as 'git diff' or the "pickaxe"
 interface briefly mentioned in the following paragraph.

Re: Re* [PATCH] Documentation/git-blame.txt: --follow is a NO-OP

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Let's do this for now instead.  That would make it clear to people
> who (rightly or wrongly) think the "--follow" option should do
> something that we already do so, and explain the output that they
> see when they do give the "--follow" option to the command.
>
> I may do a "--no-follow" patch as a follow-up, or I may not,
> depending on the mood and workload.

A patch to do so looks like this.

If you know your history did not have any rename, or if you care
only about the history after a large rename that happened some time
ago, "git blame --no-follow $path" can be a way to tell the command
not to bother about them.

When you use -C, the lines that came from the renamed file will
still be found without the whole-file rename detection anyway, and
this is not all that interesting either way, I would think.


diff --git c/builtin/blame.c w/builtin/blame.c
index cad4111..bfa6086 100644
--- c/builtin/blame.c
+++ w/builtin/blame.c
@@ -42,6 +42,7 @@ static int blank_boundary;
 static int incremental;
 static int xdl_opts;
 static int abbrev = -1;
+static int no_whole_file_rename;
 
 static enum date_mode blame_date_mode = DATE_ISO8601;
 static size_t blame_date_width;
@@ -1226,7 +1227,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 	 * The first pass looks for unrenamed path to optimize for
 	 * common cases, then we look for renames in the second pass.
 	 */
-	for (pass = 0; pass < 2; pass++) {
+	for (pass = 0; pass < 2 - no_whole_file_rename; pass++) {
 		struct origin *(*find)(struct scoreboard *,
 				       struct commit *, struct origin *);
 		find = pass ? find_rename : find_origin;
@@ -2344,6 +2345,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	init_revisions(&revs, NULL);
 	revs.date_mode = blame_date_mode;
 	DIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);
+	DIFF_OPT_SET(&revs.diffopt, FOLLOW_RENAMES);
 
 	save_commit_buffer = 0;
 	dashdash_pos = 0;
@@ -2367,6 +2369,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:
+	no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
+	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
 	argc = parse_options_end(&ctx);
 
 	if (0 < abbrev)
diff --git c/diff.c w/diff.c
index f1b0447..32ebcbb 100644
--- c/diff.c
+++ w/diff.c
@@ -3584,6 +3584,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 	else if (!strcmp(arg, "--follow"))
 		DIFF_OPT_SET(options, FOLLOW_RENAMES);
+	else if (!strcmp(arg, "--no-follow"))
+		DIFF_OPT_CLR(options, FOLLOW_RENAMES);
 	else if (!strcmp(arg, "--color"))
 		options->use_color = 1;
 	else if (!prefixcmp(arg, "--color=")) {