Newbie contribution idea for 'git log --children': input requested

Newbie contribution idea for 'git log --children': input requested

[Jacob Stopak]

Hello! This is my first idea for a suggested code change, and I'm
seeking some input on whether it makes sense to address, and if
so, a bit of help with how to implement.

I was futzing with "git log --children" and noticed that the
output doesn't display children for the HEAD commit in a
detached HEAD state (or even when HEAD points to a branch tip
that in fact does have a child commit that is ahead of the
branch).

I assume the children of the HEAD commit don't display because
of the way Git starts parsing backward in a parent-wise
direction, so the children of the HEAD commit would never be
found.

But it seems like displaying the children of the HEAD commit
_when they exist_ would be useful for users who have checked out
a commit somewhere in the middle of a long commit history.

In this case, a user might want to be able to easily find and
check out a commit in the "child" direction without having to
go all the way back up to the branch tip (or nearest tag) to
see commits between the detached HEAD and the branch tip.

I'm not sure if this really qualifies as a "bug" per se, or if
there is some way to do this that I'm just missing, or if it goes
against the way Git's rev lists work.

But I'm wondering if it could be addressed by simply adding a
discrete check to see if any commits point to HEAD as a parent,
(only when the --children option is used of course), and if so,
populate those ids into the log message.

It does seem to make sense that ALL commits with children would
show those children in the log when the --children option is
used... The current behavior seems to be that all commits _except_
HEAD will display their children.

Thx for your thoughts!

Best,
Jack

Re: Newbie contribution idea for 'git log --children': input requested

[Junio C Hamano]

Jacob Stopak <jacob@initialcommit.io> writes:

> I'm not sure if this really qualifies as a "bug" per se, or if
> there is some way to do this that I'm just missing, or if it goes
> against the way Git's rev lists work.

As it goes against the way GIt's rev lists work, you can call that
"working in the way it was designed", or a "design bug".

> But I'm wondering if it could be addressed by simply adding a
> discrete check to see if any commits point to HEAD as a parent,
> (only when the --children option is used of course), and if so,
> populate those ids into the log message.

In a history where all commits and their parent-child relationship
can be enumerated and the links can be reversed in-core, it would be
trivial to implement.  The challenge is to make sure it works
without wasting unusably large resources to do so in a real world
project.  Having a commit-graph might help.

Thanks.

Re: Newbie contribution idea for 'git log --children': input requested

[Jeff King]

On Sun, Sep 11, 2022 at 05:41:21PM -0700, Junio C Hamano wrote:

> > But I'm wondering if it could be addressed by simply adding a
> > discrete check to see if any commits point to HEAD as a parent,
> > (only when the --children option is used of course), and if so,
> > populate those ids into the log message.
> 
> In a history where all commits and their parent-child relationship
> can be enumerated and the links can be reversed in-core, it would be
> trivial to implement.  The challenge is to make sure it works
> without wasting unusably large resources to do so in a real world
> project.  Having a commit-graph might help.

I talked a little with Jacob about this at the contributor summit. As
I don't think I've ever used --children myself, I was curious about the
use case. :)

I think it is that one might want to do something like:

   git log --children $some_old_oid

and get a better idea of what was going on around the commit, both
before and after. And there I think that yeah, enumerating all available
commits to build the reverse index would make sense.

I don't think it would be a strict fix for --children, though, for two
reasons:

  - the current --children is free-ish because we're marking the commits
    as we traverse, so no extra parsing is required (just some storage
    for the reverse index)

  - the semantics aren't quite the same. One obvious issue is that you
    might care more about children reachable from your starting tips
    than arbitrary (possibly unreachable) children in your object store.
    But another is that --children implies parent rewriting, so it's not
    a strict reversal of the parent-child links.

So we'd want to retain the existing --children, and this new mode might
become --children=all or something.

Just sketching out an implementation, I think it is kind of similar to
the "decorate" mechanism, where we build the commit->metadata mapping
ahead of time, and then add output to the commits that we show. So we'd
probably want to load it around the same time we call
load_ref_decorations().

The next question is: what do we put in it?

