Currently, there exists a bug in 'contents' atom. It does not show any error if used with modifier 'trailers' and semicolon is missing before trailers arguments. This small patch series is focused on fixing that bug and also unified 'trailers' and 'contents:trailers' tests. Thus, removed duplicate code from t6300 and made tests more compact. Change log since v2: * Used simplified logic as per suggested by Eric (here https://public-inbox.org/git/CAPig+cRxCvHG70Nd00zBxYFuecu6+Z6uDP8ooN3rx9vPagoYBA@xxxxxxxxxxxxxx/ ) * Unified trailer formatting logic for pretty.c and ref-filter.c Hariom Verma (4): t6300: unify %(trailers) and %(contents:trailers) tests ref-filter: 'contents:trailers' show error if `:` is missing pretty.c: refactor trailer logic to `format_set_trailers_options()` ref-filter: using pretty.c logic for trailers Documentation/git-for-each-ref.txt | 36 ++++++-- Hariom Verma via GitGitGadget | 0 pretty.c | 83 +++++++++++------- pretty.h | 11 +++ ref-filter.c | 43 +++++----- t/t6300-for-each-ref.sh | 133 ++++++++++++++++++++++------- 6 files changed, 219 insertions(+), 87 deletions(-) create mode 100644 Hariom Verma via GitGitGadget base-commit: 675a4aaf3b226c0089108221b96559e0baae5de9 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-707%2Fharry-hov%2Ffix-trailers-atom-bug-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-707/harry-hov/fix-trailers-atom-bug-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/707 Range-diff vs v2: 1: 4816aa3cfa = 1: 383476b177 t6300: unify %(trailers) and %(contents:trailers) tests 2: 39aa46bce7 ! 2: 659b9835dc ref-filter: 'contents:trailers' show error if `:` is missing @@ Commit message ref-filter: 'contents:trailers' show error if `:` is missing The 'contents' atom does not show any error if used with 'trailers' - atom and semicolon is missing before trailers arguments. + atom and colon is missing before trailers arguments. e.g %(contents:trailersonly) works, while it shouldn't. @@ Commit message Let's fix this bug. + Acked-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Mentored-by: Heba Waly <heba.waly@xxxxxxxxx> Signed-off-by: Hariom Verma <hariom18599@xxxxxxxxx> ## ref-filter.c ## -@@ ref-filter.c: static int trailers_atom_parser(const struct ref_format *format, struct used_ato - return 0; - } - -+static int check_format_field(const char *arg, const char *field, const char **option) -+{ -+ const char *opt; -+ if (skip_prefix(arg, field, &opt)) { -+ if (*opt == '\0') { -+ *option = NULL; -+ return 1; -+ } -+ else if (*opt == ':') { -+ *option = opt + 1; -+ return 1; -+ } -+ } -+ return 0; -+} -+ - static int contents_atom_parser(const struct ref_format *format, struct used_atom *atom, - const char *arg, struct strbuf *err) - { @@ ref-filter.c: static int contents_atom_parser(const struct ref_format *format, struct used_ato atom->u.contents.option = C_SIG; else if (!strcmp(arg, "subject")) @@ ref-filter.c: static int contents_atom_parser(const struct ref_format *format, s - else if (skip_prefix(arg, "trailers", &arg)) { - skip_prefix(arg, ":", &arg); - if (trailers_atom_parser(format, atom, *arg ? arg : NULL, err)) -+ else if (check_format_field(arg, "trailers", &arg)) { ++ else if (!strcmp(arg, "trailers")) { ++ if (trailers_atom_parser(format, atom, NULL, err)) ++ return -1; ++ } else if (skip_prefix(arg, "trailers:", &arg)) { + if (trailers_atom_parser(format, atom, arg, err)) return -1; } else if (skip_prefix(arg, "lines=", &arg)) { @@ t/t6300-for-each-ref.sh: test_expect_success '%(trailers) rejects unknown traile ' +test_expect_success 'if arguments, %(contents:trailers) shows error if semicolon is missing' ' -+ # error message cannot be checked under i18n + cat >expect <<-EOF && + fatal: unrecognized %(contents) argument: trailersonly + EOF -: ---------- > 3: 712ab9aacf pretty.c: refactor trailer logic to `format_set_trailers_options()` -: ---------- > 4: d491be5d10 ref-filter: using pretty.c logic for trailers -- gitgitgadget
