{"thread":{"id":"51573","subject":"Simplify-by-decoration with decorate-refs-exclude","startedAt":"2019-08-02T08:47:57Z","lastAt":"2019-09-09T18:17:47Z","messageCount":11,"participants":["Étienne SERVAIS","René Scharfe","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"379804","messageId":"D941A96E-E5A8-4C86-A200-0BECA30216D1@voucoux.fr","threadId":"51573","inReplyTo":null,"subject":"Simplify-by-decoration with decorate-refs-exclude","fromName":"Étienne SERVAIS","fromEmail":"etienne.servais@voucoux.fr","sentAt":"2019-08-02T08:47:51Z","receivedAt":"2019-08-02T08:47:57Z","isPatch":false,"sender":{"key":"etienne.servais@voucoux.fr","avatar":null},"body":"Hi there,\n\nI've asked this question yesterday on stackoverflow: \nhttps://stackoverflow.com/q/57305719/2622010\nAnd have been confirmed it does look like a bug. \nUnfortunately I'm unable to update to latest git revision but a search through the release notes didn't show anything related to this problem. \n\nI'm working in a git (v2.19.1) repo with lots of tags and branches. To get a glance of the git tree, I'd like to use the `--simplify-by-decoration` option while excluding some of the tags with `--decorate-refs-exclude=` of `git log`, but, as per [the documentation](https://git-scm.com/docs/git-log/2.22.0#Documentation/git-log.txt---simplify-by-decoration):\n\n> --simplify-by-decoration\n> \n> Commits that are referred by some branch or tag are selected.\n\nThus every tags are selected even those that are excluded from the decoration by the pattern.\n\nThus, when I enter\n\n```\ngit log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/'\n```\nThe selected tags are properly excluded but once I add the `simplify-by-decoration` option\n\n```\ngit log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/' --simplify-by-decoration\n```\nThe excluded tags pop back again.\n\nIs there a workaround?\n\nÉtienne\n--\n+33 6 45 98 22 65\nEnvoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.\n--\n+33 6 45 98 22 65\nEnvoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.\n"},{"id":"379840","messageId":"37283d4e-3f79-a6b1-425a-f90704fbcce2@web.de","threadId":"51573","inReplyTo":"D941A96E-E5A8-4C86-A200-0BECA30216D1@voucoux.fr","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-02T16:52:51Z","receivedAt":"2019-08-02T16:53:04Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.19 um 10:47 schrieb Étienne SERVAIS:\n> Thus, when I enter\n>\n> ```\n> git log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/'\n> ```\n> The selected tags are properly excluded but once I add the\n> `simplify-by-decoration` option\n>\n> ```\n> git log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/' --simplify-by-decoration\n> ```\n> The excluded tags pop back again.\n\nDoes this help?\n\n-- >8 --\nSubject: [PATCH] revision: load decorations lazily for --simplify-by-decoration\n\nLet setup_revisions() and friends respect a filtered set of decoration\nrefs loaded by callers by postponing its own load_ref_decorations() call\nto just before decorations are used to simplify history. That function\nonly does any actual work the first time it is called.\n\nThis allows using the revision option --simplify-by-decoration together\nwith the log option --decorate-refs-exclude and having it simplify over\nthe restricted set of refs.\n\nReported-by: Étienne SERVAIS \nSigned-off-by: René Scharfe \n---\n revision.c | 8 +++++++-\n t/t4202-log.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 07412297f0..d3456c959b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -633,6 +633,13 @@ static int rev_compare_tree(struct rev_info *revs,\n \t\treturn REV_TREE_OLD;\n\n \tif (revs->simplify_by_decoration) {\n+\t\t/*\n+\t\t * Load decorations lazily; later calls have no effect.\n+\t\t * This gives callers a chance to load a restricted set\n+\t\t * beforehand.\n+\t\t */\n+\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n+\n \t\t/*\n \t\t * If we are simplifying by decoration, then the commit\n \t\t * is worth showing if it has a tag pointing at it.\n@@ -2063,7 +2070,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->simplify_by_decoration = 1;\n \t\trevs->limited = 1;\n \t\trevs->prune = 1;\n-\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \t} else if (!strcmp(arg, \"--date-order\")) {\n \t\trevs->sort_order = REV_SORT_BY_COMMIT_DATE;\n \t\trevs->topo_order = 1;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex c20209324c..bb66d1d93c 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -837,6 +837,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \ttest_cmp expect.decorate actual\n '\n\n+test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n+\tcat >expect.decorate <<-\\EOF &&\n+\tMerge-tag-reach (HEAD -> master)\n+\treach (tag: reach, reach)\n+\tseventh (tag: seventh)\n+\tMerge-branch-tangle\n+\tMerge-branch-side-early-part-into-tangle (tangle)\n+\ttangle-a (tag: tangle-a)\n+\tEOF\n+\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs-exclude=\"*octopus*\" \\\n+\t\t--simplify-by-decoration >actual &&\n+\ttest_cmp expect.decorate actual\n+'\n+\n test_expect_success 'log.decorate config parsing' '\n \tgit log --oneline --decorate=full >expect.full &&\n \tgit log --oneline --decorate=short >expect.short &&\n--\n2.22.0\n"},{"id":"379845","messageId":"xmqq36ijl6qu.fsf@gitster-ct.c.googlers.com","threadId":"51573","inReplyTo":"37283d4e-3f79-a6b1-425a-f90704fbcce2@web.de","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-02T19:14:33Z","receivedAt":"2019-08-02T19:14:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 02.08.19 um 10:47 schrieb Étienne SERVAIS:\n>> Thus, when I enter\n>>\n>> ```\n>> git log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/'\n>> ```\n>> The selected tags are properly excluded but once I add the\n>> `simplify-by-decoration` option\n>>\n>> ```\n>> git log --oneline --graph --decorate=full --decorate-refs-exclude='refs/tags/' --simplify-by-decoration\n>> ```\n>> The excluded tags pop back again.\n>\n> Does this help?\n\nI can see how this would help, but it somehow feels a bit brittle\nto rely on where the decorations get loaded.\n\nI wonder if it would help to move the ability to handle decoration\nfilter down from the log layer to revisions.c API layer.\n\nIt looks to me that this caller of setup_revisions() can prepare\ndecoration_filter before it calls setup_revisions(); we can let the\nrevisions.c layer call load_ref_decorations() in setup_revisions()\nif that is the case, no?\n\nOther two callers of load_ref_decorations() are deep inside pretty.c\nbut I wonder in the longer term if we would want to turn them into\nan \"a lot higher level should have already loaded decorations\"\nassert.\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] revision: load decorations lazily for --simplify-by-decoration\n>\n> Let setup_revisions() and friends respect a filtered set of decoration\n> refs loaded by callers by postponing its own load_ref_decorations() call\n> to just before decorations are used to simplify history. That function\n> only does any actual work the first time it is called.\n>\n> This allows using the revision option --simplify-by-decoration together\n> with the log option --decorate-refs-exclude and having it simplify over\n> the restricted set of refs.\n>\n> Reported-by: Étienne SERVAIS \n> Signed-off-by: René Scharfe \n> ---\n> revision.c | 8 +++++++-\n> t/t4202-log.sh | 15 +++++++++++++++\n> 2 files changed, 22 insertions(+), 1 deletion(-)\n>\n> diff --git a/revision.c b/revision.c\n> index 07412297f0..d3456c959b 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -633,6 +633,13 @@ static int rev_compare_tree(struct rev_info *revs,\n> \t\treturn REV_TREE_OLD;\n>\n> \tif (revs->simplify_by_decoration) {\n> +\t\t/*\n> +\t\t * Load decorations lazily; later calls have no effect.\n> +\t\t * This gives callers a chance to load a restricted set\n> +\t\t * beforehand.\n> +\t\t */\n> +\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n> +\n> \t\t/*\n> \t\t * If we are simplifying by decoration, then the commit\n> \t\t * is worth showing if it has a tag pointing at it.\n> @@ -2063,7 +2070,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t\trevs->simplify_by_decoration = 1;\n> \t\trevs->limited = 1;\n> \t\trevs->prune = 1;\n> -\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n> \t} else if (!strcmp(arg, \"--date-order\")) {\n> \t\trevs->sort_order = REV_SORT_BY_COMMIT_DATE;\n> \t\trevs->topo_order = 1;\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index c20209324c..bb66d1d93c 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -837,6 +837,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n> \ttest_cmp expect.decorate actual\n> '\n>\n> +test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n> +\tcat >expect.decorate <<-\\EOF &&\n> +\tMerge-tag-reach (HEAD -> master)\n> +\treach (tag: reach, reach)\n> +\tseventh (tag: seventh)\n> +\tMerge-branch-tangle\n> +\tMerge-branch-side-early-part-into-tangle (tangle)\n> +\ttangle-a (tag: tangle-a)\n> +\tEOF\n> +\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--decorate-refs-exclude=\"*octopus*\" \\\n> +\t\t--simplify-by-decoration >actual &&\n> +\ttest_cmp expect.decorate actual\n> +'\n> +\n> test_expect_success 'log.decorate config parsing' '\n> \tgit log --oneline --decorate=full >expect.full &&\n> \tgit log --oneline --decorate=short >expect.short &&\n> --\n> 2.22.0\n"},{"id":"379853","messageId":"5de287e9-a8e8-780a-7d39-2229b61914ca@web.de","threadId":"51573","inReplyTo":"xmqq36ijl6qu.fsf@gitster-ct.c.googlers.com","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-02T20:36:33Z","receivedAt":"2019-08-02T20:36:55Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.19 um 21:14 schrieb Junio C Hamano:\n> I can see how this would help, but it somehow feels a bit brittle\n> to rely on where the decorations get loaded.\n\nRight.\n\n> I wonder if it would help to move the ability to handle decoration\n> filter down from the log layer to revisions.c API layer.\n>\n> It looks to me that this caller of setup_revisions() can prepare\n> decoration_filter before it calls setup_revisions(); we can let the\n> revisions.c layer call load_ref_decorations() in setup_revisions()\n> if that is the case, no?\n\nHaving cmd_log_init_finish() call load_ref_decorations() before\nsetup_revisions() would indeed solve the issue as well. But we need\nto call the latter to check if --pretty=raw was given and avoid loading\ndecorations in that case, don't we?\n\n> Other two callers of load_ref_decorations() are deep inside pretty.c\n> but I wonder in the longer term if we would want to turn them into\n> an \"a lot higher level should have already loaded decorations\"\n> assert.\n\nThis would require that higher level to parse the user format to check\nif %d or %D is present before formatting the first item. Hmm.\n\nRené\n"},{"id":"379857","messageId":"xmqqh86zjmcg.fsf@gitster-ct.c.googlers.com","threadId":"51573","inReplyTo":"5de287e9-a8e8-780a-7d39-2229b61914ca@web.de","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-02T21:20:31Z","receivedAt":"2019-08-02T21:20:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Having cmd_log_init_finish() call load_ref_decorations() before\n> setup_revisions() would indeed solve the issue as well. But we need\n> to call the latter to check if --pretty=raw was given and avoid loading\n> decorations in that case, don't we?\n\nI was thinking about giving an instance of the decoration_filter to\neither rev_info or setup_revision_opt, and moving the call to\nload_ref_decorations() and the decision to make that call from\ncmd_log_init_finish() to setup_revisions().\n\n>> Other two callers of load_ref_decorations() are deep inside pretty.c\n>> but I wonder in the longer term if we would want to turn them into\n>> an \"a lot higher level should have already loaded decorations\"\n>> assert.\n>\n> This would require that higher level to parse the user format to check\n> if %d or %D is present before formatting the first item. Hmm.\n\nYes. Don't we pre-scan what kind of formatting primitives are used\nin the end-user supplied string already to optimize loading of notes\nand source information?\n\n"},{"id":"379868","messageId":"20190802232129.GA811@sigill.intra.peff.net","threadId":"51573","inReplyTo":"xmqqh86zjmcg.fsf@gitster-ct.c.googlers.com","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-08-02T23:21:30Z","receivedAt":"2019-08-02T23:21:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 02, 2019 at 02:20:31PM -0700, Junio C Hamano wrote:\n\n> > This would require that higher level to parse the user format to check\n> > if %d or %D is present before formatting the first item. Hmm.\n> \n> Yes. Don't we pre-scan what kind of formatting primitives are used\n> in the end-user supplied string already to optimize loading of notes\n> and source information?\n\nI think userformat_find_requirements() is what you're looking for.\n\nI do think it might be tricky to find all of the callers who need to use\nit, though. Some of them are not necessarily users of the traversal\nmachinery, but just have a one-off pretty_print_context.\n\n-Peff\n"},{"id":"379873","messageId":"64bc5556-b098-7991-b7c2-3483277bb73c@web.de","threadId":"51573","inReplyTo":"xmqqh86zjmcg.fsf@gitster-ct.c.googlers.com","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-03T06:51:38Z","receivedAt":"2019-08-03T06:51:52Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.08.19 um 23:20 schrieb Junio C Hamano:\n> René Scharfe writes:\n>\n>> Having cmd_log_init_finish() call load_ref_decorations() before\n>> setup_revisions() would indeed solve the issue as well. But we need\n>> to call the latter to check if --pretty=raw was given and avoid loading\n>> decorations in that case, don't we?\n>\n> I was thinking about giving an instance of the decoration_filter to\n> either rev_info or setup_revision_opt, and moving the call to\n> load_ref_decorations() and the decision to make that call from\n> cmd_log_init_finish() to setup_revisions().\n\nSure, but we'd need to move the code to handle the raw format as well, no?\nPerhaps like this? It depends on callers of parse_revision_opt() calling\nsetup_revisions() before using decorations. And it may have other side\neffects; I'm not comfortable with this change.\n\n---\n builtin/log.c | 10 +++++-----\n revision.c | 17 ++++++++++++++++-\n revision.h | 4 ++++\n t/t4202-log.sh | 15 +++++++++++++++\n 4 files changed, 40 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1cf9e37736..5a1544276f 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -203,6 +203,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n \t\t\t PARSE_OPT_KEEP_DASHDASH);\n\n+\trev->decoration_filter = &decoration_filter;\n+\trev->decoration_given = decoration_given;\n+\trev->decoration_style = decoration_style;\n+\n \tif (quiet)\n \t\trev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;\n \targc = setup_revisions(argc, argv, rev, opt);\n@@ -245,16 +249,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t * \"log --pretty=raw\" is special; ignore UI oriented\n \t\t * configuration variables such as decoration.\n \t\t */\n-\t\tif (!decoration_given)\n-\t\t\tdecoration_style = 0;\n \t\tif (!rev->abbrev_commit_given)\n \t\t\trev->abbrev_commit = 0;\n \t}\n\n-\tif (decoration_style) {\n+\tif (rev->decoration_style)\n \t\trev->show_decorations = 1;\n-\t\tload_ref_decorations(&decoration_filter, decoration_style);\n-\t}\n\n \tif (rev->line_level_traverse)\n \t\tline_log_init(rev, line_cb.prefix, &line_cb.args);\ndiff --git a/revision.c b/revision.c\nindex 07412297f0..709d6a273c 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2063,7 +2063,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->simplify_by_decoration = 1;\n \t\trevs->limited = 1;\n \t\trevs->prune = 1;\n-\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \t} else if (!strcmp(arg, \"--date-order\")) {\n \t\trevs->sort_order = REV_SORT_BY_COMMIT_DATE;\n \t\trevs->topo_order = 1;\n@@ -2716,6 +2715,22 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (revs->expand_tabs_in_log < 0)\n \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n\n+\tif (revs->pretty_given && revs->commit_format == CMIT_FMT_RAW) {\n+\t\t/*\n+\t\t * \"log --pretty=raw\" is special; ignore UI oriented\n+\t\t * configuration variables such as decoration.\n+\t\t */\n+\t\tif (!revs->decoration_given)\n+\t\t\trevs->decoration_style = 0;\n+\t}\n+\n+\tif (revs->decoration_style)\n+\t\tload_ref_decorations(revs->decoration_filter,\n+\t\t\t\t revs->decoration_style);\n+\telse if (revs->simplify_by_decoration)\n+\t\tload_ref_decorations(revs->decoration_filter,\n+\t\t\t\t DECORATE_SHORT_REFS);\n+\n \treturn left;\n }\n\ndiff --git a/revision.h b/revision.h\nindex 4134dc6029..67ffe095a9 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -186,6 +186,7 @@ struct rev_info {\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\n \t\t\tabbrev_commit_given:1,\n+\t\t\tdecoration_given:1,\n \t\t\tzero_commit:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n@@ -269,6 +270,9 @@ struct rev_info {\n \t/* line level range that we are chasing */\n \tstruct decoration line_log_data;\n\n+\tstruct decoration_filter *decoration_filter;\n+\tint decoration_style;\n+\n \t/* copies of the parent lists, for --full-diff display */\n \tstruct saved_parents *saved_parents_slab;\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex c20209324c..bb66d1d93c 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -837,6 +837,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \ttest_cmp expect.decorate actual\n '\n\n+test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n+\tcat >expect.decorate <<-\\EOF &&\n+\tMerge-tag-reach (HEAD -> master)\n+\treach (tag: reach, reach)\n+\tseventh (tag: seventh)\n+\tMerge-branch-tangle\n+\tMerge-branch-side-early-part-into-tangle (tangle)\n+\ttangle-a (tag: tangle-a)\n+\tEOF\n+\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs-exclude=\"*octopus*\" \\\n+\t\t--simplify-by-decoration >actual &&\n+\ttest_cmp expect.decorate actual\n+'\n+\n test_expect_success 'log.decorate config parsing' '\n \tgit log --oneline --decorate=full >expect.full &&\n \tgit log --oneline --decorate=short >expect.short &&\n--\n2.22.0\n"},{"id":"379880","messageId":"xmqqwofuia0x.fsf@gitster-ct.c.googlers.com","threadId":"51573","inReplyTo":"64bc5556-b098-7991-b7c2-3483277bb73c@web.de","subject":"Re: Simplify-by-decoration with decorate-refs-exclude","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-03T14:44:14Z","receivedAt":"2019-08-03T14:44:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Sure, but we'd need to move the code to handle the raw format as well, no?\n> Perhaps like this? It depends on callers of parse_revision_opt() calling\n> setup_revisions() before using decorations. And it may have other side\n> effects; I'm not comfortable with this change.\n\nHmph, fair enough. I missed the fact that we do want to keep\n\"--pretty=raw\" being \"special\" but only in \"log\"; a move in this\ndirection teaches the revision walk API layer about it, which is\nprobably not a good idea.\n\nThanks for sanity checking.\n\n"},{"id":"382049","messageId":"ea3fed37-a465-6a26-1035-6885015261b7@web.de","threadId":"51573","inReplyTo":"xmqqwofuia0x.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 1/2] log: test --decorate-refs-exclude with --simplify-by-decoration","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-09-08T17:58:41Z","receivedAt":"2019-09-08T17:58:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Demonstrate that a decoration filter given with --decorate-refs-exclude\nis inadvertently overruled by --simplify-by-decoration.\n\nReported-by: Étienne SERVAIS \nSigned-off-by: René Scharfe \n---\n t/t4202-log.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex c20209324c..01c95d1375 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -837,6 +837,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \ttest_cmp expect.decorate actual\n '\n\n+test_expect_failure 'decorate-refs-exclude and simplify-by-decoration' '\n+\tcat >expect.decorate <<-\\EOF &&\n+\tMerge-tag-reach (HEAD -> master)\n+\treach (tag: reach, reach)\n+\tseventh (tag: seventh)\n+\tMerge-branch-tangle\n+\tMerge-branch-side-early-part-into-tangle (tangle)\n+\ttangle-a (tag: tangle-a)\n+\tEOF\n+\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs-exclude=\"*octopus*\" \\\n+\t\t--simplify-by-decoration >actual &&\n+\ttest_cmp expect.decorate actual\n+'\n+\n test_expect_success 'log.decorate config parsing' '\n \tgit log --oneline --decorate=full >expect.full &&\n \tgit log --oneline --decorate=short >expect.short &&\n--\n2.23.0\n"},{"id":"382050","messageId":"c0e01bd5-667e-1f29-f355-dd0eda8c8e6e@web.de","threadId":"51573","inReplyTo":"xmqqwofuia0x.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 2/2] log-tree: call load_ref_decorations() in get_name_decoration()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-09-08T17:58:51Z","receivedAt":"2019-09-08T17:58:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Load a default set of ref name decorations at the first lookup. This\nfrees direct and indirect callers from doing so. They can still do it\nif they want to use a filter or are interested in full decorations\ninstead of the default short ones -- the first load_ref_decorations()\ncall wins.\n\nThis means that the load in builtin/log.c::cmd_log_init_finish() is\nrespected even if --simplify-by-decoration is given, as the previously\ndominating earlier load in handle_revision_opt() is gone. So a filter\ngiven with --decorate-refs-exclude is used for simplification in that\ncase, as expected.\n\nSigned-off-by: René Scharfe \n---\n log-tree.c | 1 +\n pretty.c | 2 --\n revision.c | 1 -\n t/t4202-log.sh | 2 +-\n 4 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 1e56df62a7..2d5710e707 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -77,6 +77,7 @@ void add_name_decoration(enum decoration_type type, const char *name, struct obj\n\n const struct name_decoration *get_name_decoration(const struct object *obj)\n {\n+\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \treturn lookup_decoration(&name_decoration, obj);\n }\n\ndiff --git a/pretty.c b/pretty.c\nindex e4ed14effe..b32f036953 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1239,11 +1239,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\tstrbuf_addstr(sb, get_revision_mark(NULL, commit));\n \t\treturn 1;\n \tcase 'd':\n-\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \t\tformat_decorations(sb, commit, c->auto_color);\n \t\treturn 1;\n \tcase 'D':\n-\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \t\tformat_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n \t\treturn 1;\n \tcase 'S':\t\t/* tag/branch like --source */\ndiff --git a/revision.c b/revision.c\nindex 07412297f0..1df3061e95 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2063,7 +2063,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->simplify_by_decoration = 1;\n \t\trevs->limited = 1;\n \t\trevs->prune = 1;\n-\t\tload_ref_decorations(NULL, DECORATE_SHORT_REFS);\n \t} else if (!strcmp(arg, \"--date-order\")) {\n \t\trevs->sort_order = REV_SORT_BY_COMMIT_DATE;\n \t\trevs->topo_order = 1;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 01c95d1375..bb66d1d93c 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -837,7 +837,7 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \ttest_cmp expect.decorate actual\n '\n\n-test_expect_failure 'decorate-refs-exclude and simplify-by-decoration' '\n+test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n \tcat >expect.decorate <<-\\EOF &&\n \tMerge-tag-reach (HEAD -> master)\n \treach (tag: reach, reach)\n--\n2.23.0\n"},{"id":"382084","messageId":"xmqqlfuxjpve.fsf@gitster-ct.c.googlers.com","threadId":"51573","inReplyTo":"ea3fed37-a465-6a26-1035-6885015261b7@web.de","subject":"Re: [PATCH 1/2] log: test --decorate-refs-exclude with --simplify-by-decoration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-09T18:17:41Z","receivedAt":"2019-09-09T18:17:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Demonstrate that a decoration filter given with --decorate-refs-exclude\n> is inadvertently overruled by --simplify-by-decoration.\n>\n> Reported-by: Étienne SERVAIS \n> Signed-off-by: René Scharfe \n> ---\n> t/t4202-log.sh | 15 +++++++++++++++\n> 1 file changed, 15 insertions(+)\n\nLooks vaguely familiar ;-) Thanks for resurrecting it.\n"}]}