One idea is to just reverse all the commits we know about. I.e., use
for_each_loose_object() and for_each_packed_object(), similar to the way
"cat-file --batch-all-objects" works. Use lookup_commit_in_graph() to
use the commit-graph when we can, and check object types via
oid_object_info() before trying to parse them (so we can avoid loading
non-commits from disk entirely).

But another idea is to do a _separate_ traversal in order to decide
which commits to care about. Then we have to know which tips to start
that traversal from, and we may end up with something similar to
--decorate-refs in terms of expressing that traversal. But I suspect the
output here is more interesting to the user, because now you can ask
just about reachable children, or children reachable from release tags,
or from a specific branch, etc.

So you could do something like:

  git show --children=refs/heads/* $some_old_commit

and see where we went from $some_old_commit that eventually ended up on
a branch.

But getting back to the original use case, which is about providing
context for a commit: now we have the child commit id, but it's still a
bit of a pain to actually see what it's in it.

I usually solve this by doing a single traversal, and asking the pager
to jump straight to the commit of interest, at which point I can scroll
up and down to see the context. Like:

  git log --color | less +/$some_old_commit

I'm not 100% sure I understand the original use case. But if I do, and
that solves it, I wonder if we could make it more ergonomic somehow.
Likewise, I think using something like "tig" would be nice for viewing
context, but AFAIK it does not have an option to jump to a specific
commit. You can jump by line number, but it would be nice to be able to
do:

  tig --highlight=1234abcd

and get a graph starting at HEAD, but jump immediately to the line for
1234abcd.

-Peff

Re: Newbie contribution idea for 'git log --children': input requested

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Just sketching out an implementation, I think it is kind of similar to
> the "decorate" mechanism, where we build the commit->metadata mapping
> ahead of time, and then add output to the commits that we show. So we'd
> probably want to load it around the same time we call
> load_ref_decorations().

Yup, and yuck.  I personaly doubt that the current --children is all
that useful to begin with, and as you later mention, it would not be
useful without some interactive "jump to" feature, either using the
pager over "log --graph --oneline" output or "tig".

Thanks.

Re: Newbie contribution idea for 'git log --children': input requested

[Jacob Stopak]

On Thu, Sep 15, 2022 at 12:08:10AM -0500, Jeff King wrote:
> 
> I usually solve this by doing a single traversal, and asking the pager
> to jump straight to the commit of interest, at which point I can scroll
> up and down to see the context. Like:
> 
>   git log --color | less +/$some_old_commit
> 
> I'm not 100% sure I understand the original use case. But if I do, and
> that solves it, I wonder if we could make it more ergonomic somehow.

Great points thanks for the details. I do think your method addresses the use
case I was trying to describe, which is being able to quickly see context in
both directions when you jump to a commit far from any branch/tag.

But like you implied, it would be nice to be able to do it with a real command
line option to git log instead of repiping into less, something like:

    git log --scroll-to=commit_id

I peeked into builtin/log.c and saw how it calls setup_pager(); at the end of
cmd_log_init_finish(...). I have a basic understanding now of how the default
pager and pager environment are roped in.

One obvious issue is that different pagers might have different ways (or no way
at all) to auto-scroll to a pattern. But this might be solved by allowing the
user to add their pager option such as +/ to the pager environment, which would
only be applied when the user adds the --scroll-to=commit_id option to git log.

(I guess this would rely on the user knowing what the option format is for their
pager, which isn't ideal... but I assume that it's not great practice to have git
store option formats for external tools like pagers since they could change over
time and then there is the can of worms of which pagers git would even support this
feature for - although it does seem git stores default flags for LESS and LV).

Anyway, then the commit_id would be dynamically added after, probably in a format like
"commit <commit_id>" so that the same pattern in commit messages doesn't match.
 
But I don't think I saw an existing way to pass in dynamic parameters, such as a
commit id, into the pager environment. This ability could potentially be useful
for setting other dynamic pager options as well, which can currently only be
done by stuff like repiping git log's output as in your example.

From what I can tell only static content like flags appear to be able to be set
in the current pager environment, but maybe I'm missing something obvious...

It would be great to get your thoughts.

-Jack

Re: Newbie contribution idea for 'git log --children': input requested

[Jeff King]

On Sat, Sep 17, 2022 at 08:19:01PM -0700, Jacob Stopak wrote:

> On Thu, Sep 15, 2022 at 12:08:10AM -0500, Jeff King wrote:
> > 
> > I usually solve this by doing a single traversal, and asking the pager
> > to jump straight to the commit of interest, at which point I can scroll
> > up and down to see the context. Like:
> > 
> >   git log --color | less +/$some_old_commit
> > 
> > I'm not 100% sure I understand the original use case. But if I do, and
> > that solves it, I wonder if we could make it more ergonomic somehow.
> 
> Great points thanks for the details. I do think your method addresses the use
> case I was trying to describe, which is being able to quickly see context in
> both directions when you jump to a commit far from any branch/tag.

One thing my suggestion doesn't help with is when $some_old_commit is
the current HEAD. Something like:

  git checkout HEAD~50
  git log --magically-start-at=HEAD

wouldn't work, because to get context you need to start traversing from
somewhere else. So really you want to start with "--branches --tags", or
maybe even "--all" as traversal points, and then show whatever commit
(be it HEAD or something else) in context.

> But like you implied, it would be nice to be able to do it with a real command
> line option to git log instead of repiping into less, something like:
> 
>     git log --scroll-to=commit_id
> 
> I peeked into builtin/log.c and saw how it calls setup_pager(); at the end of
> cmd_log_init_finish(...). I have a basic understanding now of how the default
> pager and pager environment are roped in.
> 
> One obvious issue is that different pagers might have different ways (or no way
> at all) to auto-scroll to a pattern. But this might be solved by allowing the
> user to add their pager option such as +/ to the pager environment, which would
> only be applied when the user adds the --scroll-to=commit_id option to git log.

Yes, exactly. It's awkward to assume too much about the pager, because
it could be literally anything. I think the worst thing we can do is
stick extra bits on the command-line like this, because it may actually
break other pagers. As you noted, we do have a default LESS variable,
which is OK, because at worst it is simply ignored.

I think we _could_ append "+/whatever" into $LESS, and that would work.
It feels kind of dirty somehow, though.

> Anyway, then the commit_id would be dynamically added after, probably in a format like
> "commit <commit_id>" so that the same pattern in commit messages doesn't match.

Right, and you can anchor it with "+/^commit ...". But of course that
depends on the output format. Something like "--oneline" would have a
partial commit. If done inside Git, it would know the format it will
use. But if you don't mind being a little sloppy, you can probably write
a regex which works well enough.

Taking it all together, a script like this works:

-- >8 --
#!/bin/sh

focus=$1; shift

# use --short rather than --verify so we match --oneline
short=$(git rev-parse --short "$focus") || exit 1

# this regex tries to match a variety of formats:
#
#   - anchor to the left to avoid false positives
#   - make "commit" optional to catch both regular and oneline
#   - skip --graph markers
#   - we can ignore colors; less skips ANSI codes when matching
regex='^[*|\\ ]*(commit )?'

# yuck, there's no way to ask "which pager would you use for git-log".
# Stuffing the value inside $LESS works, but if we override Git's
# internal value, then we have to recreate it.
LESS="${LESS-FRX} +/$regex$short"
export LESS

exec git log --branches --tags "$@"
-- >8 --

Saving that as git-focus and setting the execute bit lets you do stuff
like:

  git checkout HEAD~50
  git focus HEAD

I think this could be done inside Git, though it would be a little
awkward. Maybe it's worth living with that script for a while and seeing
if you like it.

-Peff

Re: Newbie contribution idea for 'git log --children': input requested

[Jacob Stopak]

On Tue, Sep 20, 2022 at 06:28:24PM -0400, Jeff King wrote:
> Taking it all together, a script like this works:
... 
> I think this could be done inside Git, though it would be a little
> awkward. Maybe it's worth living with that script for a while and seeing
> if you like it.
> 
> -Peff

Wow thanks for putting that script together! Looks awesome I'm looking forward
to testing it out - I think it should be fine for my purposes.

Also unrelated - thought of a different patch idea to work on related to the git
shortlog command, so will be submitting that hopefully one of these days... o.O