From: "Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Christian Couder" <christian.couder@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Junio C Hamano" <gitster@pobox.com>,
"Hariom Verma" <hariom18599@gmail.com>
Subject: [PATCH v4 0/4] Unify trailers formatting logic for pretty.c and ref-filter.c
Date: Sat, 13 Feb 2021 01:52:39 +0000 [thread overview]
Message-ID: <pull.726.v4.git.1613181163.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.726.v3.git.1612602945.gitgitgadget@gmail.com>
Currently, there exists a separate logic for %(trailers) in "pretty.{c,h}"
and "ref-filter.{c,h}". Both are actually doing the same thing, why not use
the same code for both of them?
This is the 4th version of the patch series focused on unifying the
"%(trailers)" logic for both 'pretty.{c,h}' and 'ref-filter.{c,h}'. So, we
can have one logic for trailers.
v4 changes:
* improved tests
Hariom Verma (4):
t6300: use function to test trailer options
pretty.c: refactor trailer logic to `format_set_trailers_options()`
pretty.c: capture invalid trailer argument
ref-filter: use pretty.c logic for trailers
Documentation/git-for-each-ref.txt | 8 +-
pretty.c | 98 +++++++++------
pretty.h | 12 ++
ref-filter.c | 36 +++---
t/t6300-for-each-ref.sh | 185 ++++++++++++++++++++++-------
5 files changed, 236 insertions(+), 103 deletions(-)
base-commit: 328c10930387d301560f7cbcd3351cc485a13381
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-726%2Fharry-hov%2Funify-trailers-logic-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-726/harry-hov/unify-trailers-logic-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/726
Range-diff vs v3:
-: ------------ > 1: 410b02dbad20 t6300: use function to test trailer options
1: 81030f00b11b = 2: fd275fed8347 pretty.c: refactor trailer logic to `format_set_trailers_options()`
2: f4a6b2df1444 = 3: 073c75dc4494 pretty.c: capture invalid trailer argument
3: 47d89f872314 ! 4: 9ec989176993 ref-filter: use pretty.c logic for trailers
@@ ref-filter.c: static int subject_atom_parser(const struct ref_format *format, st
## t/t6300-for-each-ref.sh ##
-@@ t/t6300-for-each-ref.sh: test_expect_success '%(trailers:unfold) unfolds trailers' '
- test_cmp expect actual
- '
+@@ t/t6300-for-each-ref.sh: test_trailer_option '%(trailers:only) shows only "key: value" trailers' \
--test_expect_success '%(trailers:only) shows only "key: value" trailers' '
-+test_show_key_value_trailers () {
-+ option="$1"
-+ test_expect_success "%($option) shows only 'key: value' trailers" '
-+ {
-+ grep -v patch.description <trailers &&
-+ echo
-+ } >expect &&
-+ git for-each-ref --format="%($option)" refs/heads/main >actual &&
-+ test_cmp expect actual &&
-+ git for-each-ref --format="%(contents:$option)" refs/heads/main >actual &&
-+ test_cmp expect actual
-+ '
-+}
-+
-+test_show_key_value_trailers 'trailers:only'
-+test_show_key_value_trailers 'trailers:only=no,only=true'
-+test_show_key_value_trailers 'trailers:only=yes'
-+
-+test_expect_success '%(trailers:only=no) shows all trailers' '
- {
-- grep -v patch.description <trailers &&
-+ cat trailers &&
- echo
- } >expect &&
-- git for-each-ref --format="%(trailers:only)" refs/heads/main >actual &&
-+ git for-each-ref --format="%(trailers:only=no)" refs/heads/main >actual &&
- test_cmp expect actual &&
-- git for-each-ref --format="%(contents:trailers:only)" refs/heads/main >actual &&
-+ git for-each-ref --format="%(contents:trailers:only=no)" refs/heads/main >actual &&
- test_cmp expect actual
- '
+ EOF
-@@ t/t6300-for-each-ref.sh: test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '
- test_cmp actual actual
- '
-
--test_expect_success '%(trailers) rejects unknown trailers arguments' '
-- # error message cannot be checked under i18n
-- cat >expect <<-EOF &&
-- fatal: unknown %(trailers) argument: unsupported
-- EOF
-- test_must_fail git for-each-ref --format="%(trailers:unsupported)" 2>actual &&
-- test_i18ncmp expect actual &&
-- test_must_fail git for-each-ref --format="%(contents:trailers:unsupported)" 2>actual &&
-- test_i18ncmp expect actual
-+test_trailer_option() {
-+ title="$1"
-+ option="$2"
-+ expect="$3"
-+ test_expect_success "$title" '
-+ printf "$expect\n" >expect &&
-+ git for-each-ref --format="%($option)" refs/heads/main >actual &&
-+ test_cmp expect actual &&
-+ git for-each-ref --format="%(contents:$option)" refs/heads/main >actual &&
-+ test_cmp expect actual
-+ '
-+}
++test_trailer_option '%(trailers:only=no,only=true) shows only "key: value" trailers' \
++ 'trailers:only=no,only=true' <<-EOF
++ $(grep -v patch.description <trailers)
++
++ EOF
++
++test_trailer_option '%(trailers:only=yes) shows only "key: value" trailers' \
++ 'trailers:only=yes' <<-EOF
++ $(grep -v patch.description <trailers)
++
++ EOF
+
++test_trailer_option '%(trailers:only=no) shows all trailers' \
++ 'trailers:only=no' <<-EOF
++ $(cat trailers)
++
++ EOF
++
+ test_trailer_option '%(trailers:only) and %(trailers:unfold) work together' \
+ 'trailers:only,unfold' <<-EOF
+ $(grep -v patch.description <trailers | unfold)
+@@ t/t6300-for-each-ref.sh: test_trailer_option '%(trailers:unfold) and %(trailers:only) work together' \
+
+ EOF
+
+test_trailer_option '%(trailers:key=foo) shows that trailer' \
-+ 'trailers:key=Signed-off-by' 'Signed-off-by: A U Thor <author@example.com>\n'
++ 'trailers:key=Signed-off-by' <<-EOF
++ Signed-off-by: A U Thor <author@example.com>
++
++ EOF
++
+test_trailer_option '%(trailers:key=foo) is case insensitive' \
-+ 'trailers:key=SiGned-oFf-bY' 'Signed-off-by: A U Thor <author@example.com>\n'
++ 'trailers:key=SiGned-oFf-bY' <<-EOF
++ Signed-off-by: A U Thor <author@example.com>
++
++ EOF
++
+test_trailer_option '%(trailers:key=foo:) trailing colon also works' \
-+ 'trailers:key=Signed-off-by:' 'Signed-off-by: A U Thor <author@example.com>\n'
++ 'trailers:key=Signed-off-by:' <<-EOF
++ Signed-off-by: A U Thor <author@example.com>
++
++ EOF
++
+test_trailer_option '%(trailers:key=foo) multiple keys' \
-+ 'trailers:key=Reviewed-by:,key=Signed-off-by' 'Reviewed-by: A U Thor <author@example.com>\nSigned-off-by: A U Thor <author@example.com>\n'
++ 'trailers:key=Reviewed-by:,key=Signed-off-by' <<-EOF
++ Reviewed-by: A U Thor <author@example.com>
++ Signed-off-by: A U Thor <author@example.com>
++
++ EOF
++
+test_trailer_option '%(trailers:key=nonexistent) becomes empty' \
-+ 'trailers:key=Shined-off-by:' ''
-+
-+test_expect_success '%(trailers:key=foo) handles multiple lines even if folded' '
-+ {
-+ grep -v patch.description <trailers | grep -v Signed-off-by | grep -v Reviewed-by &&
-+ echo
-+ } >expect &&
-+ git for-each-ref --format="%(trailers:key=Acked-by)" refs/heads/main >actual &&
-+ test_cmp expect actual &&
-+ git for-each-ref --format="%(contents:trailers:key=Acked-by)" refs/heads/main >actual &&
-+ test_cmp expect actual
-+'
-+
-+test_expect_success '%(trailers:key=foo,unfold) properly unfolds' '
-+ {
-+ unfold <trailers | grep Signed-off-by &&
-+ echo
-+ } >expect &&
-+ git for-each-ref --format="%(trailers:key=Signed-Off-by,unfold)" refs/heads/main >actual &&
-+ test_cmp expect actual &&
-+ git for-each-ref --format="%(contents:trailers:key=Signed-Off-by,unfold)" refs/heads/main >actual &&
-+ test_cmp expect actual
- '
-
-+test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
-+ {
-+ echo "Signed-off-by: A U Thor <author@example.com>" &&
-+ grep patch.description <trailers &&
-+ echo
-+ } >expect &&
-+ git for-each-ref --format="%(trailers:key=Signed-off-by,only=no)" refs/heads/main >actual &&
-+ test_cmp expect actual &&
-+ git for-each-ref --format="%(contents:trailers:key=Signed-off-by,only=no)" refs/heads/main >actual &&
-+ test_cmp expect actual
-+'
++ 'trailers:key=Shined-off-by:' <<-EOF
++
++ EOF
++
++test_trailer_option '%(trailers:key=foo) handles multiple lines even if folded' \
++ 'trailers:key=Acked-by' <<-EOF
++ $(grep -v patch.description <trailers | grep -v Signed-off-by | grep -v Reviewed-by)
++
++ EOF
++
++test_trailer_option '%(trailers:key=foo,unfold) properly unfolds' \
++ 'trailers:key=Signed-Off-by,unfold' <<-EOF
++ $(unfold <trailers | grep Signed-off-by)
++
++ EOF
++
++test_trailer_option '%(trailers:key=foo,only=no) also includes nontrailer lines' \
++ 'trailers:key=Signed-off-by,only=no' <<-EOF
++ Signed-off-by: A U Thor <author@example.com>
++ $(grep patch.description <trailers)
++
++ EOF
+
+test_trailer_option '%(trailers:key=foo,valueonly) shows only value' \
-+ 'trailers:key=Signed-off-by,valueonly' 'A U Thor <author@example.com>\n'
++ 'trailers:key=Signed-off-by,valueonly' <<-EOF
++ A U Thor <author@example.com>
++
++ EOF
++
+test_trailer_option '%(trailers:separator) changes separator' \
-+ 'trailers:separator=%x2C,key=Reviewed-by,key=Signed-off-by:' 'Reviewed-by: A U Thor <author@example.com>,Signed-off-by: A U Thor <author@example.com>'
++ 'trailers:separator=%x2C,key=Reviewed-by,key=Signed-off-by:' <<-EOF
++ Reviewed-by: A U Thor <author@example.com>,Signed-off-by: A U Thor <author@example.com>
++ EOF
++
+test_trailer_option '%(trailers:key_value_separator) changes key-value separator' \
-+ 'trailers:key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' 'Reviewed-by,A U Thor <author@example.com>\nSigned-off-by,A U Thor <author@example.com>\n'
++ 'trailers:key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' <<-EOF
++ Reviewed-by,A U Thor <author@example.com>
++ Signed-off-by,A U Thor <author@example.com>
++
++ EOF
++
+test_trailer_option '%(trailers:separator,key_value_separator) changes both separators' \
-+ 'trailers:separator=%x2C,key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' 'Reviewed-by,A U Thor <author@example.com>,Signed-off-by,A U Thor <author@example.com>'
-+
-+test_failing_trailer_option () {
-+ title="$1"
-+ option="$2"
-+ error="$3"
-+ test_expect_success "$title" '
-+ # error message cannot be checked under i18n
-+ echo $error >expect &&
-+ test_must_fail git for-each-ref --format="%($option)" refs/heads/main 2>actual &&
-+ test_i18ncmp expect actual &&
-+ test_must_fail git for-each-ref --format="%(contents:$option)" refs/heads/main 2>actual &&
-+ test_i18ncmp expect actual
-+ '
-+}
++ 'trailers:separator=%x2C,key_value_separator=%x2C,key=Reviewed-by,key=Signed-off-by:' <<-EOF
++ Reviewed-by,A U Thor <author@example.com>,Signed-off-by,A U Thor <author@example.com>
++ EOF
+
+ test_failing_trailer_option () {
+ title=$1 option=$2
+ cat >expect
+@@ t/t6300-for-each-ref.sh: test_failing_trailer_option '%(trailers) rejects unknown trailers arguments' \
+ fatal: unknown %(trailers) argument: unsupported
+ EOF
+
+test_failing_trailer_option '%(trailers:key) without value is error' \
-+ 'trailers:key' 'fatal: expected %(trailers:key=<value>)'
-+test_failing_trailer_option '%(trailers) rejects unknown trailers arguments' \
-+ 'trailers:unsupported' 'fatal: unknown %(trailers) argument: unsupported'
++ 'trailers:key' <<-\EOF
++ fatal: expected %(trailers:key=<value>)
++ EOF
+
test_expect_success 'if arguments, %(contents:trailers) shows error if colon is missing' '
cat >expect <<-EOF &&
--
gitgitgadget
next prev parent reply other threads:[~2021-02-13 1:53 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-05 19:48 [PATCH 0/2] Unify trailers formatting logic for pretty.c and ref-filter.c Hariom Verma via GitGitGadget
2020-09-05 19:48 ` [PATCH 1/2] pretty.c: refactor trailer logic to `format_set_trailers_options()` Hariom Verma via GitGitGadget
2020-09-05 21:59 ` René Scharfe
2020-09-05 19:48 ` [PATCH 2/2] ref-filter: using pretty.c logic for trailers Hariom Verma via GitGitGadget
2021-01-29 21:09 ` [PATCH v2 0/3] Unify trailers formatting logic for pretty.c and ref-filter.c Hariom Verma via GitGitGadget
2021-01-29 21:09 ` [PATCH v2 1/3] pretty.c: refactor trailer logic to `format_set_trailers_options()` Hariom Verma via GitGitGadget
2021-01-29 23:49 ` Junio C Hamano
2021-01-29 21:09 ` [PATCH v2 2/3] pretty.c: capture invalid trailer argument Hariom Verma via GitGitGadget
2021-01-29 22:28 ` Christian Couder
2021-01-30 19:16 ` Hariom verma
2021-01-30 0:01 ` Junio C Hamano
2021-01-30 19:00 ` Hariom verma
2021-01-30 0:07 ` Junio C Hamano
2021-01-30 19:06 ` Hariom verma
2021-01-29 21:09 ` [PATCH v2 3/3] ref-filter: use pretty.c logic for trailers Hariom Verma via GitGitGadget
2021-01-30 20:45 ` Ævar Arnfjörð Bjarmason
2021-02-04 18:46 ` Hariom verma
2021-02-04 20:53 ` Ævar Arnfjörð Bjarmason
2021-01-30 1:17 ` [PATCH v2 0/3] Unify trailers formatting logic for pretty.c and ref-filter.c Junio C Hamano
2021-01-30 1:28 ` Junio C Hamano
2021-01-30 19:15 ` Hariom verma
2021-01-30 20:20 ` Junio C Hamano
2021-02-06 9:15 ` [PATCH v3 " Hariom Verma via GitGitGadget
2021-02-06 9:15 ` [PATCH v3 1/3] pretty.c: refactor trailer logic to `format_set_trailers_options()` Hariom Verma via GitGitGadget
2021-02-06 9:15 ` [PATCH v3 2/3] pretty.c: capture invalid trailer argument Hariom Verma via GitGitGadget
2021-02-06 9:15 ` [PATCH v3 3/3] ref-filter: use pretty.c logic for trailers Hariom Verma via GitGitGadget
2021-02-07 5:45 ` Junio C Hamano
2021-02-07 12:06 ` Hariom verma
2021-02-07 18:19 ` Junio C Hamano
2021-02-07 19:38 ` Hariom verma
2021-02-07 20:09 ` Junio C Hamano
2021-02-08 17:07 ` Hariom verma
2021-02-08 18:29 ` Junio C Hamano
[not found] ` <xmqqlfby5o9h.fsf@gitster.c.googlers.com>
2021-02-08 23:43 ` brian m. carlson
2021-02-09 3:04 ` brian m. carlson
2021-02-09 20:54 ` Junio C Hamano
2021-02-07 3:33 ` [PATCH v3 0/3] Unify trailers formatting logic for pretty.c and ref-filter.c Junio C Hamano
2021-02-07 5:06 ` Junio C Hamano
2021-02-13 1:52 ` Hariom Verma via GitGitGadget [this message]
2021-02-13 1:52 ` [PATCH v4 1/4] t6300: use function to test trailer options Hariom Verma via GitGitGadget
2021-02-13 1:52 ` [PATCH v4 2/4] pretty.c: refactor trailer logic to `format_set_trailers_options()` Hariom Verma via GitGitGadget
2021-02-13 1:52 ` [PATCH v4 3/4] pretty.c: capture invalid trailer argument Hariom Verma via GitGitGadget
2021-02-13 1:52 ` [PATCH v4 4/4] ref-filter: use pretty.c logic for trailers Hariom Verma via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.726.v4.git.1613181163.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=avarab@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hariom18599@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.