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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 19, 2012, 04:38 UTC
Message-ID: <7v627aiq47.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1348022905-10048-1-git-send-email-n1xim.email@gmail.com>

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

Show 28 quoted lines

> 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