RFC: Adding an option to log-like commands to call an external command for each revision

RFC: Adding an option to log-like commands to call an external command for each revision

[Ævar Arnfjörð Bjarmason]

I have this alias in my .gitconfig:

    review = "!f() { for rev in $(git rev-list --reverse \"$@\"); do
git show $rev; done; }; f"

I use it after I "git pull" to see what changed, e.g.:

    git review 49ea7b8..e1ef3c1

But sometimes I find that I want to do that for other things too, so I
have these hacks:

    review-grep = "!f() { for rev in $(git log --reverse
--pretty=format:%H --grep=\"$@\"); do git show $rev; done; }; f"
    review-file = "!f() { for rev in $(git log --reverse
--pretty=format:%H \"$@\"); do git show $rev; done; }; f"

But just now I wanted to use -S instead of grep, but adding aliases
like this is a bit silly.

Maybe we should have something like:

    git log --for-each=less a..b

To call "less" for each commit, what do you think?

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Jonathan Nieder]

Hi Ævar,

Ævar Arnfjörð Bjarmason wrote:

> I have this alias in my .gitconfig:
> 
>     review = "!f() { for rev in $(git rev-list --reverse \"$@\"); do
> git show $rev; done; }; f"
> 
> I use it after I "git pull" to see what changed, e.g.:
> 
>     git review 49ea7b8..e1ef3c1

Hmm,

 git show --reverse 49ea7b8..e1ef3c1

doesn't work because cmd_show() bypasses the get_revision() magic. :(

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Jeff King]

On Sun, Aug 29, 2010 at 08:30:15PM +0000, Ævar Arnfjörð Bjarmason wrote:

> I have this alias in my .gitconfig:
> 
>     review = "!f() { for rev in $(git rev-list --reverse \"$@\"); do
> git show $rev; done; }; f"
> 
> I use it after I "git pull" to see what changed, e.g.:
> 
>     git review 49ea7b8..e1ef3c1

It took me a minute of reading this to see why you would want to call
"git show" in a loop when you could have the same data from "git log"
all at once (and much faster, too). But I guess you like having an
individual less invocation for each commit. Have you tried "tig", which
might suit your purpose even better?

> But sometimes I find that I want to do that for other things too, so I
> have these hacks:
> 
>     review-grep = "!f() { for rev in $(git log --reverse
> --pretty=format:%H --grep=\"$@\"); do git show $rev; done; }; f"
>     review-file = "!f() { for rev in $(git log --reverse
> --pretty=format:%H \"$@\"); do git show $rev; done; }; f"
> 
> But just now I wanted to use -S instead of grep, but adding aliases
> like this is a bit silly.

I don't understand why you have these at all. Just use "git log
--format=%H" in your git review above (instead of rev-list), and then
you can just do:

  git review --grep=whatever
  git review -Sfoo
  git review file

Or am I missing something subtle?

You wouldn't even need to switch to log over rev-list, except that
rev-list misses log's useful "default to HEAD if no revisions given"
behavior.

> Maybe we should have something like:
> 
>     git log --for-each=less a..b
> 
> To call "less" for each commit, what do you think?

I think it is not very Unix-y. We already have many ways to to call a
command once per commit, including:

  - for i in `git rev-list "$@"`; do git show $i; done

  - git rev-list "$@" | xargs -n 1 git show

  - git log -z "$@" | perl -0ne 'open(LESS, "|less"); print LESS'

What does your solution offer that the other do not? Because you are
actually reinvoking git for each commit, it is more efficient than the
first two (as you seem to assume that the --for-each command will
receive the entire log output). But the third one should be more or less
equivalent to what you want (though note: if you want tty-ish things
like color on, you should set GIT_PAGER_IN_USE=1 so git knows output is
eventually going to a pager). Sure, yours is slightly less typing, but
it's _way_ less flexible, and that typing should probably be hidden
behind an alias anyway.

-Peff

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Ævar Arnfjörð Bjarmason]

