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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Drew Northup <n1xim.email@xxxxxxxxx> writes:

> Make note that while the --follow option is accepted by git blame it does
> nothing.
>
> Signed-off-by: Drew Northup <n1xim.email@xxxxxxxxx>
> ---
>  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

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]