{"thread":{"id":"50361","subject":"[PATCH] reflog: specify default pretty format in config","startedAt":"2019-01-30T14:47:59Z","lastAt":"2019-02-11T19:55:05Z","messageCount":5,"participants":["Roland Hieber","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"368136","messageId":"20190130144744.3248-1-rhi@pengutronix.de","threadId":"50361","inReplyTo":null,"subject":"[PATCH] reflog: specify default pretty format in config","fromName":"Roland Hieber","fromEmail":"rhi@pengutronix.de","sentAt":"2019-01-30T14:47:44Z","receivedAt":"2019-01-30T14:47:59Z","isPatch":true,"sender":{"key":"rhi@pengutronix.de","avatar":"https://avatars.githubusercontent.com/u/192014?v=4"},"body":"The output of git-reflog is currently only customizable by calling\nreflog with --pretty=... or overriding the default \"oneline\" pretty\nformat in the configuration. To save typing, teach reflog to fall back\nto a default format specified in the config option \"reflog.pretty\"\nunless a different pretty format is given on the command line. (This is\nsimilar to the \"format.pretty\" option for git-log and git-show.)\nWhen this config option is not set, fall back to the old default of\n--pretty=oneline --abbrev-hash.\n\nSigned-off-by: Roland Hieber <rhi@pengutronix.de>\n---\n Documentation/git-reflog.txt |  2 ++\n builtin/log.c                | 12 +++++++++---\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt\nindex ff487ff77d..9dccd4bcfd 100644\n--- a/Documentation/git-reflog.txt\n+++ b/Documentation/git-reflog.txt\n@@ -41,6 +41,8 @@ command-line (or `HEAD`, by default). The reflog covers all recent\n actions, and in addition the `HEAD` reflog records branch switching.\n `git reflog show` is an alias for `git log -g --abbrev-commit\n --pretty=oneline`; see linkgit:git-log[1] for more information.\n+The config option `reflog.pretty` is used as the default pretty\n+format if nothing else is specified.\n \n The \"expire\" subcommand prunes older reflog entries. Entries older\n than `expire` time, or entries older than `expire-unreachable` time\ndiff --git a/builtin/log.c b/builtin/log.c\nindex a479642eb9..0fbd324016 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -667,6 +667,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\n+\tint cfg_have_pretty;\n \n \tinit_log_defaults();\n \tgit_config(git_log_config, NULL);\n@@ -676,11 +677,16 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \trev.verbose_header = 1;\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n+\n+\tcfg_have_pretty = git_config_get_string_const(\"reflog.pretty\", &fmt_pretty);\n \tcmd_log_init_defaults(&rev);\n-\trev.abbrev_commit = 1;\n-\trev.commit_format = CMIT_FMT_ONELINE;\n-\trev.use_terminator = 1;\n+\tif (cfg_have_pretty != 0) {\n+\t\trev.abbrev_commit = 1;\n+\t\trev.commit_format = CMIT_FMT_ONELINE;\n+\t\trev.use_terminator = 1;\n+\t}\n \trev.always_show_header = 1;\n+\n \tcmd_log_init_finish(argc, argv, prefix, &rev, &opt);\n \n \treturn cmd_log_walk(&rev);\n-- \n2.20.1\n\n"},{"id":"368154","messageId":"361643257.4118832.1548872569575.JavaMail.zimbra@matthieu-moy.fr","threadId":"50361","inReplyTo":"20190130144744.3248-1-rhi@pengutronix.de","subject":"Re: [PATCH] reflog: specify default pretty format in config","fromName":"Matthieu Moy","fromEmail":"git@matthieu-moy.fr","sentAt":"2019-01-30T18:22:49Z","receivedAt":"2019-01-30T19:06:16Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Roland Hieber <rhi@pengutronix.de> writes:\n\n> The output of git-reflog is currently only customizable by calling\n> reflog with --pretty=... or overriding the default \"oneline\" pretty\n> format in the configuration.\n\nSounds like a good idea to me, but the patch needs a bit more work:\n\n>  Documentation/git-reflog.txt |  2 ++\n\nIt's nice to refer to the config variable in git-reflog.txt, but you\nshould also document it in some Documentation/config/*.txt file,\nincluded from Documentation/config.txt, so that it appears in man\ngit-config.\n\n>  builtin/log.c                | 12 +++++++++---\n>  2 files changed, 11 insertions(+), 3 deletions(-)\n\nThis lacks tests, too (t/*.sh).\n\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -667,6 +667,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct rev_info rev;\n>  \tstruct setup_revision_opt opt;\n> +\tint cfg_have_pretty;\n>  \n>  \tinit_log_defaults();\n>  \tgit_config(git_log_config, NULL);\n> @@ -676,11 +677,16 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n>  \trev.verbose_header = 1;\n>  \tmemset(&opt, 0, sizeof(opt));\n>  \topt.def = \"HEAD\";\n> +\n> +\tcfg_have_pretty = git_config_get_string_const(\"reflog.pretty\", &fmt_pretty);\n>  \tcmd_log_init_defaults(&rev);\n> -\trev.abbrev_commit = 1;\n> -\trev.commit_format = CMIT_FMT_ONELINE;\n> -\trev.use_terminator = 1;\n> +\tif (cfg_have_pretty != 0) {\n\nI'd write just \"if (cfg_have_pretty)\".\n\n>  \trev.always_show_header = 1;\n> +\n>  \tcmd_log_init_finish(argc, argv, prefix, &rev, &opt);\n\nAvoid adding unrelated whitespace changes like this one.\n\nRegards,\n\n-- \nMatthieu Moy\nhttps://matthieu-moy.fr/\n"},{"id":"368231","messageId":"20190131101218.d6aebwuaxdmyjxoj@pengutronix.de","threadId":"50361","inReplyTo":"361643257.4118832.1548872569575.JavaMail.zimbra@matthieu-moy.fr","subject":"Re: [PATCH] reflog: specify default pretty format in config","fromName":"Roland Hieber","fromEmail":"rhi@pengutronix.de","sentAt":"2019-01-31T10:12:18Z","receivedAt":"2019-01-31T10:12:23Z","isPatch":true,"sender":{"key":"rhi@pengutronix.de","avatar":"https://avatars.githubusercontent.com/u/192014?v=4"},"body":"On Wed, Jan 30, 2019 at 07:22:49PM +0100, Matthieu Moy wrote:\n> Roland Hieber <rhi@pengutronix.de> writes:\n> \n> > The output of git-reflog is currently only customizable by calling\n> > reflog with --pretty=... or overriding the default \"oneline\" pretty\n> > format in the configuration.\n> \n> Sounds like a good idea to me, but the patch needs a bit more work:\n\nThanks for the review! I'll follow up with a v2.\n\n - Roland\n\n-- \nRoland Hieber                     | r.hieber@pengutronix.de     |\nPengutronix e.K.                  | https://www.pengutronix.de/ |\nPeiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |\nAmtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |\n"},{"id":"368853","messageId":"20190208183145.11041-1-rhi@pengutronix.de","threadId":"50361","inReplyTo":"20190130144744.3248-1-rhi@pengutronix.de","subject":"[PATCH v2] reflog: specify default pretty format in config","fromName":"Roland Hieber","fromEmail":"rhi@pengutronix.de","sentAt":"2019-02-08T18:31:45Z","receivedAt":"2019-02-08T18:32:18Z","isPatch":true,"sender":{"key":"rhi@pengutronix.de","avatar":"https://avatars.githubusercontent.com/u/192014?v=4"},"body":"The output of git-reflog(1) is currently only customisable by calling\nreflog with --pretty=... or overriding the \"pretty.oneline\" format in\nthe config, which is currently the default fallback.\n\nTo enhance flexibility and save typing, teach reflog to fall back to a\ndefault format specified in the config option \"reflog.pretty\" unless a\ndifferent pretty format is given on the command line. This behaviour is\nsimilar to the \"format.pretty\" option for git-log(1) and git-show(1).\nWhen \"reflog.pretty\" is not set, fall back to the old default of\n--pretty=oneline --abbrev-hash.\n\nSigned-off-by: Roland Hieber <rhi@pengutronix.de>\n---\nChanges in v1 -> v2:\n - add test\n - add documentation for git-config(1)\n - remove accidental whitespace change\n---\n Documentation/config.txt        |  2 ++\n Documentation/config/reflog.txt |  5 +++++\n Documentation/git-reflog.txt    |  2 ++\n builtin/log.c                   | 11 ++++++++---\n t/t1410-reflog.sh               | 21 +++++++++++++++++++++\n 5 files changed, 38 insertions(+), 3 deletions(-)\n create mode 100644 Documentation/config/reflog.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d87846faa..193c312fe 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -394,6 +394,8 @@ include::config/rebase.txt[]\n \n include::config/receive.txt[]\n \n+include::config/reflog.txt[]\n+\n include::config/remote.txt[]\n \n include::config/remotes.txt[]\ndiff --git a/Documentation/config/reflog.txt b/Documentation/config/reflog.txt\nnew file mode 100644\nindex 000000000..637cd852a\n--- /dev/null\n+++ b/Documentation/config/reflog.txt\n@@ -0,0 +1,5 @@\n+reflog.pretty::\n+\tThe default pretty format used by linkgit:git-reflog[1] if nothing\n+\telse is specified via the `--pretty=` option.  If both are unset,\n+\tgit-reflog falls back to `--pretty=oneline --abbrev-hash`.  See the\n+\tsection Pretty Formats in linkgit:git-log[1] for possible values.\ndiff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt\nindex ff487ff77..9dccd4bcf 100644\n--- a/Documentation/git-reflog.txt\n+++ b/Documentation/git-reflog.txt\n@@ -41,6 +41,8 @@ command-line (or `HEAD`, by default). The reflog covers all recent\n actions, and in addition the `HEAD` reflog records branch switching.\n `git reflog show` is an alias for `git log -g --abbrev-commit\n --pretty=oneline`; see linkgit:git-log[1] for more information.\n+The config option `reflog.pretty` is used as the default pretty\n+format if nothing else is specified.\n \n The \"expire\" subcommand prunes older reflog entries. Entries older\n than `expire` time, or entries older than `expire-unreachable` time\ndiff --git a/builtin/log.c b/builtin/log.c\nindex a479642eb..0592e5076 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -667,6 +667,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\n+\tint cfg_have_pretty;\n \n \tinit_log_defaults();\n \tgit_config(git_log_config, NULL);\n@@ -676,10 +677,14 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \trev.verbose_header = 1;\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n+\n+\tcfg_have_pretty = git_config_get_string_const(\"reflog.pretty\", &fmt_pretty);\n \tcmd_log_init_defaults(&rev);\n-\trev.abbrev_commit = 1;\n-\trev.commit_format = CMIT_FMT_ONELINE;\n-\trev.use_terminator = 1;\n+\tif (cfg_have_pretty != 0) {\n+\t\trev.abbrev_commit = 1;\n+\t\trev.commit_format = CMIT_FMT_ONELINE;\n+\t\trev.use_terminator = 1;\n+\t}\n \trev.always_show_header = 1;\n \tcmd_log_init_finish(argc, argv, prefix, &rev, &opt);\n \ndiff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\nindex ae8a448e3..74abf3d0f 100755\n--- a/t/t1410-reflog.sh\n+++ b/t/t1410-reflog.sh\n@@ -383,4 +383,25 @@ test_expect_success 'expire with multiple worktrees' '\n \t)\n '\n \n+test_expect_success '--pretty format' '\n+\tgit init pretty-formats &&\n+\t(\n+\t\tcd pretty-formats &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\tHEAD@{0} commit (initial): foobar\n+\t\tEOF\n+\t\ttest_tick &&\n+\t\ttest_commit foobar &&\n+\n+\t\tgit reflog -n1 --pretty=\"%gD %gs\" > output &&\n+\t\ttest_cmp expect output &&\n+\n+\t\tgit -c reflog.pretty=\"%gD %gs\" reflog -n1 > output &&\n+\t\ttest_cmp expect output &&\n+\n+\t\tgit -c reflog.pretty=\"%h\" reflog -n1 --pretty=\"%gD %gs\" > output &&\n+\t\ttest_cmp expect output\n+\t)\n+'\n+\n test_done\n-- \n2.19.2\n\n"},{"id":"369028","messageId":"xmqqzhr2gkx9.fsf@gitster-ct.c.googlers.com","threadId":"50361","inReplyTo":"20190208183145.11041-1-rhi@pengutronix.de","subject":"Re: [PATCH v2] reflog: specify default pretty format in config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-11T19:54:58Z","receivedAt":"2019-02-11T19:55:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roland Hieber <rhi@pengutronix.de> writes:\n\n> The output of git-reflog(1) is currently only customisable by calling\n> reflog with --pretty=... or overriding the \"pretty.oneline\" format in\n> the config, which is currently the default fallback.\n\nIt is correct to point out that \"git reflog --pretty=...\" is a valid\nway to get customized output, but I do not think the pretty.<name>\nconfiguration allows you to modify built-in formats line oneline.\n\nDon't overuse 'currently'.  The present tense is about describing\nthe current behaviour by default anyway.\n\n\tThe output from \"git reflog\" can be customized with\n\t\"--pretty=...\" option from the command line.\n\nis sufficient.\n\n> To enhance flexibility and save typing, teach reflog to fall back to a\n> default format specified in the config option \"reflog.pretty\" unless a\n> different pretty format is given on the command line. This behaviour is\n> similar to the \"format.pretty\" option for git-log(1) and git-show(1).\n\nOK.\n\n> When \"reflog.pretty\" is not set, fall back to the old default of\n> --pretty=oneline --abbrev-hash.\n\nI've never heard of the \"--abbrev-hash\" option.  Perhaps you meant\nto say --abbrev-commit?\n\nMake it a habit to try out any program example you write in your log\nmessage or documentation and you'll do much better.\n\n> diff --git a/Documentation/config/reflog.txt b/Documentation/config/reflog.txt\n> new file mode 100644\n> index 000000000..637cd852a\n> --- /dev/null\n> +++ b/Documentation/config/reflog.txt\n> @@ -0,0 +1,5 @@\n> +reflog.pretty::\n> +\tThe default pretty format used by linkgit:git-reflog[1] if nothing\n> +\telse is specified via the `--pretty=` option.  If both are unset,\n> +\tgit-reflog falls back to `--pretty=oneline --abbrev-hash`.  See the\n> +\tsection Pretty Formats in linkgit:git-log[1] for possible values.\n\nLikewise.\n\n> diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt\n> index ff487ff77..9dccd4bcf 100644\n> --- a/Documentation/git-reflog.txt\n> +++ b/Documentation/git-reflog.txt\n> @@ -41,6 +41,8 @@ command-line (or `HEAD`, by default). The reflog covers all recent\n>  actions, and in addition the `HEAD` reflog records branch switching.\n>  `git reflog show` is an alias for `git log -g --abbrev-commit\n>  --pretty=oneline`; see linkgit:git-log[1] for more information.\n> +The config option `reflog.pretty` is used as the default pretty\n> +format if nothing else is specified.\n\nThe existing text in the pre-context hints a possible pitfall in\nimplementing this new feature.  When we defeat the built-in default\nvia the configuration or an explicit --pretty=... option, what would\n(and more importantly, what should) happen to the --abbrev-commit\noption given by the default?\n\nLet's read on.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index a479642eb..0592e5076 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -667,6 +667,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct rev_info rev;\n>  \tstruct setup_revision_opt opt;\n> +\tint cfg_have_pretty;\n>  \n>  \tinit_log_defaults();\n>  \tgit_config(git_log_config, NULL);\n> @@ -676,10 +677,14 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n>  \trev.verbose_header = 1;\n>  \tmemset(&opt, 0, sizeof(opt));\n>  \topt.def = \"HEAD\";\n> +\n> +\tcfg_have_pretty = git_config_get_string_const(\"reflog.pretty\", &fmt_pretty);\n>  \tcmd_log_init_defaults(&rev);\n> -\trev.abbrev_commit = 1;\n> -\trev.commit_format = CMIT_FMT_ONELINE;\n> -\trev.use_terminator = 1;\n> +\tif (cfg_have_pretty != 0) {\n\nThis is misleading.  A natural thinking would go \"unless we have\nconfig, we use the default\", so the following three lines that makes\nthe command use the default setting must be triggered when config\ndoes *NOT* have reflog.pretty, and in C, a non-zero integer is\ngenerally TRUE.\n\nIf you are using the fact that the result of git_config_get_X() is\nnon-zero if the configuration is not found, then fixing the polarity\nof the value of the variable to match its name (or fixing the name\nto match what it means), perhaps like:\n\n\tint needs_default;\n\n\t...\n\tneeds_default = git_config_get_string_const(...);\n\tcmd_log_init_defaults(&rev);\n\tif (needs_default) {\n\t\t... set the --oneline --abbrev-commit ...\n\t}\n\nwould make the logic easier to follow.  Or if you choose to keep the\nname, then\n\n\tcfg_have_pretty = !git_config_get_string_const(...);\n\t...\n\tif (!cfg_have_pretty) {\n\t\t...\n\nI think \"needs_default\" would be a better solution that avoids\nunnecessary negation.\n\n> +\t\trev.abbrev_commit = 1;\n> +\t\trev.commit_format = CMIT_FMT_ONELINE;\n> +\t\trev.use_terminator = 1;\n> +\t}\n\nAnd this answers my earlier question.  In the original code,\nrev.abbrev_commit is always set by default and is not turned off, so\nunlike \"git log --oneline\" that never gets .abbrev_commit=1 (hence\nshows a full object name in the oneline output, unless shortened by\ngiving --abbrev-commit at the same time), \"git reflog --oneline\" would\nstill shorten the object names.\n\nEven with this change, \"git reflog --oneline\" without reflog.pretty\nconfiguration would behave the same as before.  Which is nice.\n\nWhat interests us is how the new feature, i.e.\n\n\tgit -c reflog.pretty=oneline reflog\n\nwould behave.  As we have the config, abbrev_commit is not set.  So\nmy guess is that it would give the full object names in the output?\n\nIn other words,\n\n\tgit reflog --oneline\n\tgit -c reflog.pretty=oneline reflog\n\nwould behave differently.\n\nThat is easily \"fix\"-able by moving the assignment of .abbrev_commit\noutside the conditional.  I am not sure what the best way to justify\nthat behaviour, though.\n\n> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\n> index ae8a448e3..74abf3d0f 100755\n> --- a/t/t1410-reflog.sh\n> +++ b/t/t1410-reflog.sh\n> @@ -383,4 +383,25 @@ test_expect_success 'expire with multiple worktrees' '\n>  \t)\n>  '\n>  \n> +test_expect_success '--pretty format' '\n> +\tgit init pretty-formats &&\n> +\t(\n> +\t\tcd pretty-formats &&\n> +\t\tcat >expect <<-\\EOF &&\n> +\t\tHEAD@{0} commit (initial): foobar\n> +\t\tEOF\n> +\t\ttest_tick &&\n> +\t\ttest_commit foobar &&\n> +\n> +\t\tgit reflog -n1 --pretty=\"%gD %gs\" > output &&\n\nStyle: do not write SP after redirection operator '>' and its target\nfilename.\n\nStyle: our convention is, unless there is a good reason to deviate,\ncall the expected output \"expect\" and actual output \"actual\".\n\n> +\t\ttest_cmp expect output &&\n> +\n> +\t\tgit -c reflog.pretty=\"%gD %gs\" reflog -n1 > output &&\n> +\t\ttest_cmp expect output &&\n> +\n> +\t\tgit -c reflog.pretty=\"%h\" reflog -n1 --pretty=\"%gD %gs\" > output &&\n> +\t\ttest_cmp expect output\n> +\t)\n> +'\n> +\n>  test_done\n\nIf I apply the attached patch on top of yours to restore the\nbehaviour of the command that forces .abbrev_commit to be always on,\nthese tests still pass.  Which means that this test is not covering\nenough.  You would want to make sure \"git -c reflog.pretty=oneline\",\n\"git reflog --pretty=oneline\", and \"git reflog\" behaves the same\nway, at least, I think.\n\n\n[Sample patch---do not blindly squash]\n\n builtin/log.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 619bb06686..00e29409bf 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -670,7 +670,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\n-\tint cfg_have_pretty;\n+\tint needs_default;\n \n \tinit_log_defaults();\n \tgit_config(git_log_config, NULL);\n@@ -681,13 +681,13 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n \n-\tcfg_have_pretty = git_config_get_string_const(\"reflog.pretty\", &fmt_pretty);\n+\tneeds_default = git_config_get_string_const(\"reflog.pretty\", &fmt_pretty);\n \tcmd_log_init_defaults(&rev);\n-\tif (cfg_have_pretty != 0) {\n-\t\trev.abbrev_commit = 1;\n+\tif (needs_default) {\n \t\trev.commit_format = CMIT_FMT_ONELINE;\n \t\trev.use_terminator = 1;\n \t}\n+\trev.abbrev_commit = 1;\n \trev.always_show_header = 1;\n \tcmd_log_init_finish(argc, argv, prefix, &rev, &opt);\n \n\n\n"}]}