`On Mon, Aug 30, 2010 at 03:08, Jeff King <peff@peff.net> wrote:

Sorry for not replying to this earlier.

> On Sun, Aug 29, 2010 at 08:30:15PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> I have this alias in my .gitconfig:
>>
>>     review = "!f() { for rev in $(git rev-list --reverse \"$@\"); do
>> git show $rev; done; }; f"
>>
>> I use it after I "git pull" to see what changed, e.g.:
>>
>>     git review 49ea7b8..e1ef3c1
>
> It took me a minute of reading this to see why you would want to call
> "git show" in a loop when you could have the same data from "git log"
> all at once (and much faster, too).

I just like the UI of having each commit "pop up" where I can either
page up/down within the commit, or dismiss it with "q" and go to the
next one.

You can't do both of those in a pager, up/down goes across commits,
and "q" quits the whole pager.

> But I guess you like having an individual less invocation for each
> commit. Have you tried "tig", which might suit your purpose even
> better?

I haven't tried tig, but "git review" as I implement it (below) is
sufficient for my needs.

>> But sometimes I find that I want to do that for other things too, so I
>> have these hacks:
>>
>>     review-grep = "!f() { for rev in $(git log --reverse
>> --pretty=format:%H --grep=\"$@\"); do git show $rev; done; }; f"
>>     review-file = "!f() { for rev in $(git log --reverse
>> --pretty=format:%H \"$@\"); do git show $rev; done; }; f"
>>
>> But just now I wanted to use -S instead of grep, but adding aliases
>> like this is a bit silly.
>
> I don't understand why you have these at all. Just use "git log
> --format=%H" in your git review above (instead of rev-list), and then
> you can just do:
>
>  git review --grep=whatever
>  git review -Sfoo
>  git review file
>
> Or am I missing something subtle?

You're not missing something, my alias was silly because I brainfarted
and didn't realize I could do $@, not "$@", so now it's:

    review = "!f() { for rev in $(git log --reverse --format=%H $@);
do git show $rev; done; }; f"

Which means I can do all of the commands you suggested above, thanks!

> You wouldn't even need to switch to log over rev-list, except that
> rev-list misses log's useful "default to HEAD if no revisions given"
> behavior.
>
>> Maybe we should have something like:
>>
>>     git log --for-each=less a..b
>>
>> To call "less" for each commit, what do you think?
>
> I think it is not very Unix-y. We already have many ways to to call a
> command once per commit, including:
>
>  - for i in `git rev-list "$@"`; do git show $i; done
>
>  - git rev-list "$@" | xargs -n 1 git show
>
>  - git log -z "$@" | perl -0ne 'open(LESS, "|less"); print LESS'
>
> What does your solution offer that the other do not? Because you are
> actually reinvoking git for each commit, it is more efficient than the
> first two (as you seem to assume that the --for-each command will
> receive the entire log output). But the third one should be more or less
> equivalent to what you want (though note: if you want tty-ish things
> like color on, you should set GIT_PAGER_IN_USE=1 so git knows output is
> eventually going to a pager). Sure, yours is slightly less typing, but
> it's _way_ less flexible, and that typing should probably be hidden
> behind an alias anyway.

Yeah, it's not very Unixy, I just find it so useful that I thought
there might be interest in adding it to Git. It's easily in the list
of top 5 git commands that I use.

But just using the pipe is more flexible I guess.

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Mark Lodato]

On Sat, Sep 11, 2010 at 11:56 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> `On Mon, Aug 30, 2010 at 03:08, Jeff King <peff@peff.net> wrote:
>> I think it is not very Unix-y. We already have many ways to to call a
>> command once per commit, including:
>>
>>  - for i in `git rev-list "$@"`; do git show $i; done
>>
>>  - git rev-list "$@" | xargs -n 1 git show
>>
>>  - git log -z "$@" | perl -0ne 'open(LESS, "|less"); print LESS'
>>
>> [snip]
>
> Yeah, it's not very Unixy, I just find it so useful that I thought
> there might be interest in adding it to Git. It's easily in the list
> of top 5 git commands that I use.

I also find myself wanting to walk through commits in a particular
order.  In my case, I don't want to run a command per commit, but
instead I want to check them out a la "git bisect." Let's pretend such
a command is called "git walk".  In in my case, I would run:

    git walk start --reverse 49ea7b8..e1ef3c1
    <compile, test, ...>
    git walk next
    <compile, test, ...>
    git walk next
    <...>
    git walk reset

In Ævar's case, he would just want:

    git walk start --reverse 49ea7b8..e1ef3c
    git walk run git show
    git walk reset

