Re: [PATCH v3 05/13] parse the -L options

Bo Yang [off-list ref] writes:

The point is that, the syntax we support is:

-L n1,m1 -L n2,m2 pathspec1  -L n3,m3 pathspec2

That itself smells like a bad design, unless done very carefully and
documented clearly.

For example, what does this mean?

    $ git log -L n1,m1 master

As -L wants to see at least one path before running out of the command
line argument, we take "master" as the filename.  Hence, the command line
does not have any revision specified and defaults to HEAD.  I.e. "traverse
from the current HEAD and show only commits that touch the line region
n1,m1 that appears in the version of path 'master' in HEAD".

What about this?

    $ git log -L n1,m1 master..next one two

Clearly the user wants to traverse revision range between master and next.
The -L option wants to see one path so slurps "one".  The traversal
however is further limited by a pathspec "two".  Should the use of "one"
as an argument to -L automatically add it also to the pathspec?  I.e.
"traverse from 'next' down to 'master', checking commits that touch either
path 'one' or 'two', and show only commits that touch the line region
n1,m1 that appears in the version of path 'one' in 'next'"?

Or perhaps master..next is the name of the file the user is interested in?
I.e. "Starting from branches 'one' and 'two', show only commits that touch
the line region n1,m1 that appears in file 'master..next'"?  But that is a
broken interpretation, as "-L range path" cannto possibly make sense if
you have more than one starting point, and this interpretation gives you
two (i.e. 'one' and 'two').

How would you disambiguate -Lpaths, revisions and pathspecs?  How does -Lpath
interact with pathspecs?

What if the name of the file the user wants to annotate begins with a "-"?
For pathspec limiter, the users have already learned that "--" can be used
to say "everything that comes after this token is pathspec", but that
knowledge cannot be reused with this syntax.

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.

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