Subject: [PATCH v3 0/4] [GSoC] Fix trailers atom bug and improved tests

From: "Hariom Verma via GitGitGadget" <[email-masked]>
Date: Fri, 21 Aug 2020 21:06:12 +0000

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/[email-masked]/
   )
 * 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-masked]>
          Mentored-by: Christian Couder <[email-masked]>
          Mentored-by: Heba Waly <[email-masked]>
          Signed-off-by: Hariom Verma <[email-masked]>
      
       ## 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