Or perhaps a shortcut:

    git walk start --reverse 49ea7b8..e1ef3c --run git show

There is already a simple implementation by August Lilleaas [1], but
it would be nice to re-use the git-bisect machinery.

Mark

[1] http://github.com/augustl/binbin/blob/master/git-walk

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Ævar Arnfjörð Bjarmason]

On Sat, Sep 11, 2010 at 15:56, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> `On Mon, Aug 30, 2010 at 03:08, Jeff King <peff@peff.net> wrote:

>> I don't understand why you have these at all. Just use "git log
>> --format=%H" in your git review above (instead of rev-list), and then
>> you can just do:
>>
>>  git review --grep=whatever
>>  git review -Sfoo
>>  git review file
>>
>> Or am I missing something subtle?
>
> You're not missing something, my alias was silly because I brainfarted
> and didn't realize I could do $@, not "$@", so now it's:
>
>    review = "!f() { for rev in $(git log --reverse --format=%H $@);
> do git show $rev; done; }; f"
>
> Which means I can do all of the commands you suggested above, thanks!

Hrm, actually in the case of that alias doing:

    git review -M ...

Won't do what I want, because it's `git show` that has to be invoked
by -M. An option like --for-each-invoke-pager (or something) could do
the right thing there.

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Artur Skawina]

On 09/11/10 17:56, Ævar Arnfjörð Bjarmason wrote:
>>> I have this alias in my .gitconfig:
>>>
>>>     review = "!f() { for rev in $(git rev-list --reverse \"$@\"); do
>>> git show $rev; done; }; f"
>>>
>>> I use it after I "git pull" to see what changed, e.g.:
>>>
>>>     git review 49ea7b8..e1ef3c1

> I just like the UI of having each commit "pop up" where I can either
> page up/down within the commit, or dismiss it with "q" and go to the
> next one.

Something like

$ LESS="$LESS +/^commit " git whatchanged -p --reverse ORIG_HEAD..

will let you jump between the commits using 'n' for next, 'N' for
previous.

Not 100% the same, i know, but much faster than your solution.
(mostly because of pipelining; while you're reviewing one change
the next commits are already being prepared. For deep, but filtered
queries, i often do it more explicitly as "git..|bag|less +/^commit"
to avoid the stalls.)

artur

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I just like the UI of having each commit "pop up" where I can either
> page up/down within the commit, or dismiss it with "q" and go to the
> next one.
>
> You can't do both of those in a pager, up/down goes across commits,
> and "q" quits the whole pager.

I would throw this not into the incomplete scriptability category but into
the "user does not know how to use his pager" category.  With "/^commit .*"
you can not only advance to the next commit with "/<RET>", you can go back
to the previous one with "?<RET>", and keep going in the same direction
with "n".

>>> Maybe we should have something like:
>>>
>>>     git log --for-each=less a..b

If anything that shouldn't be an option to "log", but to "rev-list", as
you are aiming for scriptability.  But

    git rev-list --for-each='cmd that takes commit as its argument'

wouldn't be much of an improvement over "git rev-list | xargs -n1 'cmd'"
pipeline from the _scriptability_ point of view.

So no.

Re: RFC: Adding an option to log-like commands to call an external command for each revision

[Ævar Arnfjörð Bjarmason]

On Sun, Sep 12, 2010 at 21:25, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I just like the UI of having each commit "pop up" where I can either
>> page up/down within the commit, or dismiss it with "q" and go to the
>> next one.
>>
>> You can't do both of those in a pager, up/down goes across commits,
>> and "q" quits the whole pager.
>
> I would throw this not into the incomplete scriptability category but into
> the "user does not know how to use his pager" category.  With "/^commit .*"
> you can not only advance to the next commit with "/<RET>", you can go back
> to the previous one with "?<RET>", and keep going in the same direction
> with "n".

That was already suggested, but I prefer not to see anything else in
the pager myself. E.g. if I have a large terminal window I want to see
*only* the pertinent commit in that entire window, nothing else.

Which is why I still use my git-review alias.

> wouldn't be much of an improvement over "git rev-list | xargs -n1 'cmd'"
> pipeline from the _scriptability_ point of view.
>
> So no.

Fine, I don't think it should be in core either after this
discussion. So we're in agreement.

The problem I was trying to solve was solved with Jeff's suggestion.