{"thread":{"id":"60325","subject":"[PATCH] pretty: fix ref filtering for %(decorate) formats","startedAt":"2023-10-08T20:23:48Z","lastAt":"2023-10-09T18:24:12Z","messageCount":2,"participants":["Andy Koppe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"482830","messageId":"20231008202307.1568477-1-andy.koppe@gmail.com","threadId":"60325","inReplyTo":null,"subject":"[PATCH] pretty: fix ref filtering for %(decorate) formats","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2023-10-08T20:23:07Z","receivedAt":"2023-10-08T20:23:48Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"Mark pretty formats containing \"%(decorate\" as requiring decoration in\nuserformat_find_requirements(), same as \"%d\" and \"%D\".\n\nWithout this, cmd_log_init_finish() didn't invoke load_ref_decorations()\nwith the decoration_filter it puts together, and hence filtering options\nsuch as --decorate-refs were quietly ignored.\n\nAmend one of the %(decorate) checks in t4205-log-pretty-formats.sh to\ntest this.\n\nSigned-off-by: Andy Koppe <andy.koppe@gmail.com>\n---\n pretty.c                      | 4 ++++\n t/t4205-log-pretty-formats.sh | 6 +++---\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 7f3abb676c..cf964b060c 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1961,6 +1961,10 @@ void userformat_find_requirements(const char *fmt, struct userformat_want *w)\n \t\tcase 'D':\n \t\t\tw->decorate = 1;\n \t\t\tbreak;\n+\t\tcase '(':\n+\t\t\tif (starts_with(fmt + 1, \"decorate\"))\n+\t\t\t\tw->decorate = 1;\n+\t\t\tbreak;\n \t\t}\n \t}\n }\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 16626e4fe9..5aabc9f7d8 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -590,9 +590,9 @@ test_expect_success 'pretty format %decorate' '\n \tgit log --format=\"%(decorate:prefix=,suffix=)\" -1 >actual2 &&\n \ttest_cmp expect2 actual2 &&\n \n-\techo \"[ HEAD -> foo; tag: bar; qux ]\" >expect3 &&\n-\tgit log --format=\"%(decorate:prefix=[ ,suffix= ],separator=%x3B )\" \\\n-\t\t-1 >actual3 &&\n+\techo \"[ bar; qux; foo ]\" >expect3 &&\n+\tgit log --format=\"%(decorate:prefix=[ ,suffix= ],separator=%x3B ,tag=)\" \\\n+\t\t--decorate-refs=refs/ -1 >actual3 &&\n \ttest_cmp expect3 actual3 &&\n \n \t# Try with a typo (in \"separator\"), in which case the placeholder should\n-- \n2.42.GIT\n\n"},{"id":"482890","messageId":"xmqq4jiz1woq.fsf@gitster.g","threadId":"60325","inReplyTo":"20231008202307.1568477-1-andy.koppe@gmail.com","subject":"Re: [PATCH] pretty: fix ref filtering for %(decorate) formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-09T18:24:05Z","receivedAt":"2023-10-09T18:24:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Koppe <andy.koppe@gmail.com> writes:\n\n> Mark pretty formats containing \"%(decorate\" as requiring decoration in\n> userformat_find_requirements(), same as \"%d\" and \"%D\".\n\nAh, of course.  The patch makes sense.\n\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index 16626e4fe9..5aabc9f7d8 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -590,9 +590,9 @@ test_expect_success 'pretty format %decorate' '\n>  \tgit log --format=\"%(decorate:prefix=,suffix=)\" -1 >actual2 &&\n>  \ttest_cmp expect2 actual2 &&\n>  \n> -\techo \"[ HEAD -> foo; tag: bar; qux ]\" >expect3 &&\n> -\tgit log --format=\"%(decorate:prefix=[ ,suffix= ],separator=%x3B )\" \\\n> -\t\t-1 >actual3 &&\n> +\techo \"[ bar; qux; foo ]\" >expect3 &&\n> +\tgit log --format=\"%(decorate:prefix=[ ,suffix= ],separator=%x3B ,tag=)\" \\\n> +\t\t--decorate-refs=refs/ -1 >actual3 &&\n>  \ttest_cmp expect3 actual3 &&\n\nThe original test shares the same, but is the order of multiple\ndecorations expected to be stable?  I feel a bit uneasy to see a\ntest that insists multiple things come out in a hardcoded order.\n\nIt is not making anything _worse_, so let's take the patch as-is.\n\nThanks.\n\n>  \t# Try with a typo (in \"separator\"), in which case the placeholder should\n"}]}