| Message ID | 465dc51cdcba28d235241021bc52369f6082d329.1706664145.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enrich Trailer API | expand |
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Linus Arver <linusa@google.com> > > We don't move the "arg_item" struct to interpret-trailers.c, because it > is now a struct that contains information about trailers that could be > added into the input text's own trailers. This is a generic concept that > extends beyond trailers defined as CLI arguments (it applies to trailers > defined in configuration as well). We will rename "arg_item" to > "trailer_template" in a follow-up patch to keep the diff here small. > > Signed-off-by: Linus Arver <linusa@google.com> > --- > builtin/interpret-trailers.c | 88 ++++++++++++++++++++++-------------- > trailer.c | 62 ++++++++++++++++++------- > trailer.h | 12 +++++ > 3 files changed, 112 insertions(+), 50 deletions(-) > > diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c > index 9f0ba39b317..9a902012912 100644 > --- a/builtin/interpret-trailers.c > +++ b/builtin/interpret-trailers.c > @@ -45,23 +45,17 @@ static int option_parse_if_missing(const struct option *opt, > return trailer_set_if_missing(opt->value, arg); > } > > -static void free_new_trailers(struct list_head *trailers) > -{ > - struct list_head *pos, *tmp; > - struct new_trailer_item *item; > - > - list_for_each_safe(pos, tmp, trailers) { > - item = list_entry(pos, struct new_trailer_item, list); > - list_del(pos); > - free(item); > - } > -} > +static char *cl_separators; It somehow feels ugly and goes backward to depend on a new global, especially when you are aiming to libify more things. If we are doing something like ... > static int option_parse_trailer(const struct option *opt, > const char *arg, int unset) > { > struct list_head *trailers = opt->value; > - struct new_trailer_item *item; > + struct strbuf tok = STRBUF_INIT; > + struct strbuf val = STRBUF_INIT; > + const struct trailer_conf *conf; > + struct trailer_conf *conf_current = new_trailer_conf(); > + ssize_t separator_pos; > > if (unset) { > free_new_trailers(trailers); > @@ -71,12 +65,31 @@ static int option_parse_trailer(const struct option *opt, > if (!arg) > return -1; > > - item = xmalloc(sizeof(*item)); > - item->text = arg; > - item->where = where; > - item->if_exists = if_exists; > - item->if_missing = if_missing; > - list_add_tail(&item->list, trailers); > + separator_pos = find_separator(arg, cl_separators); > + if (separator_pos) { > + parse_trailer(arg, separator_pos, &tok, &val, &conf); > + duplicate_trailer_conf(conf_current, conf); > + > + /* > + * Override conf_current with settings specified via CLI flags. > + */ ... this, which apparently needs to know that we are dealing with input from CLI, shouldn't we be able to do the "CLI allows '=' as well as whatever is the default" here, instead of mucking with the global variable in the caller? Even if you plan to later make this function callable from outside the context of parse_options() callback, and if you do not want "CLI allows '=' as well" for such new callers, we should be able to have a shared helper function that is the bulk of this function but takes additional parameter to tweak its behaviour slightly depending on the needs of the callers? > + trailer_conf_set(where, if_exists, if_missing, conf_current); I am getting an impression that the use of and the introduction of the new helper, mixed with movement of the responsibility, makes reviewing the change unnecessarily more cumbersome. An equivalent series with a few more steps, each of them focusing on a small change (like, "updating the conf_current members is done with assignments that appear as a pattern very often---introduce a helper to reduce boilerplate") would have been more enticing to reviewers. > + trailer_add_arg_item(strbuf_detach(&tok, NULL), > + strbuf_detach(&val, NULL), > + conf_current, > + trailers); > + } else { > + struct strbuf sb = STRBUF_INIT; > + strbuf_addstr(&sb, arg); > + strbuf_trim(&sb); > + error(_("empty trailer token in trailer '%.*s'"), > + (int) sb.len, sb.buf); > + strbuf_release(&sb); > + } > + > + free(conf_current); > + > return 0; > } > > @@ -135,7 +148,7 @@ static void read_input_file(struct strbuf *sb, const char *file) > } > > static void interpret_trailers(const struct process_trailer_options *opts, > - struct list_head *new_trailer_head, > + struct list_head *arg_trailers, > const char *file) > { > LIST_HEAD(head); > @@ -144,8 +157,6 @@ static void interpret_trailers(const struct process_trailer_options *opts, > struct trailer_block *trailer_block; > FILE *outfile = stdout; > > - trailer_config_init(); > - > read_input_file(&sb, file); > > if (opts->in_place) > @@ -162,12 +173,7 @@ static void interpret_trailers(const struct process_trailer_options *opts, > > > if (!opts->only_input) { > - LIST_HEAD(config_head); > - LIST_HEAD(arg_head); > - parse_trailers_from_config(&config_head); > - parse_trailers_from_command_line_args(&arg_head, new_trailer_head); > - list_splice(&config_head, &arg_head); > - process_trailers_lists(&head, &arg_head); > + process_trailers_lists(&head, arg_trailers); > } So, the bulk of the parsing is no longer responsibility of this function. Instead, the caller (e.g., cmd_interpret_trailers()) is expected to do that before they call us. > ... > git_config(git_default_config, NULL); > + trailer_config_init(); > + > + if (!opts.only_input) { > + parse_trailers_from_config(&configured_trailers); > + } By the way, until we add more statements in this block, lose the unnecessary {} around it. > + /* > + * In command-line arguments, '=' is accepted (in addition to the > + * separators that are defined). > + */ > + cl_separators = xstrfmt("=%s", trailer_default_separators()); > > argc = parse_options(argc, argv, prefix, options, > git_interpret_trailers_usage, 0); > > - if (opts.only_input && !list_empty(&trailers)) > + free(cl_separators); > + > + if (opts.only_input && !list_empty(&arg_trailers)) > usage_msg_opt( > _("--trailer with --only-input does not make sense"), > git_interpret_trailers_usage, > options); > > + list_splice(&configured_trailers, &arg_trailers); This corresponds to what the old code did in interpret_trailers(), OK. The move of the responsibility may make sense. > if (argc) { > int i; > for (i = 0; i < argc; i++) > - interpret_trailers(&opts, &trailers, argv[i]); > + interpret_trailers(&opts, &arg_trailers, argv[i]); > } else { > if (opts.in_place) > die(_("no input file given for in-place editing")); > - interpret_trailers(&opts, &trailers, NULL); > + interpret_trailers(&opts, &arg_trailers, NULL); > } > > - free_new_trailers(&trailers); > + free_new_trailers(&arg_trailers); > > return 0; > } > diff --git a/trailer.c b/trailer.c > index c16f552b463..19637ff295d 100644 > --- a/trailer.c > +++ b/trailer.c > @@ -66,6 +66,11 @@ static LIST_HEAD(conf_head); > > static char *separators = ":"; > > +const char *trailer_default_separators(void) > +{ > + return separators; > +} > + > static int configured; > > #define TRAILER_ARG_STRING "$ARG" > @@ -424,6 +429,25 @@ int trailer_set_if_missing(enum trailer_if_missing *item, const char *value) > return 0; > } > > +void trailer_conf_set(enum trailer_where where, > + enum trailer_if_exists if_exists, > + enum trailer_if_missing if_missing, > + struct trailer_conf *conf) > +{ > + if (where != WHERE_DEFAULT) > + conf->where = where; > + if (if_exists != EXISTS_DEFAULT) > + conf->if_exists = if_exists; > + if (if_missing != MISSING_DEFAULT) > + conf->if_missing = if_missing; > +} So, this is such a helper function. It is curious that it deliberately lacks the ability to reset what has already been configured back to the default. For example, we earlier saw this original code ... > - item = xmalloc(sizeof(*item)); > - item->text = arg; > - item->where = where; > - item->if_exists = if_exists; > - item->if_missing = if_missing; ... gets replaced with this call. > + struct trailer_conf *conf_current = new_trailer_conf(); > ... > + trailer_conf_set(where, if_exists, if_missing, conf_current); For this conversion to be correct, we require that the value of the *_DEFAULT macro to be 0 and/or these three values getting assigned are not *_DEFAULT values. Maybe that may happen to be the case in today's code, but such an unwritten assumption makes me feel nervous. I am not sure if it is worth the confusion factor to make a function whose name is $anything_set() to be making a conditional assignment. If such a conditional assignment *also* happens often and deserves to have its own helper, it is fine to have such a helper for conditional assignment, but calling it $anything_set() is probably a mistake. Other than that, the main thrust of this step seems sensible. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > > [...] > > Other than that, the main thrust of this step seems sensible. > > Thanks. Quick update: I've broken down patches 3 and 4 into smaller pieces, and am now ready to break this one down, too. Hopefully sometime over the weekend I'll have some answers ready for the many good questions you've raised. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c >> index 9f0ba39b317..9a902012912 100644 >> --- a/builtin/interpret-trailers.c >> +++ b/builtin/interpret-trailers.c >> @@ -45,23 +45,17 @@ static int option_parse_if_missing(const struct option *opt, >> return trailer_set_if_missing(opt->value, arg); >> } >> >> -static void free_new_trailers(struct list_head *trailers) >> -{ >> - struct list_head *pos, *tmp; >> - struct new_trailer_item *item; >> - >> - list_for_each_safe(pos, tmp, trailers) { >> - item = list_entry(pos, struct new_trailer_item, list); >> - list_del(pos); >> - free(item); >> - } >> -} >> +static char *cl_separators; > > It somehow feels ugly and goes backward to depend on a new global, > especially when you are aiming to libify more things. Removed in the next reroll (we can just use a regular stack variable, not a global). > Even if you plan to later make this function callable from outside > the context of parse_options() callback, and if you do not want "CLI > allows '=' as well" for such new callers, we should be able to have > a shared helper function that is the bulk of this function but takes > additional parameter to tweak its behaviour slightly depending on > the needs of the callers? I'll refrain from exploring this possibility in this series (and just make do with the non-global variable as described above) because of the additional pending changes around trailer configuration handling in the larger series. FWIW even in the larger series I don't think there's a need to have such a helper function. >> + trailer_conf_set(where, if_exists, if_missing, conf_current); > > I am getting an impression that the use of and the introduction of > the new helper, mixed with movement of the responsibility, makes > reviewing the change unnecessarily more cumbersome. An equivalent > series with a few more steps, each of them focusing on a small > change (like, "updating the conf_current members is done with > assignments that appear as a pattern very often---introduce a helper > to reduce boilerplate") would have been more enticing to reviewers. I've broken this patch down into smaller steps (along with the other notable patches in this series). >> + trailer_add_arg_item(strbuf_detach(&tok, NULL), >> + strbuf_detach(&val, NULL), >> + conf_current, >> + trailers); >> + } else { >> + struct strbuf sb = STRBUF_INIT; >> + strbuf_addstr(&sb, arg); >> + strbuf_trim(&sb); >> + error(_("empty trailer token in trailer '%.*s'"), >> + (int) sb.len, sb.buf); >> + strbuf_release(&sb); >> + } >> + >> + free(conf_current); >> + >> return 0; >> } >> >> @@ -135,7 +148,7 @@ static void read_input_file(struct strbuf *sb, const char *file) >> } >> >> static void interpret_trailers(const struct process_trailer_options *opts, >> - struct list_head *new_trailer_head, >> + struct list_head *arg_trailers, >> const char *file) >> { >> LIST_HEAD(head); >> @@ -144,8 +157,6 @@ static void interpret_trailers(const struct process_trailer_options *opts, >> struct trailer_block *trailer_block; >> FILE *outfile = stdout; >> >> - trailer_config_init(); >> - >> read_input_file(&sb, file); >> >> if (opts->in_place) >> @@ -162,12 +173,7 @@ static void interpret_trailers(const struct process_trailer_options *opts, >> >> >> if (!opts->only_input) { >> - LIST_HEAD(config_head); >> - LIST_HEAD(arg_head); >> - parse_trailers_from_config(&config_head); >> - parse_trailers_from_command_line_args(&arg_head, new_trailer_head); >> - list_splice(&config_head, &arg_head); >> - process_trailers_lists(&head, &arg_head); >> + process_trailers_lists(&head, arg_trailers); >> } > > So, the bulk of the parsing is no longer responsibility of this > function. Instead, the caller (e.g., cmd_interpret_trailers()) is > expected to do that before they call us. Yup. This movement of "parsing responsibility" logic should be easy to see in the next reroll (as it will be broken out separately). >> ... >> git_config(git_default_config, NULL); >> + trailer_config_init(); >> + >> + if (!opts.only_input) { >> + parse_trailers_from_config(&configured_trailers); >> + } > > By the way, until we add more statements in this block, lose the > unnecessary {} around it. Oops. Done. >> @@ -424,6 +429,25 @@ int trailer_set_if_missing(enum trailer_if_missing *item, const char *value) >> return 0; >> } >> >> +void trailer_conf_set(enum trailer_where where, >> + enum trailer_if_exists if_exists, >> + enum trailer_if_missing if_missing, >> + struct trailer_conf *conf) >> +{ >> + if (where != WHERE_DEFAULT) >> + conf->where = where; >> + if (if_exists != EXISTS_DEFAULT) >> + conf->if_exists = if_exists; >> + if (if_missing != MISSING_DEFAULT) >> + conf->if_missing = if_missing; >> +} > > So, this is such a helper function. It is curious that it > deliberately lacks the ability to reset what has already been > configured back to the default. > > For example, we earlier saw this original code ... > >> - item = xmalloc(sizeof(*item)); >> - item->text = arg; >> - item->where = where; >> - item->if_exists = if_exists; >> - item->if_missing = if_missing; > > ... gets replaced with this call. > >> + struct trailer_conf *conf_current = new_trailer_conf(); >> ... >> + trailer_conf_set(where, if_exists, if_missing, conf_current); > > For this conversion to be correct, we require that the value of the > *_DEFAULT macro to be 0 and/or these three values getting assigned > are not *_DEFAULT values. Maybe that may happen to be the case in > today's code, but such an unwritten assumption makes me feel nervous. > > I am not sure if it is worth the confusion factor to make a function > whose name is $anything_set() to be making a conditional assignment. > If such a conditional assignment *also* happens often and deserves > to have its own helper, it is fine to have such a helper for > conditional assignment, but calling it $anything_set() is probably a > mistake. Agreed. I've split this out into separate helpers that do not have conditionals built into them. > Other than that, the main thrust of this step seems sensible. Ack. Overall, for the next reroll I have the commits broken down and it is mostly the same. I just need to wait for CI to pass and update the cover letter, before sending it (as v4). Should be ready later tonight. Thanks.
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 9f0ba39b317..9a902012912 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -45,23 +45,17 @@ static int option_parse_if_missing(const struct option *opt, return trailer_set_if_missing(opt->value, arg); } -static void free_new_trailers(struct list_head *trailers) -{ - struct list_head *pos, *tmp; - struct new_trailer_item *item; - - list_for_each_safe(pos, tmp, trailers) { - item = list_entry(pos, struct new_trailer_item, list); - list_del(pos); - free(item); - } -} +static char *cl_separators; static int option_parse_trailer(const struct option *opt, const char *arg, int unset) { struct list_head *trailers = opt->value; - struct new_trailer_item *item; + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + const struct trailer_conf *conf; + struct trailer_conf *conf_current = new_trailer_conf(); + ssize_t separator_pos; if (unset) { free_new_trailers(trailers); @@ -71,12 +65,31 @@ static int option_parse_trailer(const struct option *opt, if (!arg) return -1; - item = xmalloc(sizeof(*item)); - item->text = arg; - item->where = where; - item->if_exists = if_exists; - item->if_missing = if_missing; - list_add_tail(&item->list, trailers); + separator_pos = find_separator(arg, cl_separators); + if (separator_pos) { + parse_trailer(arg, separator_pos, &tok, &val, &conf); + duplicate_trailer_conf(conf_current, conf); + + /* + * Override conf_current with settings specified via CLI flags. + */ + trailer_conf_set(where, if_exists, if_missing, conf_current); + + trailer_add_arg_item(strbuf_detach(&tok, NULL), + strbuf_detach(&val, NULL), + conf_current, + trailers); + } else { + struct strbuf sb = STRBUF_INIT; + strbuf_addstr(&sb, arg); + strbuf_trim(&sb); + error(_("empty trailer token in trailer '%.*s'"), + (int) sb.len, sb.buf); + strbuf_release(&sb); + } + + free(conf_current); + return 0; } @@ -135,7 +148,7 @@ static void read_input_file(struct strbuf *sb, const char *file) } static void interpret_trailers(const struct process_trailer_options *opts, - struct list_head *new_trailer_head, + struct list_head *arg_trailers, const char *file) { LIST_HEAD(head); @@ -144,8 +157,6 @@ static void interpret_trailers(const struct process_trailer_options *opts, struct trailer_block *trailer_block; FILE *outfile = stdout; - trailer_config_init(); - read_input_file(&sb, file); if (opts->in_place) @@ -162,12 +173,7 @@ static void interpret_trailers(const struct process_trailer_options *opts, if (!opts->only_input) { - LIST_HEAD(config_head); - LIST_HEAD(arg_head); - parse_trailers_from_config(&config_head); - parse_trailers_from_command_line_args(&arg_head, new_trailer_head); - list_splice(&config_head, &arg_head); - process_trailers_lists(&head, &arg_head); + process_trailers_lists(&head, arg_trailers); } /* Print trailer block. */ @@ -193,7 +199,8 @@ static void interpret_trailers(const struct process_trailer_options *opts, int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) { struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; - LIST_HEAD(trailers); + LIST_HEAD(configured_trailers); + LIST_HEAD(arg_trailers); struct option options[] = { OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")), @@ -212,33 +219,48 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) OPT_CALLBACK_F(0, "parse", &opts, NULL, N_("alias for --only-trailers --only-input --unfold"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse), OPT_BOOL(0, "no-divider", &opts.no_divider, N_("do not treat \"---\" as the end of input")), - OPT_CALLBACK(0, "trailer", &trailers, N_("trailer"), + OPT_CALLBACK(0, "trailer", &arg_trailers, N_("trailer"), N_("trailer(s) to add"), option_parse_trailer), OPT_END() }; git_config(git_default_config, NULL); + trailer_config_init(); + + if (!opts.only_input) { + parse_trailers_from_config(&configured_trailers); + } + + /* + * In command-line arguments, '=' is accepted (in addition to the + * separators that are defined). + */ + cl_separators = xstrfmt("=%s", trailer_default_separators()); argc = parse_options(argc, argv, prefix, options, git_interpret_trailers_usage, 0); - if (opts.only_input && !list_empty(&trailers)) + free(cl_separators); + + if (opts.only_input && !list_empty(&arg_trailers)) usage_msg_opt( _("--trailer with --only-input does not make sense"), git_interpret_trailers_usage, options); + list_splice(&configured_trailers, &arg_trailers); + if (argc) { int i; for (i = 0; i < argc; i++) - interpret_trailers(&opts, &trailers, argv[i]); + interpret_trailers(&opts, &arg_trailers, argv[i]); } else { if (opts.in_place) die(_("no input file given for in-place editing")); - interpret_trailers(&opts, &trailers, NULL); + interpret_trailers(&opts, &arg_trailers, NULL); } - free_new_trailers(&trailers); + free_new_trailers(&arg_trailers); return 0; } diff --git a/trailer.c b/trailer.c index c16f552b463..19637ff295d 100644 --- a/trailer.c +++ b/trailer.c @@ -66,6 +66,11 @@ static LIST_HEAD(conf_head); static char *separators = ":"; +const char *trailer_default_separators(void) +{ + return separators; +} + static int configured; #define TRAILER_ARG_STRING "$ARG" @@ -424,6 +429,25 @@ int trailer_set_if_missing(enum trailer_if_missing *item, const char *value) return 0; } +void trailer_conf_set(enum trailer_where where, + enum trailer_if_exists if_exists, + enum trailer_if_missing if_missing, + struct trailer_conf *conf) +{ + if (where != WHERE_DEFAULT) + conf->where = where; + if (if_exists != EXISTS_DEFAULT) + conf->if_exists = if_exists; + if (if_missing != MISSING_DEFAULT) + conf->if_missing = if_missing; +} + +struct trailer_conf *new_trailer_conf(void) +{ + struct trailer_conf *new = xcalloc(1, sizeof(*new)); + return new; +} + void duplicate_trailer_conf(struct trailer_conf *dst, const struct trailer_conf *src) { @@ -642,6 +666,9 @@ ssize_t find_separator(const char *line, const char *separators) /* * Obtain the token, value, and conf from the given trailer. * + * The conf needs special handling. We first read hardcoded defaults, and + * override them if we find a matching trailer configuration. + * * separator_pos must not be 0, since the token cannot be an empty string. * * If separator_pos is -1, interpret the whole trailer as a token. @@ -691,22 +718,13 @@ static struct trailer_item *add_trailer_item(struct list_head *head, char *tok, return new_item; } -static void trailer_add_arg_item(struct list_head *arg_head, char *tok, char *val, - const struct trailer_conf *conf, - const struct new_trailer_item *new_trailer_item) +void trailer_add_arg_item(char *tok, char *val, const struct trailer_conf *conf, + struct list_head *arg_head) { struct arg_item *new_item = xcalloc(1, sizeof(*new_item)); new_item->token = tok; new_item->value = val; duplicate_trailer_conf(&new_item->conf, conf); - if (new_trailer_item) { - if (new_trailer_item->where != WHERE_DEFAULT) - new_item->conf.where = new_trailer_item->where; - if (new_trailer_item->if_exists != EXISTS_DEFAULT) - new_item->conf.if_exists = new_trailer_item->if_exists; - if (new_trailer_item->if_missing != MISSING_DEFAULT) - new_item->conf.if_missing = new_trailer_item->if_missing; - } list_add_tail(&new_item->list, arg_head); } @@ -719,10 +737,10 @@ void parse_trailers_from_config(struct list_head *config_head) list_for_each(pos, &conf_head) { item = list_entry(pos, struct arg_item, list); if (item->conf.command) - trailer_add_arg_item(config_head, - xstrdup(token_from_item(item, NULL)), + trailer_add_arg_item(xstrdup(token_from_item(item, NULL)), xstrdup(""), - &item->conf, NULL); + &item->conf, + config_head); } } @@ -755,10 +773,10 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head, strbuf_release(&sb); } else { parse_trailer(tr->text, separator_pos, &tok, &val, &conf); - trailer_add_arg_item(arg_head, - strbuf_detach(&tok, NULL), + trailer_add_arg_item(strbuf_detach(&tok, NULL), strbuf_detach(&val, NULL), - conf, tr); + conf, + arg_head); } } @@ -1148,6 +1166,16 @@ void free_trailers(struct list_head *trailers) } } +void free_new_trailers(struct list_head *trailers) +{ + struct list_head *pos, *p; + + list_for_each_safe(pos, p, trailers) { + list_del(pos); + free_arg_item(list_entry(pos, struct arg_item, list)); + } +} + size_t trailer_block_start(struct trailer_block *trailer_block) { return trailer_block->start; diff --git a/trailer.h b/trailer.h index d724263e4f6..8fcf1969a3c 100644 --- a/trailer.h +++ b/trailer.h @@ -46,9 +46,20 @@ struct new_trailer_item { enum trailer_if_missing if_missing; }; +void trailer_conf_set(enum trailer_where where, + enum trailer_if_exists if_exists, + enum trailer_if_missing if_missing, + struct trailer_conf *conf); + +struct trailer_conf *new_trailer_conf(void); void duplicate_trailer_conf(struct trailer_conf *dst, const struct trailer_conf *src); +const char *trailer_default_separators(void); + +void trailer_add_arg_item(char *tok, char *val, const struct trailer_conf *conf, + struct list_head *arg_head); + struct process_trailer_options { int in_place; int trim_empty; @@ -95,6 +106,7 @@ void format_trailers(const struct process_trailer_options *opts, struct list_head *trailers, struct strbuf *out); void free_trailers(struct list_head *); +void free_new_trailers(struct list_head *); /* * Convenience function to format the trailers from the commit msg "msg" into