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

Thomas Rast <trast@student.ethz.ch> writes:

We are not saying very different things, I think.

I guess I shouldn't have mentined "blame --incremental"; I was referring to a quick-hack experiment we did to show an early output when we pass blame to parent. You can imagine that inside pass_blame_to_parent() we can detect if the lines around the range we are passing the blame to parent were touched, we could show pretty-printed commit log and relevant diff-tree output at that point, which would give you pretty much the same history we are talking about.

Well, --follow is from "content-id" camp, while pathspec is a revision limiter that follows quite different philosophy, so they do not mix very well. "log --follow -- $path" is an unfortunate cross between these two philosophies and spelled that way only because that was the easiest way to cheat in the command line parser.

As you alluded to, "log --follow HEAD -- $path" would mean exactly the same as "log -L<range>,HEAD:$path HEAD" in the mock syntax I gave you earlier in the thread, but that does not change the fact that it is conceptually very different from the normal "git log".

I am sure it is "entirely possible", even though it might be quite expensive (both development wise and also the end result).

A funny thing is, I do not think it is so "hard to explain and rarely used" case. Imagine that you have this history:

                 y---z---D maint
                /
    ---o---A---x---B---o---C master

Wouldn't it be nice if you can ask this question?

    I have these lines in frotz.c at 'master' version, and I recently made
    fixes that I need to backport to 'maint'.  How does the corresponding
    region look like in 'maint', and what changes that may be necessary
    for and interfere with my backport are there since these two branches
    forked?

In the ugly mock syntax, you would write it like this:

    log -p -L<range>,master:frotz.c master...maint

In this example, you have two positive revisions (notice three-dots for symmetric difference), but you can easily imagine that this will work well when you have more than two branches you are interested in.

To produce a useful output, you would need to dig from C down to x while listing the commit that touch the specified range and adjusting it, then dig the other way from x through y, z and D, again mapping the corresponding range.

I am not saying that what -L does is a bad thing, though. All I am saying is that it is conceptually very different, and trying to coax it into existing command line parsing rules of "git log" might not work well.

You could use a few rules to allow something like this which might be more friendly to the eyes that are used to looking at the input to setup_revisions() parser:

    log -p -L<range1> <path1> -L<range2> <revs>...

The short-hand rules may look something like this:

    - -L<range> with missing <path> is the same as -L<range>,<path> for
       the last <path> we have seen used with -L (it would be cleaner to
       use a single string given to -L i.e. "-L<range>,<path>" instead of
       a rule "-L takes one or two parameters" that is totally odd-ball
       from the point of view of normal POSIX command line rules, though);

    - If there is no <path> anywhere in the first use of -L, we take the
      first pathspec element as if it were <path>.

    - -L<range>,<path> is the same as -L<range>,<rev>:<path> where <rev>
       is the first positive revision given on the command line.

No matter what syntax sugarcoating is done, conceptually when -L says "these lines in this path", it has to talk about the range in one revision. -L<range>,<rev>:<path> instructs us: "you are to follow this block of lines in that path in this particular revision---figure out how that migrated while you traverse the history and show its evolution". There is no room for the normal pathspec rules to take effect---as we are operating in the "content-id" domain, we would ideally want to automatically even do --find-copies-harder, just like --follow and blame automatically runs rename detection (that is another reason I think the traversal semantics is very different from the regular "log", in addition to the need to traverse forward in addition to backward which I already mentioned). We may want to error out when we have any <pathspec> (after taking the second short-hand rule above into account, that is).

In short, I don't really have a very deep preference between "log -L" and "blame -L".

But as a very special case of allowing only one single positive rev and no forward-walking of the history, what "log -L" gives us is much closer to "blame -L". That is where my suggestion came from. If we were to eventually have that "log -p -L<range>,master:frotz.c master...maint", not the syntax but the capability, I am all for it to be part of the "log" command.