git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* What does 'git blame --before <rev> -- <file>' supposed to mean?
@ 2017-08-20 18:45 Kaartic Sivaraam
  2017-08-24  7:58 ` Kevin Daudt
  0 siblings, 1 reply; 3+ messages in thread
From: Kaartic Sivaraam @ 2017-08-20 18:45 UTC (permalink / raw)
  To: git

Hello all,

I tried to do a 'git blame' on a file and wanted to see the blame
before a particular revision of that file. I initially didn't know that
you could achieve that with,

    $ git blame <rev> <file>

I thought I need to pass the revision as a parameter so I tried (before
looking into the documentation) the command found in the subject. It
worked but to my surprise it had the same result as,

    $ git blame -- <file>

I was confused and came to know from the documentation that blame
doesn't have any '--before' option. That was even more surprising. Why
does blame accept an option which it doesn't identify? Shouldn't it
have warned that it doesn't accept the '--before' option? I guess it
should not accept it because it confuses the user a lot as the could
make it hard time for him to identify the issue.

'git blame' doesn't seem to be the only command that accepts options
not specified in the documentation there's 'git show' for it's company,

    $ git show --grep 'regex'

But the good thing with the above command is it behaves as expected. I
suspect this should be documented, anyway.

Thoughts ?

-- 
Kaartic

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: What does 'git blame --before <rev> -- <file>' supposed to mean?
  2017-08-20 18:45 What does 'git blame --before <rev> -- <file>' supposed to mean? Kaartic Sivaraam
@ 2017-08-24  7:58 ` Kevin Daudt
  2017-08-24 16:35   ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Kevin Daudt @ 2017-08-24  7:58 UTC (permalink / raw)
  To: Kaartic Sivaraam; +Cc: git

On Mon, Aug 21, 2017 at 12:15:59AM +0530, Kaartic Sivaraam wrote:
> Hello all,
> 
> I tried to do a 'git blame' on a file and wanted to see the blame
> before a particular revision of that file. I initially didn't know that
> you could achieve that with,
> 
>     $ git blame <rev> <file>
> 
> I thought I need to pass the revision as a parameter so I tried (before
> looking into the documentation) the command found in the subject. It
> worked but to my surprise it had the same result as,
> 
>     $ git blame -- <file>
> 
> I was confused and came to know from the documentation that blame
> doesn't have any '--before' option. That was even more surprising. Why
> does blame accept an option which it doesn't identify? Shouldn't it
> have warned that it doesn't accept the '--before' option? I guess it
> should not accept it because it confuses the user a lot as the could
> make it hard time for him to identify the issue.
> 
> 'git blame' doesn't seem to be the only command that accepts options
> not specified in the documentation there's 'git show' for it's company,
> 
>     $ git show --grep 'regex'
> 
> But the good thing with the above command is it behaves as expected. I
> suspect this should be documented, anyway.
> 
> Thoughts ?
> 
> -- 
> Kaartic

Git blame takes options that are fed to git rev-list, to limit the
commits being taken into account for blaming.

The man page shows "[--since=<date>]", which is one such option, but
before is valid as well.

git blame -h shows:

    <rev-opts> are documented in git-rev-list(1) 

and man git-blame shows under specifying ranges (emphasis mine): 

     When you are not interested in changes older than version v2.6.18,
     or changes older than 3 weeks, *you can use revision range
     specifiers similar to git rev-list*:

So these options are not documented under git blame, but git rev-list.

Perhaps the synopsis of man git-blame could be expanded so that that
it's clear it accepts rev-list options.

Kevin

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: What does 'git blame --before <rev> -- <file>' supposed to mean?
  2017-08-24  7:58 ` Kevin Daudt
@ 2017-08-24 16:35   ` Junio C Hamano
  0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2017-08-24 16:35 UTC (permalink / raw)
  To: Kevin Daudt; +Cc: Kaartic Sivaraam, git

Kevin Daudt <me@ikke.info> writes:

> Git blame takes options that are fed to git rev-list, to limit the
> commits being taken into account for blaming.
>
> The man page shows "[--since=<date>]", which is one such option, but
> before is valid as well.
>
> git blame -h shows:
>
>     <rev-opts> are documented in git-rev-list(1) 
>
> and man git-blame shows under specifying ranges (emphasis mine): 
>
>      When you are not interested in changes older than version v2.6.18,
>      or changes older than 3 weeks, *you can use revision range
>      specifiers similar to git rev-list*:
>
> So these options are not documented under git blame, but git rev-list.
>
> Perhaps the synopsis of man git-blame could be expanded so that that
> it's clear it accepts rev-list options.

While nothing in what you said is incorrect per-se, many options
that rev-list takes are parsed but then ignored by blame, simply
because they do not make much sense in the context of the command,
and "--before" is one of them.  

It is interesting to realize that "--since" (and its synonym
"--after") does make sense, unlike "--before" (and its synonym
"--until") which does not.

Let's imagine a history like this (time flows from left to right):

     --c1--c2--c4--c6--c8--c9
             \         /
             c3--c5--c7

where the tip of the history is at commit "c9", and the number in
the name of each commit denotes its timestamp.

 - "git rev-list c9" starts from "c9", and follows the chain of
   parents and would produce c9, c8, c7, c6, ..., c1, ....

 - If you add "--after 2", i.e. "git rev-list --after 2 c9" does
   exactly the same traversal as the above, but stops following the
   chain of parents for commits that is earlier than the specified
   time.

 - If you add "--before 6", i.e. "git rev-list --before 6 c9" does
   exactly the same traversal as the first one, but does not show
   the commit whose timestamp is later than the specified time.  It
   would be like saying "git rev-list c5 c6" in the above topology;
   the traversal from c9 down to c5 and c6 is done only to find c5
   and c6 to start the "real" traversal from. 

Now, "--after 2" will still show "c9", the tip you start your
traversal from, and this is important in the context of "blame".

Unlike "git rev-list" (and "git log" family of commands), which can
take more than one positive endpoints in the history (e.g. it is
perfectly sensible to ask "git log -p ^c1 c5 c6 -- myfile" in the
example topology above), "git blame" must be given exactly one
positive endpoint, as "git blame ^c1 c5 c6 -- myfile" would not make
any sense (ask: "do we want to know about myfile in c5?  or myfile
in c6?").

I think we also ignore "-n 4" in "blame -n 4 c9 -- myfile" (I didn't
check), but that is not because the operation fundamentally does not
make sense (like "--until") but because whoever wrote "blame" did
not think of the need to support it---in other words, if somebody
wants to add support for that option, I offhand do not see a
fundamental reason to reject it.

I do think appearing to take and understand and then silently
ignoring an option is bad, and it will help users if we tighten the
error checking while parsing the command line.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-08-24 16:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-20 18:45 What does 'git blame --before <rev> -- <file>' supposed to mean? Kaartic Sivaraam
2017-08-24  7:58 ` Kevin Daudt
2017-08-24 16:35   ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).