git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Rast <trast@student.ethz.ch>
To: Junio C Hamano <gitster@pobox.com>
Cc: Bo Yang <struggleyb.nku@gmail.com>, <git@vger.kernel.org>
Subject: Re: [PATCH v3 05/13] parse the -L options
Date: Tue, 20 Jul 2010 00:48:17 +0200	[thread overview]
Message-ID: <201007200048.18284.trast@student.ethz.ch> (raw)
In-Reply-To: <7vwrsrnwg0.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> It almost feels as if you want to have something more like
> 
>     -L <begin>,<end>[,<path>]
> 
> where <path> is mandatory for the first use of -L (i.e. missing ,<path>
> means the same path from the previous -L that has one) to make it clear
> that this is completely different from the normal pathspec.

I think that would just needlessly break the analogy to git-blame.[0]
With the current code,

  git blame -L 2,3 <path>
  git log -L 2,3 <path>

work the same.  Multiple -L options could be retrofitted to git-blame,
making

  git {blame,log} -L 2,3 -L 4,5 <path>

work as expected.

As long as you only give a single path, even blame disambiguates in
favour of the filename:

  git blame -L 2,3 master # wants a file 'master'

It only starts breaking down as soon as you put the -L further away
from the filename:

  git blame -L 2,3 master master # looks for the file master:master
  git blame master -L 2,3 master # ditto
  git log -L 2,3 master master   # errors out since the second file has no -L [1]
  git log master -L 2,3 master   # looks for the file master:master

And as you have noted, the -- can also cause weird effects.  Currently

  git log -L 2,3 --branches      # error
  git log -L 2,3 ./--branches    # ok
  git log -L 2,3 -- --branches   # error

The last one is unfortunate, but fixing it while allowing a natural
positioning of the -L would require parsing the -L after a --.  That's
just as inconsistent, only in a different way.


[0] It also requires special support from shell completion.

[1] I agree with erroring out but I think the message wrongly
    recommends path filtering (since that doesn't work well with
    renames):

      fatal: Path master need a -L <range> option
      If you want follow the history of the whole file whether to using 'git log' without -L or using 'git log -L 1,$ <path>'

    Bo, can you please change it to e.g.:

      fatal: Path 'master' needs a -L<range> option
      If you want to follow the history of the whole file, use 'git log -L 1,$ <path>'

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

  reply	other threads:[~2010-07-19 22:48 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-11  6:18 [PATCH v3 01/13] parse-options: stop when encounter a non-option Bo Yang
2010-07-11  6:18 ` [PATCH v3 02/13] parse-options: add two helper functions Bo Yang
2010-07-12 16:45   ` Junio C Hamano
2010-07-11  6:18 ` [PATCH v3 03/13] add the basic data structure for line level history Bo Yang
2010-07-12 14:16   ` Bo Yang
2010-07-12 16:50     ` Junio C Hamano
2010-07-18 14:35       ` Bo Yang
2010-07-11  6:18 ` [PATCH v3 04/13] refactor parse_loc Bo Yang
2010-07-12 18:32   ` Junio C Hamano
2010-07-11  6:18 ` [PATCH v3 05/13] parse the -L options Bo Yang
2010-07-13 23:03   ` Junio C Hamano
2010-07-18 14:49     ` Bo Yang
2010-07-19 18:44       ` Junio C Hamano
2010-07-19 22:48         ` Thomas Rast [this message]
2010-07-19 23:53           ` Junio C Hamano
2010-07-20  7:51             ` Thomas Rast
2010-07-20 15:45               ` Junio C Hamano
2010-07-22  9:06                 ` Thomas Rast
2010-07-23  2:08                   ` Junio C Hamano
2010-07-20 15:46               ` Bo Yang
2010-07-20 15:47                 ` Bo Yang
2010-07-11  6:18 ` [PATCH v3 06/13] export three functions from diff.c Bo Yang
2010-07-11  6:18 ` [PATCH v3 07/13] add range clone functions Bo Yang
2010-07-12 21:52   ` Junio C Hamano
2010-07-11  6:18 ` [PATCH v3 08/13] map/take range to parent Bo Yang
2010-07-12 21:52   ` Junio C Hamano
2010-07-11  6:18 ` [PATCH v3 09/13] print the line log Bo Yang
2010-07-12 21:52   ` Junio C Hamano
2010-07-11  6:18 ` [PATCH v3 10/13] map/print ranges along traversing the history topologically Bo Yang
2010-07-12 21:52   ` Junio C Hamano
2010-07-11  6:18 ` [PATCH v3 11/13] add --always-print option Bo Yang
2010-07-13 20:35   ` Junio C Hamano
2010-07-11  6:19 ` [PATCH v3 12/13] add two test cases Bo Yang
2010-07-11  6:19 ` [PATCH v3 13/13] some document update Bo Yang
2010-07-11  8:27   ` Jakub Narebski
2010-07-12 14:12     ` Bo Yang
2010-07-12 18:45       ` Junio C Hamano
2010-07-18 14:37         ` Bo Yang
2010-07-12 16:45 ` [PATCH v3 01/13] parse-options: stop when encounter a non-option Junio C Hamano

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

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

  git send-email \
    --in-reply-to=201007200048.18284.trast@student.ethz.ch \
    --to=trast@student.ethz.ch \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=struggleyb.nku@gmail.com \
    /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.
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 NNTP newsgroup(s).