{"thread":{"id":"58417","subject":"Newbie contribution idea for 'git log --children': input requested","startedAt":"2022-09-11T23:09:08Z","lastAt":"2022-09-21T00:24:13Z","messageCount":7,"participants":["Jacob Stopak","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"462921","messageId":"Yx5qjPhZ5AHkPHr7@MacBook-Pro-3.local","threadId":"58417","inReplyTo":null,"subject":"Newbie contribution idea for 'git log --children': input requested","fromName":"Jacob Stopak","fromEmail":"jacob@initialcommit.io","sentAt":"2022-09-11T23:09:00Z","receivedAt":"2022-09-11T23:09:08Z","isPatch":false,"sender":{"key":"jacob@initialcommit.io","avatar":"https://avatars.githubusercontent.com/u/49353917?v=4"},"body":"Hello! This is my first idea for a suggested code change, and I'm\nseeking some input on whether it makes sense to address, and if\nso, a bit of help with how to implement.\n\nI was futzing with \"git log --children\" and noticed that the\noutput doesn't display children for the HEAD commit in a\ndetached HEAD state (or even when HEAD points to a branch tip\nthat in fact does have a child commit that is ahead of the\nbranch).\n\nI assume the children of the HEAD commit don't display because\nof the way Git starts parsing backward in a parent-wise\ndirection, so the children of the HEAD commit would never be\nfound.\n\nBut it seems like displaying the children of the HEAD commit\n_when they exist_ would be useful for users who have checked out\na commit somewhere in the middle of a long commit history.\n\nIn this case, a user might want to be able to easily find and\ncheck out a commit in the \"child\" direction without having to\ngo all the way back up to the branch tip (or nearest tag) to\nsee commits between the detached HEAD and the branch tip.\n\nI'm not sure if this really qualifies as a \"bug\" per se, or if\nthere is some way to do this that I'm just missing, or if it goes\nagainst the way Git's rev lists work.\n\nBut I'm wondering if it could be addressed by simply adding a\ndiscrete check to see if any commits point to HEAD as a parent,\n(only when the --children option is used of course), and if so,\npopulate those ids into the log message.\n\nIt does seem to make sense that ALL commits with children would\nshow those children in the log when the --children option is\nused... The current behavior seems to be that all commits _except_\nHEAD will display their children.\n\nThx for your thoughts!\n\nBest,\nJack\n"},{"id":"462924","messageId":"xmqqillth1am.fsf@gitster.g","threadId":"58417","inReplyTo":"Yx5qjPhZ5AHkPHr7@MacBook-Pro-3.local","subject":"Re: Newbie contribution idea for 'git log --children': input requested","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-12T00:41:21Z","receivedAt":"2022-09-12T00:41:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Stopak <jacob@initialcommit.io> writes:\n\n> I'm not sure if this really qualifies as a \"bug\" per se, or if\n> there is some way to do this that I'm just missing, or if it goes\n> against the way Git's rev lists work.\n\nAs it goes against the way GIt's rev lists work, you can call that\n\"working in the way it was designed\", or a \"design bug\".\n\n> But I'm wondering if it could be addressed by simply adding a\n> discrete check to see if any commits point to HEAD as a parent,\n> (only when the --children option is used of course), and if so,\n> populate those ids into the log message.\n\nIn a history where all commits and their parent-child relationship\ncan be enumerated and the links can be reversed in-core, it would be\ntrivial to implement.  The challenge is to make sure it works\nwithout wasting unusably large resources to do so in a real world\nproject.  Having a commit-graph might help.\n\nThanks.\n\n"},{"id":"463032","messageId":"YyKzOk5AQBz1pmAh@coredump.intra.peff.net","threadId":"58417","inReplyTo":"xmqqillth1am.fsf@gitster.g","subject":"Re: Newbie contribution idea for 'git log --children': input requested","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-15T05:08:10Z","receivedAt":"2022-09-15T05:08:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 11, 2022 at 05:41:21PM -0700, Junio C Hamano wrote:\n\n> > But I'm wondering if it could be addressed by simply adding a\n> > discrete check to see if any commits point to HEAD as a parent,\n> > (only when the --children option is used of course), and if so,\n> > populate those ids into the log message.\n> \n> In a history where all commits and their parent-child relationship\n> can be enumerated and the links can be reversed in-core, it would be\n> trivial to implement.  The challenge is to make sure it works\n> without wasting unusably large resources to do so in a real world\n> project.  Having a commit-graph might help.\n\nI talked a little with Jacob about this at the contributor summit. As\nI don't think I've ever used --children myself, I was curious about the\nuse case. :)\n\nI think it is that one might want to do something like:\n\n   git log --children $some_old_oid\n\nand get a better idea of what was going on around the commit, both\nbefore and after. And there I think that yeah, enumerating all available\ncommits to build the reverse index would make sense.\n\nI don't think it would be a strict fix for --children, though, for two\nreasons:\n\n  - the current --children is free-ish because we're marking the commits\n    as we traverse, so no extra parsing is required (just some storage\n    for the reverse index)\n\n  - the semantics aren't quite the same. One obvious issue is that you\n    might care more about children reachable from your starting tips\n    than arbitrary (possibly unreachable) children in your object store.\n    But another is that --children implies parent rewriting, so it's not\n    a strict reversal of the parent-child links.\n\nSo we'd want to retain the existing --children, and this new mode might\nbecome --children=all or something.\n\nJust sketching out an implementation, I think it is kind of similar to\nthe \"decorate\" mechanism, where we build the commit->metadata mapping\nahead of time, and then add output to the commits that we show. So we'd\nprobably want to load it around the same time we call\nload_ref_decorations().\n\nThe next question is: what do we put in it?\n\nOne idea is to just reverse all the commits we know about. I.e., use\nfor_each_loose_object() and for_each_packed_object(), similar to the way\n\"cat-file --batch-all-objects\" works. Use lookup_commit_in_graph() to\nuse the commit-graph when we can, and check object types via\noid_object_info() before trying to parse them (so we can avoid loading\nnon-commits from disk entirely).\n\nBut another idea is to do a _separate_ traversal in order to decide\nwhich commits to care about. Then we have to know which tips to start\nthat traversal from, and we may end up with something similar to\n--decorate-refs in terms of expressing that traversal. But I suspect the\noutput here is more interesting to the user, because now you can ask\njust about reachable children, or children reachable from release tags,\nor from a specific branch, etc.\n\nSo you could do something like:\n\n  git show --children=refs/heads/* $some_old_commit\n\nand see where we went from $some_old_commit that eventually ended up on\na branch.\n\nBut getting back to the original use case, which is about providing\ncontext for a commit: now we have the child commit id, but it's still a\nbit of a pain to actually see what it's in it.\n\nI usually solve this by doing a single traversal, and asking the pager\nto jump straight to the commit of interest, at which point I can scroll\nup and down to see the context. Like:\n\n  git log --color | less +/$some_old_commit\n\nI'm not 100% sure I understand the original use case. But if I do, and\nthat solves it, I wonder if we could make it more ergonomic somehow.\nLikewise, I think using something like \"tig\" would be nice for viewing\ncontext, but AFAIK it does not have an option to jump to a specific\ncommit. You can jump by line number, but it would be nice to be able to\ndo:\n\n  tig --highlight=1234abcd\n\nand get a graph starting at HEAD, but jump immediately to the line for\n1234abcd.\n\n-Peff\n"},{"id":"463054","messageId":"xmqqk064o995.fsf@gitster.g","threadId":"58417","inReplyTo":"YyKzOk5AQBz1pmAh@coredump.intra.peff.net","subject":"Re: Newbie contribution idea for 'git log --children': input requested","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-15T17:08:54Z","receivedAt":"2022-09-15T17:09:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Just sketching out an implementation, I think it is kind of similar to\n> the \"decorate\" mechanism, where we build the commit->metadata mapping\n> ahead of time, and then add output to the commits that we show. So we'd\n> probably want to load it around the same time we call\n> load_ref_decorations().\n\nYup, and yuck.  I personaly doubt that the current --children is all\nthat useful to begin with, and as you later mention, it would not be\nuseful without some interactive \"jump to\" feature, either using the\npager over \"log --graph --oneline\" output or \"tig\".\n\nThanks.\n"},{"id":"463148","messageId":"YyaOJa1FNyWK+M4U.jacob@initialcommit.io","threadId":"58417","inReplyTo":"YyKzOk5AQBz1pmAh@coredump.intra.peff.net","subject":"Re: Newbie contribution idea for 'git log --children': input requested","fromName":"Jacob Stopak","fromEmail":"jacob@initialcommit.io","sentAt":"2022-09-18T03:19:01Z","receivedAt":"2022-09-18T03:19:11Z","isPatch":false,"sender":{"key":"jacob@initialcommit.io","avatar":"https://avatars.githubusercontent.com/u/49353917?v=4"},"body":"On Thu, Sep 15, 2022 at 12:08:10AM -0500, Jeff King wrote:\n> \n> I usually solve this by doing a single traversal, and asking the pager\n> to jump straight to the commit of interest, at which point I can scroll\n> up and down to see the context. Like:\n> \n>   git log --color | less +/$some_old_commit\n> \n> I'm not 100% sure I understand the original use case. But if I do, and\n> that solves it, I wonder if we could make it more ergonomic somehow.\n\nGreat points thanks for the details. I do think your method addresses the use\ncase I was trying to describe, which is being able to quickly see context in\nboth directions when you jump to a commit far from any branch/tag.\n\nBut like you implied, it would be nice to be able to do it with a real command\nline option to git log instead of repiping into less, something like:\n\n    git log --scroll-to=commit_id\n\nI peeked into builtin/log.c and saw how it calls setup_pager(); at the end of\ncmd_log_init_finish(...). I have a basic understanding now of how the default\npager and pager environment are roped in.\n\nOne obvious issue is that different pagers might have different ways (or no way\nat all) to auto-scroll to a pattern. But this might be solved by allowing the\nuser to add their pager option such as +/ to the pager environment, which would\nonly be applied when the user adds the --scroll-to=commit_id option to git log.\n\n(I guess this would rely on the user knowing what the option format is for their\npager, which isn't ideal... but I assume that it's not great practice to have git\nstore option formats for external tools like pagers since they could change over\ntime and then there is the can of worms of which pagers git would even support this\nfeature for - although it does seem git stores default flags for LESS and LV).\n\nAnyway, then the commit_id would be dynamically added after, probably in a format like\n\"commit <commit_id>\" so that the same pattern in commit messages doesn't match.\n \nBut I don't think I saw an existing way to pass in dynamic parameters, such as a\ncommit id, into the pager environment. This ability could potentially be useful\nfor setting other dynamic pager options as well, which can currently only be\ndone by stuff like repiping git log's output as in your example.\n\nFrom what I can tell only static content like flags appear to be able to be set\nin the current pager environment, but maybe I'm missing something obvious...\n\nIt would be great to get your thoughts.\n\n-Jack\n"},{"id":"463354","messageId":"Yyo+iBNF1bydcQt+@coredump.intra.peff.net","threadId":"58417","inReplyTo":"YyaOJa1FNyWK+M4U.jacob@initialcommit.io","subject":"Re: Newbie contribution idea for 'git log --children': input requested","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-20T22:28:24Z","receivedAt":"2022-09-20T22:28:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 17, 2022 at 08:19:01PM -0700, Jacob Stopak wrote:\n\n> On Thu, Sep 15, 2022 at 12:08:10AM -0500, Jeff King wrote:\n> > \n> > I usually solve this by doing a single traversal, and asking the pager\n> > to jump straight to the commit of interest, at which point I can scroll\n> > up and down to see the context. Like:\n> > \n> >   git log --color | less +/$some_old_commit\n> > \n> > I'm not 100% sure I understand the original use case. But if I do, and\n> > that solves it, I wonder if we could make it more ergonomic somehow.\n> \n> Great points thanks for the details. I do think your method addresses the use\n> case I was trying to describe, which is being able to quickly see context in\n> both directions when you jump to a commit far from any branch/tag.\n\nOne thing my suggestion doesn't help with is when $some_old_commit is\nthe current HEAD. Something like:\n\n  git checkout HEAD~50\n  git log --magically-start-at=HEAD\n\nwouldn't work, because to get context you need to start traversing from\nsomewhere else. So really you want to start with \"--branches --tags\", or\nmaybe even \"--all\" as traversal points, and then show whatever commit\n(be it HEAD or something else) in context.\n\n> But like you implied, it would be nice to be able to do it with a real command\n> line option to git log instead of repiping into less, something like:\n> \n>     git log --scroll-to=commit_id\n> \n> I peeked into builtin/log.c and saw how it calls setup_pager(); at the end of\n> cmd_log_init_finish(...). I have a basic understanding now of how the default\n> pager and pager environment are roped in.\n> \n> One obvious issue is that different pagers might have different ways (or no way\n> at all) to auto-scroll to a pattern. But this might be solved by allowing the\n> user to add their pager option such as +/ to the pager environment, which would\n> only be applied when the user adds the --scroll-to=commit_id option to git log.\n\nYes, exactly. It's awkward to assume too much about the pager, because\nit could be literally anything. I think the worst thing we can do is\nstick extra bits on the command-line like this, because it may actually\nbreak other pagers. As you noted, we do have a default LESS variable,\nwhich is OK, because at worst it is simply ignored.\n\nI think we _could_ append \"+/whatever\" into $LESS, and that would work.\nIt feels kind of dirty somehow, though.\n\n> Anyway, then the commit_id would be dynamically added after, probably in a format like\n> \"commit <commit_id>\" so that the same pattern in commit messages doesn't match.\n\nRight, and you can anchor it with \"+/^commit ...\". But of course that\ndepends on the output format. Something like \"--oneline\" would have a\npartial commit. If done inside Git, it would know the format it will\nuse. But if you don't mind being a little sloppy, you can probably write\na regex which works well enough.\n\nTaking it all together, a script like this works:\n\n-- >8 --\n#!/bin/sh\n\nfocus=$1; shift\n\n# use --short rather than --verify so we match --oneline\nshort=$(git rev-parse --short \"$focus\") || exit 1\n\n# this regex tries to match a variety of formats:\n#\n#   - anchor to the left to avoid false positives\n#   - make \"commit\" optional to catch both regular and oneline\n#   - skip --graph markers\n#   - we can ignore colors; less skips ANSI codes when matching\nregex='^[*|\\\\ ]*(commit )?'\n\n# yuck, there's no way to ask \"which pager would you use for git-log\".\n# Stuffing the value inside $LESS works, but if we override Git's\n# internal value, then we have to recreate it.\nLESS=\"${LESS-FRX} +/$regex$short\"\nexport LESS\n\nexec git log --branches --tags \"$@\"\n-- >8 --\n\nSaving that as git-focus and setting the execute bit lets you do stuff\nlike:\n\n  git checkout HEAD~50\n  git focus HEAD\n\nI think this could be done inside Git, though it would be a little\nawkward. Maybe it's worth living with that script for a while and seeing\nif you like it.\n\n-Peff\n"},{"id":"463355","messageId":"YypZpsAlZtt0b2hp.jacob@initialcommit.io","threadId":"58417","inReplyTo":"Yyo+iBNF1bydcQt+@coredump.intra.peff.net","subject":"Re: Newbie contribution idea for 'git log --children': input requested","fromName":"Jacob Stopak","fromEmail":"jacob@initialcommit.io","sentAt":"2022-09-21T00:24:06Z","receivedAt":"2022-09-21T00:24:13Z","isPatch":false,"sender":{"key":"jacob@initialcommit.io","avatar":"https://avatars.githubusercontent.com/u/49353917?v=4"},"body":"On Tue, Sep 20, 2022 at 06:28:24PM -0400, Jeff King wrote:\n> Taking it all together, a script like this works:\n... \n> I think this could be done inside Git, though it would be a little\n> awkward. Maybe it's worth living with that script for a while and seeing\n> if you like it.\n> \n> -Peff\n\nWow thanks for putting that script together! Looks awesome I'm looking forward\nto testing it out - I think it should be fine for my purposes.\n\nAlso unrelated - thought of a different patch idea to work on related to the git\nshortlog command, so will be submitting that hopefully one of these days... o.O\n"}]}