Skip to content

Fix trailers atom bug and improved tests

Hariom Verma via GitGitGadget git 3 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v3 Latest4 patches 5 repliesIn review
  2. v22 patches 8 repliesSuperseded
  3. v12 patches 7 repliesSuperseded

Changes in v3 from the cover letter

* Used simplified logic as per suggested by Eric (here
https://public-inbox.org/git/CAPig+cRxCvHG70Nd00zBxYFuecu6+Z6uDP8ooN3rx9vPagoYBA@mail.gmail.com/
)
* Unified trailer formatting logic for pretty.c and ref-filter.c

Patches in v3 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/4t6300: unify %(trailers) and %(contents:trailers) testst/t6300-for-each-ref.sh+14 -420 replies
2/4ref-filter: 'contents:trailers' show error if `:` is missingref-filter.c t/t6300-for-each-ref.sh+13 -33 replies
3/4pretty.c: refactor trailer logic to `format_set_trailers_options()`Hariom Verma via GitGitGadget pretty.c pretty.h+64 -300 replies
4/4ref-filter: using pretty.c logic for trailersDocumentation/git-for-each-ref.txt ref-filter.c t/t6300-for-each-ref.sh+151 -350 replies

Reviews trailers per patch

1 of 4 patches has a Reviewed-by or Acked-by.

PatchChristian CouderHeba WalyEric Sunshine
SeriesMentored-by Mentored-by
1/4Mentored-by Mentored-by
2/4Mentored-by Mentored-by A-b
3/4Mentored-by Mentored-by
4/4Mentored-by Mentored-by

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 3 reviewers.

Read the v3 thread

Cover letter [PATCH v3 0/4] [GSoC] Fix trailers atom bug and improved tests

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@mail.gmail.com/
)
* 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 <[email protected]>

## 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
Signature · 1 line
gitgitgadget