{"thread":{"id":"54374","subject":"[RFC PATCH] log: add log.showStat configuration variable","startedAt":"2020-10-08T16:20:36Z","lastAt":"2020-10-12T16:14:38Z","messageCount":7,"participants":["Robert Karszniewicz","Junio C Hamano","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"407164","messageId":"20201008162015.23898-1-avoidr@posteo.de","threadId":"54374","inReplyTo":null,"subject":"[RFC PATCH] log: add log.showStat configuration variable","fromName":"Robert Karszniewicz","fromEmail":"avoidr@posteo.de","sentAt":"2020-10-08T16:20:15Z","receivedAt":"2020-10-08T16:20:36Z","isPatch":true,"sender":{"key":"avoidr@posteo.de","avatar":null},"body":"Changes default behaviour of `git log` and `git show` when no\ncommand-line options are given. Doesn't affect behaviour otherwise (same\nbehaviour as with stash.showStat).\n---\nI've wanted to have `show` and `log` show --stat by default, and I\ncouldn't find any better solution for it. And I've discovered that there\nis stash.showStat, which is exactly what I want. So I wanted to bring\nstash.showStat to `show` and `log`.\n\nSo far, setting log.showStat affects behaviour as described in the\ncommit message.\nBut it does so for `show` and `log` at the same time. I think they\nshould be configurable separately. (log.showStat and show.showStat)\n\nBefore I do all the work, please tell me if this is the right approach\nso far, and if the feature - when ready - would be accepted. (I'm aware\nthat documentation and tests are missing.)\n\n builtin/log.c | 22 ++++++++++++++++++----\n revision.h    |  1 +\n 2 files changed, 19 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0a7ed4bef9..225252f0b4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -45,6 +45,7 @@ static int default_abbrev_commit;\n static int default_show_root = 1;\n static int default_follow;\n static int default_show_signature;\n+static int default_show_stat;\n static int default_encode_email_headers = 1;\n static int decoration_style;\n static int decoration_given;\n@@ -151,6 +152,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n \trev->show_signature = default_show_signature;\n+\trev->show_stat = default_show_stat;\n \trev->encode_email_headers = default_encode_email_headers;\n \trev->diffopt.flags.allow_textconv = 1;\n \n@@ -488,6 +490,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tdefault_show_signature = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"log.showstat\")) {\n+\t\tdefault_show_stat = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\n@@ -607,8 +613,11 @@ static void show_setup_revisions_tweak(struct rev_info *rev,\n \t\t\trev->dense_combined_merges = 1;\n \t\t}\n \t}\n-\tif (!rev->diffopt.output_format)\n+\tif (!rev->diffopt.output_format) {\n \t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n+\t\tif (rev->show_stat)\n+\t\t\trev->diffopt.output_format |= DIFF_FORMAT_DIFFSTAT;\n+\t}\n }\n \n int cmd_show(int argc, const char **argv, const char *prefix)\n@@ -727,9 +736,14 @@ static void log_setup_revisions_tweak(struct rev_info *rev,\n \t    rev->prune_data.nr == 1)\n \t\trev->diffopt.flags.follow_renames = 1;\n \n-\t/* Turn --cc/-c into -p --cc/-c when -p was not given */\n-\tif (!rev->diffopt.output_format && rev->combine_merges)\n-\t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n+\tif (!rev->diffopt.output_format) {\n+\t\t/* Turn --cc/-c into -p --cc/-c when -p was not given */\n+\t\tif (rev->combine_merges)\n+\t\t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n+\n+\t\tif (rev->show_stat)\n+\t\t\trev->diffopt.output_format |= DIFF_FORMAT_DIFFSTAT;\n+\t}\n \n \tif (rev->first_parent_only && rev->ignore_merges < 0)\n \t\trev->ignore_merges = 0;\ndiff --git a/revision.h b/revision.h\nindex f6bf860d19..e402c519d8 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -204,6 +204,7 @@ struct rev_info {\n \t\t\tshow_merge:1,\n \t\t\tshow_notes_given:1,\n \t\t\tshow_signature:1,\n+\t\t\tshow_stat:1,\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\n \t\t\tabbrev_commit_given:1,\n-- \n2.28.0\n\n"},{"id":"407172","messageId":"xmqq1ri8y4zl.fsf@gitster.c.googlers.com","threadId":"54374","inReplyTo":"20201008162015.23898-1-avoidr@posteo.de","subject":"Re: [RFC PATCH] log: add log.showStat configuration variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-08T17:58:22Z","receivedAt":"2020-10-08T17:58:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Karszniewicz <avoidr@posteo.de> writes:\n\n> Changes default behaviour of `git log` and `git show` when no\n> command-line options are given. Doesn't affect behaviour otherwise (same\n> behaviour as with stash.showStat).\n> ---\n> I've wanted to have `show` and `log` show --stat by default, and I\n> couldn't find any better solution for it. And I've discovered that there\n> is stash.showStat, which is exactly what I want. So I wanted to bring\n> stash.showStat to `show` and `log`.\n\nI would be happy if I can configure my \"git show\" to \n\n - show not just patch but stat by default;\n - keep showing nothing when told to be silent with \"git show -s\"\n\nindependently what happens to my \"git log\".  Specifically, I do not\nwant to see a configuration that I use to tweak \"git show\" the way I\nwant (see above) to make my \"git log\" to become \"git log --stat\".\n\nAnd why is \"stat\" so special?  I am sure there are people who want\nto do --numstat or --summary or combinations of these by default,\nso I doubt that a new bit in rev_info structure is a good way to go.\n\n> So far, setting log.showStat affects behaviour as described in the\n> commit message.\n> But it does so for `show` and `log` at the same time. I think they\n> should be configurable separately. (log.showStat and show.showStat)\n\nAbsolutely.\n\n> Before I do all the work, please tell me if this is the right approach\n> so far, and if the feature - when ready - would be accepted. (I'm aware\n> that documentation and tests are missing.)\n\nNobody will get such a guarantee.  A good test to see if a topic is\nworth spending the reviewers' time on is if the authors are willing\nto spend their time whether it will be in the official relesae or it\nwill have to be kept in a private fork for the authors' own use.  If\nit is not good enough that the authors won't keep a fork of Git just\nto use it for themselves, it is hard to imagine that it would be\ngood enough for public consumption.\n\nIn short, make it so useful that we'd come to you begging for it ;-)\n\n> diff --git a/revision.h b/revision.h\n> index f6bf860d19..e402c519d8 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -204,6 +204,7 @@ struct rev_info {\n>  \t\t\tshow_merge:1,\n>  \t\t\tshow_notes_given:1,\n>  \t\t\tshow_signature:1,\n> +\t\t\tshow_stat:1,\n>  \t\t\tpretty_given:1,\n>  \t\t\tabbrev_commit:1,\n>  \t\t\tabbrev_commit_given:1,\n\nThe change to the code we saw in builtin/log.c, e.g.\n\n> +\tif (!rev->diffopt.output_format) {\n> +\t\t/* Turn --cc/-c into -p --cc/-c when -p was not given */\n> +\t\tif (rev->combine_merges)\n> +\t\t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n> +\n> +\t\tif (rev->show_stat)\n> +\t\t\trev->diffopt.output_format |= DIFF_FORMAT_DIFFSTAT;\n> +\t}\n\nhints us that this new bit belongs to the group that the\ncombine_merges bit belongs to, not here, no?\n\nBut again, I am not sure if a new bit in rev_info structure is a\ngood way to proceed---after all, when a diff (in various forms, like\n\"patch\", \"stat only\", \"patch and stat\", \"patch, stat, and summary\")\nis shown, how exactly they are shown is not controlled by bits in this\nstructure (rather, that comes from the diffopt field).\n\nThanks.\n"},{"id":"407173","messageId":"bec999ef-5f9c-0ca1-ddd9-70b54b8c51b1@gmail.com","threadId":"54374","inReplyTo":"20201008162015.23898-1-avoidr@posteo.de","subject":"Re: [RFC PATCH] log: add log.showStat configuration variable","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-10-08T18:12:50Z","receivedAt":"2020-10-08T18:12:56Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/8/2020 12:20 PM, Robert Karszniewicz wrote:\n> Changes default behaviour of `git log` and `git show` when no\n> command-line options are given. Doesn't affect behaviour otherwise (same\n> behaviour as with stash.showStat).\n> ---\n> I've wanted to have `show` and `log` show --stat by default, and I\n> couldn't find any better solution for it. And I've discovered that there\n> is stash.showStat, which is exactly what I want. So I wanted to bring\n> stash.showStat to `show` and `log`.\n\nI'm wondering: why should this be a config setting instead of just\na configure alias?\n\n\tgit config --global alias.logs \"log --stat\"\n \nMy personal preference is to use \"--graph --oneline\" by default, so\nI use\n\n\tgit config --global alias.lg \"log --graph --oneline\"\n\nand then type \"git lg ...\" whenever I'm looking at history. I also\nhave an easy way to turn off the graph by using just \"git log\" when\nI want that disabled.\n\n> So far, setting log.showStat affects behaviour as described in the\n> commit message.\n> But it does so for `show` and `log` at the same time. I think they\n> should be configurable separately. (log.showStat and show.showStat)\n> \n> Before I do all the work, please tell me if this is the right approach\n> so far, and if the feature - when ready - would be accepted. (I'm aware\n> that documentation and tests are missing.)\n\nIf this is something we want to do as a config instead of alias,\nI'm wondering if it is worth expanding the scope and thinking about\nthese other arguments (like --graph, --oneline, etc.) and how they\ncould be incorporated into a coherent config system.\n\nI worry that this initial step leads us down a road of slowly adding\none-off config settings for each option when:\n\n 1. aliases exist, and\n 2. it becomes unclear which arguments have configured defaults.\n\nThanks,\n-Stolee\n\n"},{"id":"407283","messageId":"20201010140202.GA20470@HP","threadId":"54374","inReplyTo":"xmqq1ri8y4zl.fsf@gitster.c.googlers.com","subject":"Re: [RFC PATCH] log: add log.showStat configuration variable","fromName":"Robert Karszniewicz","fromEmail":"avoidr@posteo.de","sentAt":"2020-10-10T14:02:02Z","receivedAt":"2020-10-10T23:10:43Z","isPatch":true,"sender":{"key":"avoidr@posteo.de","avatar":null},"body":"On Thu, Oct 08, 2020 at 10:58:22AM -0700, Junio C Hamano wrote:\n> Robert Karszniewicz <avoidr@posteo.de> writes:\n> \n> > Changes default behaviour of `git log` and `git show` when no\n> > command-line options are given. Doesn't affect behaviour otherwise (same\n> > behaviour as with stash.showStat).\n> > ---\n> > I've wanted to have `show` and `log` show --stat by default, and I\n> > couldn't find any better solution for it. And I've discovered that there\n> > is stash.showStat, which is exactly what I want. So I wanted to bring\n> > stash.showStat to `show` and `log`.\n> \n> I would be happy if I can configure my \"git show\" to \n> \n>  - show not just patch but stat by default;\n>  - keep showing nothing when told to be silent with \"git show -s\"\n> \n> independently what happens to my \"git log\".  Specifically, I do not\n> want to see a configuration that I use to tweak \"git show\" the way I\n> want (see above) to make my \"git log\" to become \"git log --stat\".\n> \n> And why is \"stat\" so special?  I am sure there are people who want\n> to do --numstat or --summary or combinations of these by default,\n\nI think --stat is \"special\" because it is the most prominent one,\npopularized by the format-patch format. I've personally come to like\n--stat very much, to me it serves as a TOC of a commit, an extension of\nthe commit message, an essential description of a commit/patch.\n\nIt makes sense for format-patch, but it does not make less sense for\n`show`. (Or does it? I mean, if it is a good idea for distributable\npatch files, why is it less of a good idea for local commits/patches?)\n\nThen we also have `stash-show`, which shows nothing /but/ --stat by\ndefault. Here again: what's the difference between `show` and\n`stash-show`? One might say \"different contexts\", but I don't see them\nbeing that different, really. It just seems inconsistent to me. \n\nAnd that was what I wanted to achieve with my patch - to make it\npossible to make the three formats consistent with each other.\n\nThat's how I think --stat is special. For other \"unknown\"/\"custom\"\noptions I would use an alias, as I already do for variations of `log`\noptions.\nThen why did I still add log.showStat? Because it seemed like too close\nof a relative not to do it. Also because I personally use it and I\nbelieve that commit message and stat belong together and it's an\ninjustice to separate them. And still only because \"--stat is special\".\n\n> > diff --git a/revision.h b/revision.h\n> > index f6bf860d19..e402c519d8 100644\n> > --- a/revision.h\n> > +++ b/revision.h\n> > @@ -204,6 +204,7 @@ struct rev_info {\n> >  \t\t\tshow_merge:1,\n> >  \t\t\tshow_notes_given:1,\n> >  \t\t\tshow_signature:1,\n> > +\t\t\tshow_stat:1,\n> >  \t\t\tpretty_given:1,\n> >  \t\t\tabbrev_commit:1,\n> >  \t\t\tabbrev_commit_given:1,\n> \n> The change to the code we saw in builtin/log.c, e.g.\n> \n> > +\tif (!rev->diffopt.output_format) {\n> > +\t\t/* Turn --cc/-c into -p --cc/-c when -p was not given */\n> > +\t\tif (rev->combine_merges)\n> > +\t\t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n> > +\n> > +\t\tif (rev->show_stat)\n> > +\t\t\trev->diffopt.output_format |= DIFF_FORMAT_DIFFSTAT;\n> > +\t}\n> \n> hints us that this new bit belongs to the group that the\n> combine_merges bit belongs to, not here, no?\n\nRight! I remember being unsure about it, but then the peer pressure of\nall the show* variables made me group it to them.\n\n> \n> But again, I am not sure if a new bit in rev_info structure is a\n> good way to proceed---after all, when a diff (in various forms, like\n> \"patch\", \"stat only\", \"patch and stat\", \"patch, stat, and summary\")\n> is shown, how exactly they are shown is not controlled by bits in this\n> structure (rather, that comes from the diffopt field).\n\nI will try to find a better way.\n\n> \n> Thanks.\n\nThank you for your comments.\n"},{"id":"407295","messageId":"20201011095916.GA14933@HP","threadId":"54374","inReplyTo":"bec999ef-5f9c-0ca1-ddd9-70b54b8c51b1@gmail.com","subject":"Re: [RFC PATCH] log: add log.showStat configuration variable","fromName":"Robert Karszniewicz","fromEmail":"avoidr@posteo.de","sentAt":"2020-10-11T09:59:16Z","receivedAt":"2020-10-11T09:59:27Z","isPatch":true,"sender":{"key":"avoidr@posteo.de","avatar":null},"body":"On Thu, Oct 08, 2020 at 02:12:50PM -0400, Derrick Stolee wrote:\n> On 10/8/2020 12:20 PM, Robert Karszniewicz wrote:\n> > Changes default behaviour of `git log` and `git show` when no\n> > command-line options are given. Doesn't affect behaviour otherwise (same\n> > behaviour as with stash.showStat).\n> > ---\n> > I've wanted to have `show` and `log` show --stat by default, and I\n> > couldn't find any better solution for it. And I've discovered that there\n> > is stash.showStat, which is exactly what I want. So I wanted to bring\n> > stash.showStat to `show` and `log`.\n> \n> I'm wondering: why should this be a config setting instead of just\n> a configure alias?\n\nI answered this in the reply to Junio C Hamano.\n\nActually, the first thing I tried, was make an alias named after the git\ncommand, like so:\n\n  git config --global alias.show \"show --stat\"\n  git config --global alias.log \"log --stat\"\n\nBut that didn't work. Why, actually? We're used to it from our POSIX\nshells, and other places I can't think of, but it feels familiar.\nPerhaps this would be a good way to enable changing default behaviour of\neach git command without having to change anything about config\nhandling? Would this be difficult to do?\n\n> If this is something we want to do as a config instead of alias,\n> I'm wondering if it is worth expanding the scope and thinking about\n> these other arguments (like --graph, --oneline, etc.) and how they\n> could be incorporated into a coherent config system.\n> \n> I worry that this initial step leads us down a road of slowly adding\n> one-off config settings for each option when:\n\nI worried about that, too. But I think the initial step was already in\n2015, when stash.showStat and stash.showPatch were added. No flood of\noptions happened since then? I was actually surprised about it, too,\nthat it took so long until someone wanted to have showStat for show and\nlog, too.\n\n> \n>  1. aliases exist, and\n>  2. it becomes unclear which arguments have configured defaults.\n> \n> Thanks,\n> -Stolee\n> \n\nThank you!\n"},{"id":"407331","messageId":"1f53a7d8-6aa5-e1c7-ecb9-b99a37500034@gmail.com","threadId":"54374","inReplyTo":"20201011095916.GA14933@HP","subject":"Re: [RFC PATCH] log: add log.showStat configuration variable","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-10-12T12:50:35Z","receivedAt":"2020-10-12T12:50:39Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/11/2020 5:59 AM, Robert Karszniewicz wrote:\n> On Thu, Oct 08, 2020 at 02:12:50PM -0400, Derrick Stolee wrote:\n>> On 10/8/2020 12:20 PM, Robert Karszniewicz wrote:\n>>> Changes default behaviour of `git log` and `git show` when no\n>>> command-line options are given. Doesn't affect behaviour otherwise (same\n>>> behaviour as with stash.showStat).\n>>> ---\n>>> I've wanted to have `show` and `log` show --stat by default, and I\n>>> couldn't find any better solution for it. And I've discovered that there\n>>> is stash.showStat, which is exactly what I want. So I wanted to bring\n>>> stash.showStat to `show` and `log`.\n>>\n>> I'm wondering: why should this be a config setting instead of just\n>> a configure alias?\n> \n> I answered this in the reply to Junio C Hamano.\n> \n> Actually, the first thing I tried, was make an alias named after the git\n> command, like so:\n> \n>   git config --global alias.show \"show --stat\"\n>   git config --global alias.log \"log --stat\"\n> \n> But that didn't work. Why, actually? We're used to it from our POSIX\n> shells, and other places I can't think of, but it feels familiar.\n> Perhaps this would be a good way to enable changing default behaviour of\n> each git command without having to change anything about config\n> handling? Would this be difficult to do?\n\nYou can't replace a builtin with an alias, because that creates a\nrecursive loop. Note that I changed the name to \"slog\" for my example.\n\nIf you are going to customize it, then you need to remember your new\nname. But this is something you can do right now without needing to\npatch Git.\n\n>> If this is something we want to do as a config instead of alias,\n>> I'm wondering if it is worth expanding the scope and thinking about\n>> these other arguments (like --graph, --oneline, etc.) and how they\n>> could be incorporated into a coherent config system.\n>>\n>> I worry that this initial step leads us down a road of slowly adding\n>> one-off config settings for each option when:\n> \n> I worried about that, too. But I think the initial step was already in\n> 2015, when stash.showStat and stash.showPatch were added. No flood of\n> options happened since then? I was actually surprised about it, too,\n> that it took so long until someone wanted to have showStat for show and\n> log, too.\n\nI'm not sure these examples will help your case.\n\nDoes 'stash' have more things that would be beneficial to show\nevery time? If no, then 'stash' is much more specialized than\n'show' and 'log' which have many more options. If yes, then this\nis exactly what we want to avoid happening: an incomplete set of\nconfig options that are tailored to a small subset of options.\n\nWhile my stance is still \"an alias should suffice,\" perhaps it is\nworth investigating the \"status.*\" config options, which include\nthis kind of behavior:\n\n* status.aheadBehind can disable some output normally there by\n  default. (This was created for performance implications.)\n\n* status.showStash enables --show-stash\n\n* status.showUntrackedFiles enables --untracked-files\n\n* status.submoduleSummary interacts with --ignore-submodules\n\nThanks,\n-Stolee\n"},{"id":"407343","messageId":"xmqqtuuzqv4o.fsf@gitster.c.googlers.com","threadId":"54374","inReplyTo":"1f53a7d8-6aa5-e1c7-ecb9-b99a37500034@gmail.com","subject":"Re: [RFC PATCH] log: add log.showStat configuration variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-12T16:14:31Z","receivedAt":"2020-10-12T16:14:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n>> I worried about that, too. But I think the initial step was already in\n>> 2015, when stash.showStat and stash.showPatch were added. No flood of\n>> options happened since then? I was actually surprised about it, too,\n>> that it took so long until someone wanted to have showStat for show and\n>> log, too.\n>\n> I'm not sure these examples will help your case.\n>\n> Does 'stash' have more things that would be beneficial to show\n> every time? If no, then 'stash' is much more specialized than\n> 'show' and 'log' which have many more options. If yes, then this\n> is exactly what we want to avoid happening: an incomplete set of\n> config options that are tailored to a small subset of options.\n\nWell said---I do not have anything more to add to that point that\n'stash' is not a very good example to mimic.\n"}]}