git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kevin Daudt <me@ikke.info>
Cc: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>, git@vger.kernel.org
Subject: Re: What does 'git blame --before <rev> -- <file>' supposed to mean?
Date: Thu, 24 Aug 2017 09:35:03 -0700	[thread overview]
Message-ID: <xmqqy3q8x6tk.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170824075848.GG3839@alpha.vpn.ikke.info> (Kevin Daudt's message of "Thu, 24 Aug 2017 09:58:48 +0200")

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.


      reply	other threads:[~2017-08-24 16:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy3q8x6tk.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kaarticsivaraam91196@gmail.com \
    --cc=me@ikke.info \
    /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.
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).