[PATCH v13 00/11] Add interpret-trailers builtin

[PATCH v13 00/11] Add interpret-trailers builtin

[Christian Couder]

This patch series implements a new command:

        git interpret-trailers

and an infrastructure to process trailers that can be reused,
for example in "commit.c".

1) Rationale:

This command should help with RFC 822 style headers, called
"trailers", that are found at the end of commit messages.

(Note that these headers do not follow and are not intended to
follow many rules that are in RFC 822. For example they do not
follow the line breaking rules, the encoding rules and probably
many other rules.)

For a long time, these trailers have become a de facto standard
way to add helpful information into commit messages.

Until now git commit has only supported the well known
"Signed-off-by: " trailer, that is used by many projects like
the Linux kernel and Git.

It is better to keep builtin/commit.c uncontaminated by any more
hard-wired logic, like what we have for the signed-off-by line.  Any
new things can and should be doable in hooks, and this filter would
help writing these hooks.

And that is why the design goal of the filter is to make it at least
as powerful as the built-in logic we have for signed-off-by lines;
that would allow us to later eject the hard-wired logic for
signed-off-by line from the main codepath, if/when we wanted to.

Alternatively, we could build a library-ish API around this filter
code and replace the hard-wired logic for signed-off-by line with a
call into that API, if/when we wanted to, but that requires (in
addition to the "at least as powerful as the built-in logic") that
the implementation of this stand-alone filter can be cleanly made
into a reusable library, so that is a bit higher bar to cross than
"everything can be doable with hooks" alternative.

2) Current state:

Currently the usage string of this command is:

git interpret-trailers [--trim-empty] [(--trailer <token>[(=|:)<value>])...] [<file>...]

The following features are implemented:

        - the result is printed on stdout
        - the --trailer arguments are interpreted
        - messages read from <file>... or stdin are interpreted
	- the "trailer.separators" option in the config is interpreted (new)
	- the "trailer.where" option is interpreted (new)
	- the "trailer.ifexists" option is interpreted (new)
	- the "trailer.ifmissing" option is interpreted (new)
        - the "trailer.<token>.key" options are interpreted
        - the "trailer.<token>.where" options are interpreted
        - the "trailer.<token>.ifexist" options are interpreted
        - the "trailer.<token>.ifmissing" options are interpreted
        - the "trailer.<token>.command" config works
        - $ARG can be used in commands
        - messages can contain a patch
        - lines in messages starting with a comment char are ignored
        - there are 49 tests
        - there is some documentation
        - there are examples in the documentation

3) Changes since version 12, thanks to Jakub, Michael, Johan and Junio:

* "end" and "start" values for "trailer.<token>.where" have been
  implemented
* "end" has been made the default value for "where", but this default
  value can be changed using the new "trailer.where" config
  variable
* "addIfDifferentNeighbor" is now the default value for "ifexists",
  but this default can be changed using the new "trailer.ifexists"
  config variable
* the new "trailer.ifmissing" can be used to change the default value
  for "ifmissing" (which is "add")
* by default the only separator is ':', this can be changed by using
  the new "trailer.separators" config variable
* only the configured separators (or just ':' by default) are used for
  both input parsing and output printing; the only exception is that
  '=' is always accepted as separator when parsing
  "--trailer '<token><sep><value>'" command line arguments, for
  compatibility with other git commands
* 14 tests have been added

Only patches 7/11, 9/11 and 10/11 have not been changed since v12.

Christian Couder (11):
  trailer: add data structures and basic functions
  trailer: process trailers from input message and arguments
  trailer: read and process config information
  trailer: process command line trailer arguments
  trailer: parse trailers from file or stdin
  trailer: put all the processing together and print
  trailer: add interpret-trailers command
  trailer: add tests for "git interpret-trailers"
  trailer: execute command from 'trailer.<name>.command'
  trailer: add tests for commands in config file
  Documentation: add documentation for 'git interpret-trailers'

 .gitignore                               |   1 +
 Documentation/git-interpret-trailers.txt | 308 +++++++++++
 Makefile                                 |   2 +
 builtin.h                                |   1 +
 builtin/interpret-trailers.c             |  44 ++
 command-list.txt                         |   1 +
 git.c                                    |   1 +
 t/t7513-interpret-trailers.sh            | 850 +++++++++++++++++++++++++++++++
 trailer.c                                | 830 ++++++++++++++++++++++++++++++
 trailer.h                                |   6 +
 10 files changed, 2044 insertions(+)
 create mode 100644 Documentation/git-interpret-trailers.txt
 create mode 100644 builtin/interpret-trailers.c
 create mode 100755 t/t7513-interpret-trailers.sh
 create mode 100644 trailer.c
 create mode 100644 trailer.h

-- 
2.0.1.674.ga7f57b7

[PATCH v13 01/11] trailer: add data structures and basic functions

[Christian Couder]

We will use a doubly linked list to store all information
about trailers and their configuration.

This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile  |  1 +
 trailer.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 trailer.c

diff --git a/Makefile b/Makefile
index 63a210d..ef82972 100644
--- a/Makefile
+++ b/Makefile
@@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
 LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
 LIB_OBJS += transport.o
 LIB_OBJS += transport-helper.o
 LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 0000000..2adc1b7
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,64 @@
+#include "cache.h"
+/*
+ * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
+ */
+
+enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START };
+enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, EXISTS_ADD_IF_DIFFERENT,
+			EXISTS_ADD, EXISTS_REPLACE, EXISTS_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
+
+struct conf_info {
+	char *name;
+	char *key;
+	char *command;
+	enum action_where where;
+	enum action_if_exists if_exists;
+	enum action_if_missing if_missing;
+};
+
+static struct conf_info default_conf_info;
+
+struct trailer_item {
+	struct trailer_item *previous;
+	struct trailer_item *next;
+	const char *token;
+	const char *value;
+	struct conf_info conf;
+};
+
+static struct trailer_item *first_conf_item;
+
+static char *separators = ":";
+
+static int after_or_end(enum action_where where)
+{
+	return (where == WHERE_AFTER) || (where == WHERE_END);
+}
+
+/* Get the length of buf from its beginning until its last alphanumeric character */
+static size_t alnum_len(const char *buf, size_t len)
+{
+	while (len > 0 && !isalnum(buf[len - 1]))
+		len--;
+	return len;
+}
+
+static int same_token(struct trailer_item *a, struct trailer_item *b)
+{
+	size_t a_alnum_len = alnum_len(a->token, strlen(a->token));
+	size_t b_alnum_len = alnum_len(b->token, strlen(b->token));
+	size_t min_alnum_len = (a_alnum_len > b_alnum_len) ? b_alnum_len : a_alnum_len;
+
+	return !strncasecmp(a->token, b->token, min_alnum_len);
+}
+
+static int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+	return !strcasecmp(a->value, b->value);
+}
+
+static int same_trailer(struct trailer_item *a, struct trailer_item *b)
+{
+	return same_token(a, b) && same_value(a, b);
+}
-- 
2.0.1.674.ga7f57b7

[PATCH v13 02/11] trailer: process trailers from input message and arguments

[Christian Couder]

Implement the logic to process trailers from the input message
and from arguments.

At the beginning trailers from the input message are in their
own "in_tok" doubly linked list, and trailers from arguments
are in their own "arg_tok" doubly linked list.

The lists are traversed and when an "arg_tok" should be "applied",
it is removed from its list and inserted into the "in_tok" list.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 trailer.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

diff --git a/trailer.c b/trailer.c
index 2adc1b7..4940e06 100644
--- a/trailer.c
+++ b/trailer.c
@@ -62,3 +62,213 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b)
 {
 	return same_token(a, b) && same_value(a, b);
 }
+
+static void free_trailer_item(struct trailer_item *item)
+{
+	free(item->conf.name);
+	free(item->conf.key);
+	free(item->conf.command);
+	free((char *)item->token);
+	free((char *)item->value);
+	free(item);
+}
+
+static void update_last(struct trailer_item **last)
+{
+	if (*last)
+		while ((*last)->next != NULL)
+			*last = (*last)->next;
+}
+
+static void update_first(struct trailer_item **first)
+{
+	if (*first)
+		while ((*first)->previous != NULL)
+			*first = (*first)->previous;
+}
+
+static void add_arg_to_input_list(struct trailer_item *on_tok,
+				  struct trailer_item *arg_tok,
+				  struct trailer_item **first,
+				  struct trailer_item **last)
+{
+	if (after_or_end(arg_tok->conf.where)) {
+		arg_tok->next = on_tok->next;
+		on_tok->next = arg_tok;
+		arg_tok->previous = on_tok;
+		if (arg_tok->next)
+			arg_tok->next->previous = arg_tok;
+		update_last(last);
+	} else {
+		arg_tok->previous = on_tok->previous;
+		on_tok->previous = arg_tok;
+		arg_tok->next = on_tok;
+		if (arg_tok->previous)
+			arg_tok->previous->next = arg_tok;
+		update_first(first);
+	}
+}
+
+static int check_if_different(struct trailer_item *in_tok,
+			      struct trailer_item *arg_tok,
+			      int check_all)
+{
+	enum action_where where = arg_tok->conf.where;
+	do {
+		if (!in_tok)
+			return 1;
+		if (same_trailer(in_tok, arg_tok))
+			return 0;
+		/*
+		 * if we want to add a trailer after another one,
+		 * we have to check those before this one
+		 */
+		in_tok = after_or_end(where) ? in_tok->previous : in_tok->next;
+	} while (check_all);
+	return 1;
+}
+
+static void remove_from_list(struct trailer_item *item,
+			     struct trailer_item **first,
+			     struct trailer_item **last)
+{
+	struct trailer_item *next = item->next;
+	struct trailer_item *previous = item->previous;
+
+	if (next) {
+		item->next->previous = previous;
+		item->next = NULL;
+	} else if (last)
+		*last = previous;
+
+	if (previous) {
+		item->previous->next = next;
+		item->previous = NULL;
+	} else if (first)
+		*first = next;
+}
+
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+	struct trailer_item *item = *first;
+	*first = item->next;
+	if (item->next) {
+		item->next->previous = NULL;
+		item->next = NULL;
+	}
+	return item;
+}
+
+static void apply_arg_if_exists(struct trailer_item *in_tok,
+				struct trailer_item *arg_tok,
+				struct trailer_item *on_tok,
+				struct trailer_item **in_tok_first,
+				struct trailer_item **in_tok_last)
+{
+	switch (arg_tok->conf.if_exists) {
+	case EXISTS_DO_NOTHING:
+		free_trailer_item(arg_tok);
+		break;
+	case EXISTS_REPLACE:
+		add_arg_to_input_list(on_tok, arg_tok,
+				      in_tok_first, in_tok_last);
+		remove_from_list(in_tok, in_tok_first, in_tok_last);
+		free_trailer_item(in_tok);
+		break;
+	case EXISTS_ADD:
+		add_arg_to_input_list(on_tok, arg_tok,
+				      in_tok_first, in_tok_last);
+		break;
+	case EXISTS_ADD_IF_DIFFERENT:
+		if (check_if_different(in_tok, arg_tok, 1))
+			add_arg_to_input_list(on_tok, arg_tok,
+					      in_tok_first, in_tok_last);
+		else
+			free_trailer_item(arg_tok);
+		break;
+	case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
+		if (check_if_different(on_tok, arg_tok, 0))
+			add_arg_to_input_list(on_tok, arg_tok,
+					      in_tok_first, in_tok_last);
+		else
+			free_trailer_item(arg_tok);
+		break;
+	}
+}
+
+static void apply_arg_if_missing(struct trailer_item **in_tok_first,
+				 struct trailer_item **in_tok_last,
+				 struct trailer_item *arg_tok)
+{
+	struct trailer_item **in_tok;
+	enum action_where where;
+
+	switch (arg_tok->conf.if_missing) {
+	case MISSING_DO_NOTHING:
+		free_trailer_item(arg_tok);
+		break;
+	case MISSING_ADD:
+		where = arg_tok->conf.where;
+		in_tok = after_or_end(where) ? in_tok_last : in_tok_first;
+		if (*in_tok) {
+			add_arg_to_input_list(*in_tok, arg_tok,
+					      in_tok_first, in_tok_last);
+		} else {
+			*in_tok_first = arg_tok;
+			*in_tok_last = arg_tok;
+		}
+		break;
+	}
+}
+
+static int find_same_and_apply_arg(struct trailer_item **in_tok_first,
+				   struct trailer_item **in_tok_last,
+				   struct trailer_item *arg_tok)
+{
+	struct trailer_item *in_tok;
+	struct trailer_item *on_tok;
+	struct trailer_item *following_tok;
+
+	enum action_where where = arg_tok->conf.where;
+	int middle = (where == WHERE_AFTER) || (where == WHERE_BEFORE);
+	int backwards = after_or_end(where);
+	struct trailer_item *start_tok = backwards ? *in_tok_last : *in_tok_first;
+
+	for (in_tok = start_tok; in_tok; in_tok = following_tok) {
+		following_tok = backwards ? in_tok->previous : in_tok->next;
+		if (!same_token(in_tok, arg_tok))
+			continue;
+		on_tok = middle ? in_tok : start_tok;
+		apply_arg_if_exists(in_tok, arg_tok, on_tok,
+				    in_tok_first, in_tok_last);
+		return 1;
+	}
+	return 0;
+}
+
+static void process_trailers_lists(struct trailer_item **in_tok_first,
+				   struct trailer_item **in_tok_last,
+				   struct trailer_item **arg_tok_first)
+{
+	struct trailer_item *arg_tok;
+	struct trailer_item *next_arg;
+
+	if (!*arg_tok_first)
+		return;
+
+	for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+		int applied = 0;
+
+		next_arg = arg_tok->next;
+		remove_from_list(arg_tok, arg_tok_first, NULL);
+
+		applied = find_same_and_apply_arg(in_tok_first,
+						  in_tok_last,
+						  arg_tok);
+
+		if (!applied)
+			apply_arg_if_missing(in_tok_first,
+					     in_tok_last,
+					     arg_tok);
+	}
+}
-- 
2.0.1.674.ga7f57b7

[PATCH v13 03/11] trailer: read and process config information

[Christian Couder]

Read the configuration to get trailer information, and then process
it and store it in a doubly linked list.

The config information is stored in the list whose first item is
pointed to by:

static struct trailer_item *first_conf_item;

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 trailer.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 185 insertions(+)

diff --git a/trailer.c b/trailer.c
index 4940e06..2d391f3 100644
--- a/trailer.c
+++ b/trailer.c
@@ -272,3 +272,188 @@ static void process_trailers_lists(struct trailer_item **in_tok_first,
 					     arg_tok);
 	}
 }
+
+static int set_where(struct conf_info *item, const char *value)
+{
+	if (!strcasecmp("after", value))
+		item->where = WHERE_AFTER;
+	else if (!strcasecmp("before", value))
+		item->where = WHERE_BEFORE;
+	else if (!strcasecmp("end", value))
+		item->where = WHERE_END;
+	else if (!strcasecmp("start", value))
+		item->where = WHERE_START;
+	else
+		return -1;
+	return 0;
+}
+
+static int set_if_exists(struct conf_info *item, const char *value)
+{
+	if (!strcasecmp("addIfDifferent", value))
+		item->if_exists = EXISTS_ADD_IF_DIFFERENT;
+	else if (!strcasecmp("addIfDifferentNeighbor", value))
+		item->if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
+	else if (!strcasecmp("add", value))
+		item->if_exists = EXISTS_ADD;
+	else if (!strcasecmp("replace", value))
+		item->if_exists = EXISTS_REPLACE;
+	else if (!strcasecmp("doNothing", value))
+		item->if_exists = EXISTS_DO_NOTHING;
+	else
+		return -1;
+	return 0;
+}
+
+static int set_if_missing(struct conf_info *item, const char *value)
+{
+	if (!strcasecmp("doNothing", value))
+		item->if_missing = MISSING_DO_NOTHING;
+	else if (!strcasecmp("add", value))
+		item->if_missing = MISSING_ADD;
+	else
+		return -1;
+	return 0;
+}
+
+static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
+{
+	*dst = *src;
+	if (src->name)
+		dst->name = xstrdup(src->name);
+	if (src->key)
+		dst->key = xstrdup(src->key);
+	if (src->command)
+		dst->command = xstrdup(src->command);
+}
+
+static struct trailer_item *get_conf_item(const char *name)
+{
+	struct trailer_item *item;
+	struct trailer_item *previous;
+
+	/* Look up item with same name */
+	for (previous = NULL, item = first_conf_item;
+	     item;
+	     previous = item, item = item->next) {
+		if (!strcasecmp(item->conf.name, name))
+			return item;
+	}
+
+	/* Item does not already exists, create it */
+	item = xcalloc(sizeof(struct trailer_item), 1);
+	duplicate_conf(&item->conf, &default_conf_info);
+	item->conf.name = xstrdup(name);
+
+	if (!previous)
+		first_conf_item = item;
+	else {
+		previous->next = item;
+		item->previous = previous;
+	}
+
+	return item;
+}
+
+enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,
+			 TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
+
+static struct {
+	const char *name;
+	enum trailer_info_type type;
+} trailer_config_items[] = {
+	{ "key", TRAILER_KEY },
+	{ "command", TRAILER_COMMAND },
+	{ "where", TRAILER_WHERE },
+	{ "ifexists", TRAILER_IF_EXISTS },
+	{ "ifmissing", TRAILER_IF_MISSING }
+};
+
+static int git_trailer_default_config(const char *conf_key, const char *value, void *cb)
+{
+	const char *trailer_item, *variable_name;
+
+	if (!skip_prefix(conf_key, "trailer.", &trailer_item))
+		return 0;
+
+	variable_name = strrchr(trailer_item, '.');
+	if (!variable_name) {
+		if (!strcmp(trailer_item, "where")) {
+			if (set_where(&default_conf_info, value) < 0)
+				warning(_("unknown value '%s' for key '%s'"),
+					value, conf_key);
+		} else if (!strcmp(trailer_item, "ifexists")) {
+			if (set_if_exists(&default_conf_info, value) < 0)
+				warning(_("unknown value '%s' for key '%s'"),
+					value, conf_key);
+		} else if (!strcmp(trailer_item, "ifmissing")) {
+			if (set_if_missing(&default_conf_info, value) < 0)
+				warning(_("unknown value '%s' for key '%s'"),
+					value, conf_key);
+		} else if (!strcmp(trailer_item, "separators")) {
+			separators = xstrdup(value);
+		}
+	}
+	return 0;
+}
+
+static int git_trailer_config(const char *conf_key, const char *value, void *cb)
+{
+	const char *trailer_item, *variable_name;
+	struct trailer_item *item;
+	struct conf_info *conf;
+	char *name = NULL;
+	enum trailer_info_type type;
+	int i;
+
+	if (!skip_prefix(conf_key, "trailer.", &trailer_item))
+		return 0;
+
+	variable_name = strrchr(trailer_item, '.');
+	if (!variable_name)
+		return 0;
+
+	variable_name++;
+	for (i = 0; i < ARRAY_SIZE(trailer_config_items); i++) {
+		if (strcmp(trailer_config_items[i].name, variable_name))
+			continue;
+		name = xstrndup(trailer_item,  variable_name - trailer_item - 1);
+		type = trailer_config_items[i].type;
+		break;
+	}
+
+	if (!name)
+		return 0;
+
+	item = get_conf_item(name);
+	conf = &item->conf;
+	free(name);
+
+	switch (type) {
+	case TRAILER_KEY:
+		if (conf->key)
+			warning(_("more than one %s"), conf_key);
+		conf->key = xstrdup(value);
+		break;
+	case TRAILER_COMMAND:
+		if (conf->command)
+			warning(_("more than one %s"), conf_key);
+		conf->command = xstrdup(value);
+		break;
+	case TRAILER_WHERE:
+		if (set_where(conf, value))
+			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
+		break;
+	case TRAILER_IF_EXISTS:
+		if (set_if_exists(conf, value))
+			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
+		break;
+	case TRAILER_IF_MISSING:
+		if (set_if_missing(conf, value))
+			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
+		break;
+	default:
+		die("internal bug in trailer.c");
+	}
+	return 0;
+}
-- 
2.0.1.674.ga7f57b7

[PATCH v13 04/11] trailer: process command line trailer arguments

[Christian Couder]

Parse the trailer command line arguments and put
the result into an arg_tok doubly linked list.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 trailer.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)

diff --git a/trailer.c b/trailer.c
index 2d391f3..b9d3ed4 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "string-list.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -457,3 +458,127 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
 	}
 	return 0;
 }
+
+static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer)
+{
+	size_t len;
+	struct strbuf seps = STRBUF_INIT;
+	strbuf_addstr(&seps, separators);
+	strbuf_addch(&seps, '=');
+	len = strcspn(trailer, seps.buf);
+	strbuf_release(&seps);
+	if (len == 0)
+		return error(_("empty trailer token in trailer '%s'"), trailer);
+	if (len < strlen(trailer)) {
+		strbuf_add(tok, trailer, len);
+		strbuf_trim(tok);
+		strbuf_addstr(val, trailer + len + 1);
+		strbuf_trim(val);
+	} else {
+		strbuf_addstr(tok, trailer);
+		strbuf_trim(tok);
+	}
+	return 0;
+}
+
+
+static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
+{
+	*dst = *src;
+	if (src->name)
+		dst->name = xstrdup(src->name);
+	if (src->key)
+		dst->key = xstrdup(src->key);
+	if (src->command)
+		dst->command = xstrdup(src->command);
+}
+
+static const char *token_from_item(struct trailer_item *item)
+{
+	if (item->conf.key)
+		return item->conf.key;
+
+	return item->conf.name;
+}
+
+static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
+					     char *tok, char *val)
+{
+	struct trailer_item *new = xcalloc(sizeof(*new), 1);
+	new->value = val;
+
+	if (conf_item) {
+		duplicate_conf(&new->conf, &conf_item->conf);
+		new->token = xstrdup(token_from_item(conf_item));
+		free(tok);
+	} else {
+		duplicate_conf(&new->conf, &default_conf_info);
+		new->token = tok;
+	}
+
+	return new;
+}
+
+static int token_matches_item(const char *tok, struct trailer_item *item, int alnum_len)
+{
+	if (!strncasecmp(tok, item->conf.name, alnum_len))
+		return 1;
+	return item->conf.key ? !strncasecmp(tok, item->conf.key, alnum_len) : 0;
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+	struct strbuf tok = STRBUF_INIT;
+	struct strbuf val = STRBUF_INIT;
+	struct trailer_item *item;
+	int tok_alnum_len;
+
+	if (parse_trailer(&tok, &val, string))
+		return NULL;
+
+	tok_alnum_len = alnum_len(tok.buf, tok.len);
+
+	/* Lookup if the token matches something in the config */
+	for (item = first_conf_item; item; item = item->next) {
+		if (token_matches_item(tok.buf, item, tok_alnum_len)) {
+			strbuf_release(&tok);
+			return new_trailer_item(item,
+						NULL,
+						strbuf_detach(&val, NULL));
+		}
+	}
+
+	return new_trailer_item(NULL,
+				strbuf_detach(&tok, NULL),
+				strbuf_detach(&val, NULL));
+}
+
+static void add_trailer_item(struct trailer_item **first,
+			     struct trailer_item **last,
+			     struct trailer_item *new)
+{
+	if (!new)
+		return;
+	if (!*last) {
+		*first = new;
+		*last = new;
+	} else {
+		(*last)->next = new;
+		new->previous = *last;
+		*last = new;
+	}
+}
+
+static struct trailer_item *process_command_line_args(struct string_list *trailers)
+{
+	struct trailer_item *arg_tok_first = NULL;
+	struct trailer_item *arg_tok_last = NULL;
+	struct string_list_item *tr;
+
+	for_each_string_list_item(tr, trailers) {
+		struct trailer_item *new = create_trailer_item(tr->string);
+		add_trailer_item(&arg_tok_first, &arg_tok_last, new);
+	}
+
+	return arg_tok_first;
+}
-- 
2.0.1.674.ga7f57b7

[PATCH v13 05/11] trailer: parse trailers from file or stdin

[Christian Couder]

Read trailers from a file or from stdin, parse the trailers and then
put the result into a doubly linked list.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 trailer.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/trailer.c b/trailer.c
index b9d3ed4..46e2fcb 100644
--- a/trailer.c
+++ b/trailer.c
@@ -64,6 +64,14 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b)
 	return same_token(a, b) && same_value(a, b);
 }
 
+static inline int contains_only_spaces(const char *str)
+{
+	const char *s = str;
+	while (*s && isspace(*s))
+		s++;
+	return !*s;
+}
+
 static void free_trailer_item(struct trailer_item *item)
 {
 	free(item->conf.name);
@@ -582,3 +590,118 @@ static struct trailer_item *process_command_line_args(struct string_list *traile
 
 	return arg_tok_first;
 }
+
+static struct strbuf **read_input_file(const char *file)
+{
+	struct strbuf **lines;
+	struct strbuf sb = STRBUF_INIT;
+
+	if (file) {
+		if (strbuf_read_file(&sb, file, 0) < 0)
+			die_errno(_("could not read input file '%s'"), file);
+	} else {
+		if (strbuf_read(&sb, fileno(stdin), 0) < 0)
+			die_errno(_("could not read from stdin"));
+	}
+
+	lines = strbuf_split(&sb, '\n');
+
+	strbuf_release(&sb);
+
+	return lines;
+}
+
+/*
+ * Return the (0 based) index of the start of the patch or the line
+ * count if there is no patch in the message.
+ */
+static int find_patch_start(struct strbuf **lines, int count)
+{
+	int i;
+
+	/* Get the start of the patch part if any */
+	for (i = 0; i < count; i++) {
+		if (starts_with(lines[i]->buf, "---"))
+			return i;
+	}
+
+	return count;
+}
+
+/*
+ * Return the (0 based) index of the first trailer line or count if
+ * there are no trailers. Trailers are searched only in the lines from
+ * index (count - 1) down to index 0.
+ */
+static int find_trailer_start(struct strbuf **lines, int count)
+{
+	int start, only_spaces = 1;
+
+	/*
+	 * Get the start of the trailers by looking starting from the end
+	 * for a line with only spaces before lines with one separator.
+	 */
+	for (start = count - 1; start >= 0; start--) {
+		if (lines[start]->buf[0] == comment_line_char)
+			continue;
+		if (contains_only_spaces(lines[start]->buf)) {
+			if (only_spaces)
+				continue;
+			return start + 1;
+		}
+		if (strcspn(lines[start]->buf, separators) < lines[start]->len) {
+			if (only_spaces)
+				only_spaces = 0;
+			continue;
+		}
+		return count;
+	}
+
+	return only_spaces ? count : 0;
+}
+
+static int has_blank_line_before(struct strbuf **lines, int start)
+{
+	for (;start >= 0; start--) {
+		if (lines[start]->buf[0] == comment_line_char)
+			continue;
+		return contains_only_spaces(lines[start]->buf);
+	}
+	return 0;
+}
+
+static void print_lines(struct strbuf **lines, int start, int end)
+{
+	int i;
+	for (i = start; lines[i] && i < end; i++)
+		printf("%s", lines[i]->buf);
+}
+
+static int process_input_file(struct strbuf **lines,
+			      struct trailer_item **in_tok_first,
+			      struct trailer_item **in_tok_last)
+{
+	int count = 0;
+	int patch_start, trailer_start, i;
+
+	/* Get the line count */
+	while (lines[count])
+		count++;
+
+	patch_start = find_patch_start(lines, count);
+	trailer_start = find_trailer_start(lines, patch_start);
+
+	/* Print lines before the trailers as is */
+	print_lines(lines, 0, trailer_start);
+
+	if (!has_blank_line_before(lines, trailer_start - 1))
+		printf("\n");
+
+	/* Parse trailer lines */
+	for (i = trailer_start; i < patch_start; i++) {
+		struct trailer_item *new = create_trailer_item(lines[i]->buf);
+		add_trailer_item(in_tok_first, in_tok_last, new);
+	}
+
+	return patch_start;
+}
-- 
2.0.1.674.ga7f57b7

[PATCH v13 06/11] trailer: put all the processing together and print

[Christian Couder]

This patch adds the process_trailers() function that
calls all the previously added processing functions
and then prints the results on the standard output.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 trailer.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 trailer.h |  6 +++++
 2 files changed, 75 insertions(+), 12 deletions(-)
 create mode 100644 trailer.h

diff --git a/trailer.c b/trailer.c
index 46e2fcb..8e41b8f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "string-list.h"
+#include "trailer.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -82,6 +83,35 @@ static void free_trailer_item(struct trailer_item *item)
 	free(item);
 }
 
+static char last_non_space_char(const char *s)
+{
+	int i;
+	for (i = strlen(s) - 1; i >= 0; i--)
+		if (!isspace(s[i]))
+			return s[i];
+	return '\0';
+}
+
+static void print_tok_val(const char *tok, const char *val)
+{
+	char c = last_non_space_char(tok);
+	if (!c)
+		return;
+	if (strchr(separators, c))
+		printf("%s%s\n", tok, val);
+	else
+		printf("%s%c %s\n", tok, separators[0], val);
+}
+
+static void print_all(struct trailer_item *first, int trim_empty)
+{
+	struct trailer_item *item;
+	for (item = first; item; item = item->next) {
+		if (!trim_empty || strlen(item->value) > 0)
+			print_tok_val(item->token, item->value);
+	}
+}
+
 static void update_last(struct trailer_item **last)
 {
 	if (*last)
@@ -489,18 +519,6 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra
 	return 0;
 }
 
-
-static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
-{
-	*dst = *src;
-	if (src->name)
-		dst->name = xstrdup(src->name);
-	if (src->key)
-		dst->key = xstrdup(src->key);
-	if (src->command)
-		dst->command = xstrdup(src->command);
-}
-
 static const char *token_from_item(struct trailer_item *item)
 {
 	if (item->conf.key)
@@ -705,3 +723,42 @@ static int process_input_file(struct strbuf **lines,
 
 	return patch_start;
 }
+
+static void free_all(struct trailer_item **first)
+{
+	while (*first) {
+		struct trailer_item *item = remove_first(first);
+		free_trailer_item(item);
+	}
+}
+
+void process_trailers(const char *file, int trim_empty, struct string_list *trailers)
+{
+	struct trailer_item *in_tok_first = NULL;
+	struct trailer_item *in_tok_last = NULL;
+	struct trailer_item *arg_tok_first;
+	struct strbuf **lines;
+	int patch_start;
+
+	/* Default config must be setup first */
+	git_config(git_trailer_default_config, NULL);
+	git_config(git_trailer_config, NULL);
+
+	lines = read_input_file(file);
+
+	/* Print the lines before the trailers */
+	patch_start = process_input_file(lines, &in_tok_first, &in_tok_last);
+
+	arg_tok_first = process_command_line_args(trailers);
+
+	process_trailers_lists(&in_tok_first, &in_tok_last, &arg_tok_first);
+
+	print_all(in_tok_first, trim_empty);
+
+	free_all(&in_tok_first);
+
+	/* Print the lines after the trailers as is */
+	print_lines(lines, patch_start, INT_MAX);
+
+	strbuf_list_free(lines);
+}
diff --git a/trailer.h b/trailer.h
new file mode 100644
index 0000000..8eb25d5
--- /dev/null
+++ b/trailer.h
@@ -0,0 +1,6 @@
+#ifndef TRAILER_H
+#define TRAILER_H
+
+void process_trailers(const char *file, int trim_empty, struct string_list *trailers);
+
+#endif /* TRAILER_H */
-- 
2.0.1.674.ga7f57b7

[PATCH v13 07/11] trailer: add interpret-trailers command

[Christian Couder]

This patch adds the "git interpret-trailers" command.
This command uses the previously added process_trailers()
function in trailer.c.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 .gitignore                   |  1 +
 Makefile                     |  1 +
 builtin.h                    |  1 +
 builtin/interpret-trailers.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 git.c                        |  1 +
 5 files changed, 48 insertions(+)
 create mode 100644 builtin/interpret-trailers.c

diff --git a/.gitignore b/.gitignore
index cf0d191..85593de 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
 /git-index-pack
 /git-init
 /git-init-db
+/git-interpret-trailers
 /git-instaweb
 /git-log
 /git-ls-files
diff --git a/Makefile b/Makefile
index ef82972..0b06ae0 100644
--- a/Makefile
+++ b/Makefile
@@ -953,6 +953,7 @@ BUILTIN_OBJS += builtin/hash-object.o
 BUILTIN_OBJS += builtin/help.o
 BUILTIN_OBJS += builtin/index-pack.o
 BUILTIN_OBJS += builtin/init-db.o
+BUILTIN_OBJS += builtin/interpret-trailers.o
 BUILTIN_OBJS += builtin/log.o
 BUILTIN_OBJS += builtin/ls-files.o
 BUILTIN_OBJS += builtin/ls-remote.o
diff --git a/builtin.h b/builtin.h
index 5d91f31..b87df70 100644
--- a/builtin.h
+++ b/builtin.h
@@ -73,6 +73,7 @@ extern int cmd_hash_object(int argc, const char **argv, const char *prefix);
 extern int cmd_help(int argc, const char **argv, const char *prefix);
 extern int cmd_index_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_init_db(int argc, const char **argv, const char *prefix);
+extern int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);
 extern int cmd_log(int argc, const char **argv, const char *prefix);
 extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
new file mode 100644
index 0000000..46838d2
--- /dev/null
+++ b/builtin/interpret-trailers.c
@@ -0,0 +1,44 @@
+/*
+ * Builtin "git interpret-trailers"
+ *
+ * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
+ *
+ */
+
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "string-list.h"
+#include "trailer.h"
+
+static const char * const git_interpret_trailers_usage[] = {
+	N_("git interpret-trailers [--trim-empty] [(--trailer <token>[(=|:)<value>])...] [<file>...]"),
+	NULL
+};
+
+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+{
+	int trim_empty = 0;
+	struct string_list trailers = STRING_LIST_INIT_DUP;
+
+	struct option options[] = {
+		OPT_BOOL(0, "trim-empty", &trim_empty, N_("trim empty trailers")),
+		OPT_STRING_LIST(0, "trailer", &trailers, N_("trailer"),
+				N_("trailer(s) to add")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_interpret_trailers_usage, 0);
+
+	if (argc) {
+		int i;
+		for (i = 0; i < argc; i++)
+			process_trailers(argv[i], trim_empty, &trailers);
+	} else
+		process_trailers(NULL, trim_empty, &trailers);
+
+	string_list_clear(&trailers, 0);
+
+	return 0;
+}
diff --git a/git.c b/git.c
index 5ebb32f..e327a90 100644
--- a/git.c
+++ b/git.c
@@ -417,6 +417,7 @@ static struct cmd_struct commands[] = {
 	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
 	{ "init", cmd_init_db, NO_SETUP },
 	{ "init-db", cmd_init_db, NO_SETUP },
+	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP },
 	{ "log", cmd_log, RUN_SETUP },
 	{ "ls-files", cmd_ls_files, RUN_SETUP },
 	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
-- 
2.0.1.674.ga7f57b7

[PATCH v13 08/11] trailer: add tests for "git interpret-trailers"

[Christian Couder]

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7513-interpret-trailers.sh | 726 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 726 insertions(+)
 create mode 100755 t/t7513-interpret-trailers.sh

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
new file mode 100755
index 0000000..fa37565
--- /dev/null
+++ b/t/t7513-interpret-trailers.sh
@@ -0,0 +1,726 @@
+#!/bin/sh
+#
+# Copyright (c) 2013, 2014 Christian Couder
+#
+
+test_description='git interpret-trailers'
+
+. ./test-lib.sh
+
+# When we want one trailing space at the end of each line, let's use sed
+# to make sure that these spaces are not removed by any automatic tool.
+
+test_expect_success 'setup' '
+	: >empty &&
+	cat >basic_message <<-\EOF &&
+		subject
+
+		body
+	EOF
+	cat >complex_message_body <<-\EOF &&
+		my subject
+
+		my body which is long
+		and contains some special
+		chars like : = ? !
+
+	EOF
+	sed -e "s/ Z\$/ /" >complex_message_trailers <<-\EOF &&
+		Fixes: Z
+		Acked-by: Z
+		Reviewed-by: Z
+		Signed-off-by: Z
+	EOF
+	cat >basic_patch <<-\EOF
+		---
+		 foo.txt | 2 +-
+		 1 file changed, 1 insertion(+), 1 deletion(-)
+
+		diff --git a/foo.txt b/foo.txt
+		index 0353767..1d91aa1 100644
+		--- a/foo.txt
+		+++ b/foo.txt
+		@@ -1,3 +1,3 @@
+
+		-bar
+		+baz
+
+		--
+		1.9.rc0.11.ga562ddc
+
+	EOF
+'
+
+test_expect_success 'without config' '
+	sed -e "s/ Z\$/ /" >expected <<-\EOF &&
+
+		ack: Peff
+		Reviewed-by: Z
+		Acked-by: Johan
+	EOF
+	git interpret-trailers --trailer "ack = Peff" --trailer "Reviewed-by" \
+		--trailer "Acked-by: Johan" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'without config in another order' '
+	sed -e "s/ Z\$/ /" >expected <<-\EOF &&
+
+		Acked-by: Johan
+		Reviewed-by: Z
+		ack: Peff
+	EOF
+	git interpret-trailers --trailer "Acked-by: Johan" --trailer "Reviewed-by" \
+		--trailer "ack = Peff" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '--trim-empty without config' '
+	cat >expected <<-\EOF &&
+
+		ack: Peff
+		Acked-by: Johan
+	EOF
+	git interpret-trailers --trim-empty --trailer ack=Peff \
+		--trailer "Reviewed-by" --trailer "Acked-by: Johan" \
+		--trailer "sob:" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with config setup' '
+	git config trailer.ack.key "Acked-by: " &&
+	cat >expected <<-\EOF &&
+
+		Acked-by: Peff
+	EOF
+	git interpret-trailers --trim-empty --trailer "ack = Peff" empty >actual &&
+	test_cmp expected actual &&
+	git interpret-trailers --trim-empty --trailer "Acked-by = Peff" empty >actual &&
+	test_cmp expected actual &&
+	git interpret-trailers --trim-empty --trailer "Acked-by :Peff" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with config setup and ":=" as separators' '
+	git config trailer.separators ":=" &&
+	git config trailer.ack.key "Acked-by= " &&
+	cat >expected <<-\EOF &&
+
+		Acked-by= Peff
+	EOF
+	git interpret-trailers --trim-empty --trailer "ack = Peff" empty >actual &&
+	test_cmp expected actual &&
+	git interpret-trailers --trim-empty --trailer "Acked-by= Peff" empty >actual &&
+	test_cmp expected actual &&
+	git interpret-trailers --trim-empty --trailer "Acked-by : Peff" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with config setup and "%" as separators' '
+	git config trailer.separators "%" &&
+	cat >expected <<-\EOF &&
+
+		bug% 42
+		count% 10
+		bug% 422
+	EOF
+	git interpret-trailers --trim-empty --trailer "bug = 42" \
+		--trailer count%10 --trailer "test: stuff" \
+		--trailer "bug % 422" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with "%" as separators and a message with trailers' '
+	cat >special_message <<-\EOF &&
+		Special Message
+
+		bug% 42
+		count% 10
+		bug% 422
+	EOF
+	cat >expected <<-\EOF &&
+		Special Message
+
+		bug% 42
+		count% 10
+		bug% 422
+		count% 100
+	EOF
+	git interpret-trailers --trailer count%100 \
+		special_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with config setup and ":=#" as separators' '
+	git config trailer.separators ":=#" &&
+	git config trailer.bug.key "Bug #" &&
+	cat >expected <<-\EOF &&
+
+		Bug #42
+	EOF
+	git interpret-trailers --trim-empty --trailer "bug = 42" empty >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with commit basic message' '
+	cat basic_message >expected &&
+	echo >>expected &&
+	git interpret-trailers <basic_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with basic patch' '
+	cat basic_message >input &&
+	cat basic_patch >>input &&
+	cat basic_message >expected &&
+	echo >>expected &&
+	cat basic_patch >>expected &&
+	git interpret-trailers <input >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with commit complex message as argument' '
+	cat complex_message_body complex_message_trailers >complex_message &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by: Z
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with 2 files arguments' '
+	cat basic_message >>expected &&
+	echo >>expected &&
+	cat basic_patch >>expected &&
+	git interpret-trailers complex_message input >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with message that has comments' '
+	cat basic_message >>message_with_comments &&
+	sed -e "s/ Z\$/ /" >>message_with_comments <<-\EOF &&
+		# comment
+
+		# other comment
+		Cc: Z
+		# yet another comment
+		Reviewed-by: Johan
+		Reviewed-by: Z
+		# last comment
+
+	EOF
+	cat basic_patch >>message_with_comments &&
+	cat basic_message >expected &&
+	cat >>expected <<-\EOF &&
+		# comment
+
+		Reviewed-by: Johan
+		Cc: Peff
+	EOF
+	cat basic_patch >>expected &&
+	git interpret-trailers --trim-empty --trailer "Cc: Peff" message_with_comments >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with commit complex message and trailer args' '
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by: Z
+		Signed-off-by: Z
+		Acked-by= Peff
+		Bug #42
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "bug: 42" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with complex patch, args and --trim-empty' '
+	cat complex_message >complex_patch &&
+	cat basic_patch >>complex_patch &&
+	cat complex_message_body >expected &&
+	cat >>expected <<-\EOF &&
+		Acked-by= Peff
+		Bug #42
+	EOF
+	cat basic_patch >>expected &&
+	git interpret-trailers --trim-empty --trailer "ack: Peff" \
+		--trailer "bug: 42" <complex_patch >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "where = before"' '
+	git config trailer.bug.where "before" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by: Z
+		Signed-off-by: Z
+		Acked-by= Peff
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "bug: 42" complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "where = after"' '
+	git config trailer.ack.where "after" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Reviewed-by: Z
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "bug: 42" complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "where = end"' '
+	git config trailer.review.key "Reviewed-by" &&
+	git config trailer.review.where "end" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Reviewed-by: Z
+		Signed-off-by: Z
+		Reviewed-by: Junio
+		Reviewed-by: Johannes
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "Reviewed-by: Junio" --trailer "Reviewed-by: Johannes" \
+		complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "where = start"' '
+	git config trailer.review.key "Reviewed-by" &&
+	git config trailer.review.where "start" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Reviewed-by: Johannes
+		Reviewed-by: Junio
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Reviewed-by: Z
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "Reviewed-by: Junio" --trailer "Reviewed-by: Johannes" \
+		complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "where = before" for a token in the middle of the message' '
+	git config trailer.review.key "Reviewed-by:" &&
+	git config trailer.review.where "before" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Reviewed-by:Johan
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "bug: 42" \
+		--trailer "review: Johan" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "where = before" and --trim-empty' '
+	cat complex_message_body >expected &&
+	cat >>expected <<-\EOF &&
+		Bug #46
+		Bug #42
+		Acked-by= Peff
+		Reviewed-by:Johan
+	EOF
+	git interpret-trailers --trim-empty --trailer "ack: Peff" \
+		--trailer "bug: 42" --trailer "review: Johan" \
+		--trailer "Bug: 46" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'the default is "ifExists = addIfDifferentNeighbor"' '
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "review:" \
+		--trailer "ack: Junio" --trailer "bug: 42" --trailer "ack: Peff" \
+		--trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'default "ifExists" is now "addIfDifferent"' '
+	git config trailer.ifexists "addIfDifferent" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Acked-by= Junio
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "review:" \
+		--trailer "ack: Junio" --trailer "bug: 42" --trailer "ack: Peff" \
+		--trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = addIfDifferent" with "where = end"' '
+	git config trailer.ack.ifExists "addIfDifferent" &&
+	git config trailer.ack.where "end" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Acked-by= Peff
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "review:" \
+		--trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = addIfDifferent" with "where = before"' '
+	git config trailer.ack.ifExists "addIfDifferent" &&
+	git config trailer.ack.where "before" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Peff
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "review:" \
+		--trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = addIfDifferentNeighbor" with "where = end"' '
+	git config trailer.ack.ifExists "addIfDifferentNeighbor" &&
+	git config trailer.ack.where "end" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Acked-by= Peff
+		Acked-by= Junio
+		Tested-by: Jakub
+		Acked-by= Junio
+		Acked-by= Peff
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "review:" \
+		--trailer "ack: Junio" --trailer "bug: 42" \
+		--trailer "Tested-by: Jakub" --trailer "ack: Junio" \
+		--trailer "ack: Junio" --trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = addIfDifferentNeighbor"  with "where = after"' '
+	git config trailer.ack.ifExists "addIfDifferentNeighbor" &&
+	git config trailer.ack.where "after" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+		Tested-by: Jakub
+	EOF
+	git interpret-trailers --trailer "ack: Peff" --trailer "review:" \
+		--trailer "ack: Junio" --trailer "bug: 42" \
+		--trailer "Tested-by: Jakub" --trailer "ack: Junio" \
+		--trailer "ack: Junio" --trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = addIfDifferentNeighbor" and --trim-empty' '
+	git config trailer.ack.ifExists "addIfDifferentNeighbor" &&
+	cat complex_message_body >expected &&
+	cat >>expected <<-\EOF &&
+		Bug #42
+		Acked-by= Peff
+		Acked-by= Junio
+		Acked-by= Peff
+	EOF
+	git interpret-trailers --trim-empty --trailer "ack: Peff" \
+		--trailer "Acked-by= Peff" --trailer "review:" \
+		--trailer "ack: Junio" --trailer "bug: 42" \
+		--trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = add" with "where = end"' '
+	git config trailer.ack.ifExists "add" &&
+	git config trailer.ack.where "end" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Acked-by= Peff
+		Acked-by= Peff
+		Tested-by: Jakub
+		Acked-by= Junio
+		Tested-by: Johannes
+		Acked-by= Peff
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "Acked-by= Peff" --trailer "review:" \
+		--trailer "Tested-by: Jakub" --trailer "ack: Junio" \
+		--trailer "bug: 42" --trailer "Tested-by: Johannes" \
+		--trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = add" with "where = after"' '
+	git config trailer.ack.ifExists "add" &&
+	git config trailer.ack.where "after" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Acked-by= Peff
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "Acked-by= Peff" --trailer "review:" \
+		--trailer "ack: Junio" --trailer "bug: 42" \
+		--trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = replace"' '
+	git config trailer.fix.key "Fixes: " &&
+	git config trailer.fix.ifExists "replace" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+		Fixes: 22
+	EOF
+	git interpret-trailers --trailer "review:" \
+		--trailer "fix=53" --trailer "ack: Junio" --trailer "fix=22" \
+		--trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = replace" with "where = after"' '
+	git config trailer.fix.where "after" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: 22
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "review:" \
+		--trailer "fix=53" --trailer "ack: Junio" --trailer "fix=22" \
+		--trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifExists = doNothing"' '
+	git config trailer.fix.ifExists "doNothing" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=53" \
+		--trailer "ack: Junio" --trailer "fix=22" \
+		--trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'the default is "ifMissing = add"' '
+	git config trailer.cc.key "Cc: " &&
+	git config trailer.cc.where "before" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Cc: Linus
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=53" \
+		--trailer "cc=Linus" --trailer "ack: Junio" \
+		--trailer "fix=22" --trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'when default "ifMissing" is "doNothing"' '
+	git config trailer.ifmissing "doNothing" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=53" \
+		--trailer "cc=Linus" --trailer "ack: Junio" \
+		--trailer "fix=22" --trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual &&
+	git config trailer.ifmissing "add"
+'
+
+test_expect_success 'using "ifMissing = add" with "where = end"' '
+	git config trailer.cc.key "Cc: " &&
+	git config trailer.cc.where "end" &&
+	git config trailer.cc.ifMissing "add" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+		Cc: Linus
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=53" \
+		--trailer "ack: Junio" --trailer "fix=22" \
+		--trailer "bug: 42" --trailer "cc=Linus" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifMissing = add" with "where = before"' '
+	git config trailer.cc.key "Cc: " &&
+	git config trailer.cc.where "before" &&
+	git config trailer.cc.ifMissing "add" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Cc: Linus
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=53" \
+		--trailer "ack: Junio" --trailer "fix=22" \
+		--trailer "bug: 42" --trailer "cc=Linus" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'using "ifMissing = doNothing"' '
+	git config trailer.cc.ifMissing "doNothing" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=53" \
+		--trailer "cc=Linus" --trailer "ack: Junio" \
+		--trailer "fix=22" --trailer "bug: 42" --trailer "ack: Peff" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'default "where" is now "after"' '
+	git config trailer.where "after" &&
+	git config --unset trailer.ack.where &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Bug #42
+		Fixes: Z
+		Acked-by= Z
+		Acked-by= Peff
+		Acked-by= Peff
+		Acked-by= Junio
+		Acked-by= Peff
+		Reviewed-by:
+		Signed-off-by: Z
+		Tested-by: Jakub
+		Tested-by: Johannes
+	EOF
+	git interpret-trailers --trailer "ack: Peff" \
+		--trailer "Acked-by= Peff" --trailer "review:" \
+		--trailer "Tested-by: Jakub" --trailer "ack: Junio" \
+		--trailer "bug: 42" --trailer "Tested-by: Johannes" \
+		--trailer "ack: Peff" <complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
2.0.1.674.ga7f57b7

[PATCH v13 09/11] trailer: execute command from 'trailer.<name>.command'

[Christian Couder]

Let the user specify a command that will give on its standard output
the value to use for the specified trailer.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 trailer.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/trailer.c b/trailer.c
index 8e41b8f..baef034 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,7 @@
 #include "cache.h"
 #include "string-list.h"
+#include "run-command.h"
+#include "string-list.h"
 #include "trailer.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
@@ -14,6 +16,7 @@ struct conf_info {
 	char *name;
 	char *key;
 	char *command;
+	unsigned command_uses_arg : 1;
 	enum action_where where;
 	enum action_if_exists if_exists;
 	enum action_if_missing if_missing;
@@ -33,6 +36,8 @@ static struct trailer_item *first_conf_item;
 
 static char *separators = ":";
 
+#define TRAILER_ARG_STRING "$ARG"
+
 static int after_or_end(enum action_where where)
 {
 	return (where == WHERE_AFTER) || (where == WHERE_END);
@@ -73,6 +78,13 @@ static inline int contains_only_spaces(const char *str)
 	return !*s;
 }
 
+static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b)
+{
+	const char *ptr = strstr(sb->buf, a);
+	if (ptr)
+		strbuf_splice(sb, ptr - sb->buf, strlen(a), b, strlen(b));
+}
+
 static void free_trailer_item(struct trailer_item *item)
 {
 	free(item->conf.name);
@@ -478,6 +490,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
 		if (conf->command)
 			warning(_("more than one %s"), conf_key);
 		conf->command = xstrdup(value);
+		conf->command_uses_arg = !!strstr(conf->command, TRAILER_ARG_STRING);
 		break;
 	case TRAILER_WHERE:
 		if (set_where(conf, value))
@@ -519,6 +532,46 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra
 	return 0;
 }
 
+static int read_from_command(struct child_process *cp, struct strbuf *buf)
+{
+	if (run_command(cp))
+		return error("running trailer command '%s' failed", cp->argv[0]);
+	if (strbuf_read(buf, cp->out, 1024) < 1)
+		return error("reading from trailer command '%s' failed", cp->argv[0]);
+	strbuf_trim(buf);
+	return 0;
+}
+
+static const char *apply_command(const char *command, const char *arg)
+{
+	struct strbuf cmd = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process cp;
+	const char *argv[] = {NULL, NULL};
+	const char *result;
+
+	strbuf_addstr(&cmd, command);
+	if (arg)
+		strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
+
+	argv[0] = cmd.buf;
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.no_stdin = 1;
+	cp.out = -1;
+	cp.use_shell = 1;
+
+	if (read_from_command(&cp, &buf)) {
+		strbuf_release(&buf);
+		result = xstrdup("");
+	} else
+		result = strbuf_detach(&buf, NULL);
+
+	strbuf_release(&cmd);
+	return result;
+}
+
 static const char *token_from_item(struct trailer_item *item)
 {
 	if (item->conf.key)
@@ -537,6 +590,10 @@ static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
 		duplicate_conf(&new->conf, &conf_item->conf);
 		new->token = xstrdup(token_from_item(conf_item));
 		free(tok);
+		if (conf_item->conf.command_uses_arg || !val) {
+			new->value = apply_command(conf_item->conf.command, val);
+			free(val);
+		}
 	} else {
 		duplicate_conf(&new->conf, &default_conf_info);
 		new->token = tok;
@@ -600,12 +657,21 @@ static struct trailer_item *process_command_line_args(struct string_list *traile
 	struct trailer_item *arg_tok_first = NULL;
 	struct trailer_item *arg_tok_last = NULL;
 	struct string_list_item *tr;
+	struct trailer_item *item;
 
 	for_each_string_list_item(tr, trailers) {
 		struct trailer_item *new = create_trailer_item(tr->string);
 		add_trailer_item(&arg_tok_first, &arg_tok_last, new);
 	}
 
+	/* Add conf commands that don't use $ARG */
+	for (item = first_conf_item; item; item = item->next) {
+		if (item->conf.command && !item->conf.command_uses_arg) {
+			struct trailer_item *new = new_trailer_item(item, NULL, NULL);
+			add_trailer_item(&arg_tok_first, &arg_tok_last, new);
+		}
+	}
+
 	return arg_tok_first;
 }
 
-- 
2.0.1.674.ga7f57b7

[PATCH v13 10/11] trailer: add tests for commands in config file

[Christian Couder]

And add a few other tests for some special cases.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7513-interpret-trailers.sh | 124 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index fa37565..562c00a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -723,4 +723,128 @@ test_expect_success 'default "where" is now "after"' '
 	test_cmp expected actual
 '
 
+test_expect_success 'with simple command' '
+	git config trailer.sign.key "Signed-off-by: " &&
+	git config trailer.sign.where "after" &&
+	git config trailer.sign.ifExists "addIfDifferentNeighbor" &&
+	git config trailer.sign.command "echo \"A U Thor <author@example.com>\"" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Signed-off-by: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=22" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with command using commiter information' '
+	git config trailer.sign.ifExists "addIfDifferent" &&
+	git config trailer.sign.command "echo \"\$GIT_COMMITTER_NAME <\$GIT_COMMITTER_EMAIL>\"" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Signed-off-by: C O Mitter <committer@example.com>
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=22" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with command using author information' '
+	git config trailer.sign.key "Signed-off-by: " &&
+	git config trailer.sign.where "after" &&
+	git config trailer.sign.ifExists "addIfDifferentNeighbor" &&
+	git config trailer.sign.command "echo \"\$GIT_AUTHOR_NAME <\$GIT_AUTHOR_EMAIL>\"" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Signed-off-by: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=22" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'setup a commit' '
+	echo "Content of the first commit." > a.txt &&
+	git add a.txt &&
+	git commit -m "Add file a.txt"
+'
+
+test_expect_success 'with command using $ARG' '
+	git config trailer.fix.ifExists "overwrite" &&
+	git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG" &&
+	FIXED=$(git log -1 --oneline --format="%h (%s)" --abbrev-commit --abbrev=14 HEAD) &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-EOF &&
+		Fixes: $FIXED
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Signed-off-by: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with failing command using $ARG' '
+	git config trailer.fix.ifExists "overwrite" &&
+	git config trailer.fix.command "false \$ARG" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-EOF &&
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Signed-off-by: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \
+		<complex_message >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with empty tokens' '
+	cat >expected <<-EOF &&
+
+		Signed-off-by: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers --trailer ":" --trailer ":test" >actual <<-EOF &&
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'with command but no key' '
+	git config --unset trailer.sign.key &&
+	cat >expected <<-EOF &&
+
+		sign: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers >actual <<-EOF &&
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'with no command and no key' '
+	git config --unset trailer.review.key &&
+	cat >expected <<-EOF &&
+
+		review: Junio
+		sign: A U Thor <author@example.com>
+	EOF
+	git interpret-trailers --trailer "review:Junio" >actual <<-EOF &&
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
2.0.1.674.ga7f57b7

[PATCH v13 11/11] Documentation: add documentation for 'git interpret-trailers'

[Christian Couder]

While at it add git-interpret-trailers to "command-list.txt".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-interpret-trailers.txt | 308 +++++++++++++++++++++++++++++++
 command-list.txt                         |   1 +
 2 files changed, 309 insertions(+)
 create mode 100644 Documentation/git-interpret-trailers.txt

diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
new file mode 100644
index 0000000..cf5b194
--- /dev/null
+++ b/Documentation/git-interpret-trailers.txt
@@ -0,0 +1,308 @@
+git-interpret-trailers(1)
+=========================
+
+NAME
+----
+git-interpret-trailers - help add stuctured information into commit messages
+
+SYNOPSIS
+--------
+[verse]
+'git interpret-trailers' [--trim-empty] [(--trailer <token>[(=|:)<value>])...] [<file>...]
+
+DESCRIPTION
+-----------
+Help adding 'trailers' lines, that look similar to RFC 822 e-mail
+headers, at the end of the otherwise free-form part of a commit
+message.
+
+This command reads some patches or commit messages from either the
+<file> arguments or the standard input if no <file> is specified. Then
+this command applies the arguments passed using the `--trailer`
+option, if any, to the commit message part of each input file. The
+result is emitted on the standard output.
+
+Some configuration variables control the way the `--trailer` arguments
+are applied to each commit message and the way any existing trailer in
+the commit message is changed. They also make it possible to
+automatically add some trailers.
+
+By default, a '<token>=<value>' or '<token>:<value>' argument given
+using `--trailer` will be appended after the existing trailers only if
+the last trailer has a different (<token>, <value>) pair (or if there
+is no existing trailer). The <token> and <value> parts will be trimmed
+to remove starting and trailing whitespace, and the resulting trimmed
+<token> and <value> will appear in the message like this:
+
+------------------------------------------------
+token: value
+------------------------------------------------
+
+This means that the trimmed <token> and <value> will be separated by
+`': '` (one colon followed by one space).
+
+By default the new trailer will appear at the end of all the existing
+trailers. If there is no existing trailer, the new trailer will appear
+after the commit message part of the ouput, and, if there is no line
+with only spaces at the end of the commit message part, one blank line
+will be added before the new trailer.
+
+The trailers are recognized in the input message using the following
+rules:
+
+* by default only lines that contains a ':' (colon) are considered
+  trailers,
+
+* the trailer lines must all be next to each other,
+
+* after them it's only possible to have some lines that contain only
+  spaces, and then a patch; the patch part is recognized using the
+  fact that its first line starts with '---' (three minus signs),
+
+* before them there must be at least one line with only spaces.
+
+Note that 'trailers' do not follow and are not intended to follow many
+rules for RFC 822 headers. For example they do not follow the line
+folding rules, the encoding rules and probably many other rules.
+
+OPTIONS
+-------
+--trim-empty::
+	If the <value> part of any trailer contains only whitespace,
+	the whole trailer will be removed from the resulting message.
+	This apply to existing trailers as well as new trailers.
+
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer to the input messages. See the description of this
+	command.
+
+CONFIGURATION VARIABLES
+-----------------------
+
+trailer.separators::
+	This option tells which characters are recognized as trailer
+	separators. By default only ':' is recognized as a trailer
+	separator, except that '=' is always accepted on the command
+	line for compatibility with other git commands.
++
+The first character given by this option will be the default character
+used when another separator is not specified in the config for this
+trailer.
++
+For example, if the value for this option is "%=$", then only lines
+using the format '<token><sep><value>' with <sep> containing '%', '='
+or '$' and then spaces will be considered trailers. And '%' will be
+the default separator used, so by default trailers will appear like:
+'<token>% <value>' (one percent sign and one space will appear between
+the token and the value).
+
+trailer.where::
+	This option tells where a new trailer will be added.
++
+This can be `end`, which is the default, `start`, `after` or `before`.
++
+If it is `end`, then each new trailer will appear at the end of the
+existing trailers.
++
+If it is `start`, then each new trailer will appear at the start,
+instead of the end, of the existing trailers.
++
+If it is `after`, then each new trailer will appear just after the
+last trailer with the same <token>.
++
+If it is `before`, then each new trailer will appear just before the
+last trailer with the same <token>.
+
+trailer.ifexists::
+	This option makes it possible to choose what action will be
+	performed when there is already at least one trailer with the
+	same <token> in the message.
++
+The valid values for this option are: `addIfDifferentNeighbor` (this
+is the default), `addIfDifferent`, `add`, `overwrite` or `doNothing`.
++
+With `addIfDifferentNeighbor`, a new trailer will be added only if no
+trailer with the same (<token>, <value>) pair is above or below the line
+where the new trailer will be added.
++
+With `addIfDifferent`, a new trailer will be added only if no trailer
+with the same (<token>, <value>) pair is already in the message.
++
+With `add`, a new trailer will be added, even if some trailers with
+the same (<token>, <value>) pair are already in the message.
++
+With `replace`, an existing trailer with the same <token> will be
+deleted and the new trailer will be added. The deleted trailer will be
+the closest one (with the same <token>) to the place where the new one
+will be added.
++
+With `doNothing`, nothing will be done; that is no new trailer will be
+added if there is already one with the same <token> in the message.
+
+trailer.ifmissing::
+	This option makes it possible to choose what action will be
+	performed when there is not yet any trailer with the same
+	<token> in the message.
++
+The valid values for this option are: `add` (this is the default) and
+`doNothing`.
++
+With `add`, a new trailer will be added.
++
+With `doNothing`, nothing will be done.
+
+trailer.<token>.key::
+	This `key` will be used instead of <token> in the trailer. At
+	the end of this key, a separator can appear and then some
+	space characters. By default the only valid separator is ':',
+	but this can be changed using the `trailer.separators` config
+	variable.
++
+If there is a separator, then the key will be used instead of both the
+<token> and the default separator when adding the trailer.
+
+trailer.<token>.where::
+	This option takes the same values as the 'trailer.where'
+	configuration variable and it overrides what is specified by
+	that option for trailers with the specified <token>.
+
+trailer.<token>.ifexist::
+	This option takes the same values as the 'trailer.ifexist'
+	configuration variable and it overrides what is specified by
+	that option for trailers with the specified <token>.
+
+trailer.<token>.ifmissing::
+	This option takes the same values as the 'trailer.ifmissing'
+	configuration variable and it overrides what is specified by
+	that option for trailers with the specified <token>.
+
+trailer.<token>.command::
+	This option can be used to specify a shell command that will
+	be used to automatically add or modify a trailer with the
+	specified <token>.
++
+When this option is specified, the behavior is as if a special
+'<token>=<value>' argument were added at the end of the command line,
+where <value> is taken to be the standard output of the specified
+command with any leading and trailing whitespace trimmed off.
++
+If the command contains the `$ARG` string, this string will be
+replaced with the <value> part of an existing trailer with the same
+<token>, if any, before the command is launched.
+
+EXAMPLES
+--------
+
+* Configure a 'sign' trailer with a 'Signed-off-by' key, and then
+  add two of these trailers to a message:
++
+------------
+$ git config trailer.sign.key "Signed-off-by"
+$ cat msg.txt
+subject
+
+message
+$ cat msg.txt | git interpret-trailers --trailer 'sign: Alice <alice@example.com>' --trailer 'sign: Bob <bob@example.com>'
+subject
+
+message
+
+Signed-off-by: Alice <alice@example.com>
+Signed-off-by: Bob <bob@example.com>
+------------
+
+* Extract the last commit as a patch, and add a 'Cc' and a
+  'Reviewed-by' trailer to it:
++
+------------
+$ git format-patch -1
+0001-foo.patch
+$ git interpret-trailers --trailer 'Cc: Alice <alice@example.com>' --trailer 'Reviewed-by: Bob <bob@example.com>' 0001-foo.patch >0001-bar.patch
+------------
+
+* Configure a 'sign' trailer with a command to automatically add a
+  'Signed-off-by: ' with the author information only if there is no
+  'Signed-off-by: ' already, and show how it works:
++
+------------
+$ git config trailer.sign.key "Signed-off-by: "
+$ git config trailer.sign.ifmissing add
+$ git config trailer.sign.ifexists doNothing
+$ git config trailer.sign.command 'echo "$(git config user.name) <$(git config user.email)>"'
+$ git interpret-trailers <<EOF
+> EOF
+
+Signed-off-by: Bob <bob@example.com>
+$ git interpret-trailers <<EOF
+> Signed-off-by: Alice <alice@example.com>
+> EOF
+
+Signed-off-by: Alice <alice@example.com>
+------------
+
+* Configure a 'fix' trailer with a key that contains a '#' and no
+  space after this character, and show how it works:
++
+------------
+$ git config trailer.separators ":#"
+$ git config trailer.fix.key "Fix #"
+$ echo "subject" | git interpret-trailers --trailer fix=42
+subject
+
+Fix #42
+------------
+
+* Configure a 'see' trailer with a command to show the subject of a
+  commit that is related, and show how it works:
++
+------------
+$ git config trailer.see.key "See-also: "
+$ git config trailer.see.ifExists "replace"
+$ git config trailer.see.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG"
+$ git interpret-trailers <<EOF
+> subject
+> 
+> message
+> 
+> see: HEAD~2
+> EOF
+subject
+
+message
+
+See-also: fe3187489d69c4 (subject of related commit)
+------------
+
+* Configure a commit template with some trailers with empty values,
+  then configure a commit-msg hook that uses 'git interpret-trailers'
+  to remove trailers with empty values and to add a 'git-version'
+  trailer:
++
+------------
+$ cat >commit_template.txt <<EOF
+> ***subject***
+> 
+> ***message***
+> 
+> Fixes: 
+> Cc: 
+> Reviewed-by: 
+> Signed-off-by: 
+> EOF
+$ git config commit.template commit_template.txt
+$ cat >.git/hooks/commit-msg <<EOF
+> #!/bin/sh
+> git interpret-trailers --trim-empty --trailer "git-version: \$(git describe)" "\$1" > "\$1.new"
+> mv "\$1.new" "\$1"
+> EOF
+$ chmod +x .git/hooks/commit-msg
+------------
+
+SEE ALSO
+--------
+linkgit:git-commit[1], linkgit:git-format-patch[1], linkgit:git-config[1]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/command-list.txt b/command-list.txt
index a3ff0c9..f1eae08 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -62,6 +62,7 @@ git-imap-send                           foreignscminterface
 git-index-pack                          plumbingmanipulators
 git-init                                mainporcelain common
 git-instaweb                            ancillaryinterrogators
+git-interpret-trailers                  purehelpers
 gitk                                    mainporcelain
 git-log                                 mainporcelain common
 git-ls-files                            plumbinginterrogators
-- 
2.0.1.674.ga7f57b7

Re: [PATCH v13 00/11] Add interpret-trailers builtin

[Christian Couder]

On Sat, Aug 16, 2014 at 6:06 PM, Christian Couder
<chriscool@tuxfamily.org> wrote:
>
> 3) Changes since version 12, thanks to Jakub, Michael, Johan and Junio:
>
> * "end" and "start" values for "trailer.<token>.where" have been
>   implemented
> * "end" has been made the default value for "where", but this default
>   value can be changed using the new "trailer.where" config
>   variable
> * "addIfDifferentNeighbor" is now the default value for "ifexists",
>   but this default can be changed using the new "trailer.ifexists"
>   config variable
> * the new "trailer.ifmissing" can be used to change the default value
>   for "ifmissing" (which is "add")
> * by default the only separator is ':', this can be changed by using
>   the new "trailer.separators" config variable
> * only the configured separators (or just ':' by default) are used for
>   both input parsing and output printing; the only exception is that
>   '=' is always accepted as separator when parsing
>   "--trailer '<token><sep><value>'" command line arguments, for
>   compatibility with other git commands
> * 14 tests have been added

I forgot the following change:

* "overwrite" value for "ifexists" option has been replaced with
  "replace" value; when used with "where = after" or
  "where = before", "replace" does the same thing as "overwrite"
  did; but when used with "where = end" or "where = start", the
  new trailer appears at the end or the start of the existing trailers,
  instead of where the replaced trailer was

Re: [PATCH v13 11/11] Documentation: add documentation for 'git interpret-trailers'

[Marc Branchaud]

On 14-08-16 12:06 PM, Christian Couder wrote:
> While at it add git-interpret-trailers to "command-list.txt".
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-interpret-trailers.txt | 308 +++++++++++++++++++++++++++++++
>  command-list.txt                         |   1 +
>  2 files changed, 309 insertions(+)
>  create mode 100644 Documentation/git-interpret-trailers.txt
> 
> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
> new file mode 100644
> index 0000000..cf5b194
> --- /dev/null
> +++ b/Documentation/git-interpret-trailers.txt
> @@ -0,0 +1,308 @@
> +git-interpret-trailers(1)
> +=========================
> +
> +NAME
> +----
> +git-interpret-trailers - help add stuctured information into commit messages
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git interpret-trailers' [--trim-empty] [(--trailer <token>[(=|:)<value>])...] [<file>...]
> +
> +DESCRIPTION
> +-----------
> +Help adding 'trailers' lines, that look similar to RFC 822 e-mail
> +headers, at the end of the otherwise free-form part of a commit
> +message.
> +
> +This command reads some patches or commit messages from either the
> +<file> arguments or the standard input if no <file> is specified. Then
> +this command applies the arguments passed using the `--trailer`
> +option, if any, to the commit message part of each input file. The
> +result is emitted on the standard output.
> +
> +Some configuration variables control the way the `--trailer` arguments
> +are applied to each commit message and the way any existing trailer in
> +the commit message is changed. They also make it possible to
> +automatically add some trailers.
> +
> +By default, a '<token>=<value>' or '<token>:<value>' argument given
> +using `--trailer` will be appended after the existing trailers only if
> +the last trailer has a different (<token>, <value>) pair (or if there
> +is no existing trailer). The <token> and <value> parts will be trimmed
> +to remove starting and trailing whitespace, and the resulting trimmed
> +<token> and <value> will appear in the message like this:
> +
> +------------------------------------------------
> +token: value
> +------------------------------------------------
> +
> +This means that the trimmed <token> and <value> will be separated by
> +`': '` (one colon followed by one space).
> +
> +By default the new trailer will appear at the end of all the existing
> +trailers. If there is no existing trailer, the new trailer will appear
> +after the commit message part of the ouput, and, if there is no line
> +with only spaces at the end of the commit message part, one blank line
> +will be added before the new trailer.
> +
> +The trailers are recognized in the input message using the following
> +rules:
> +
> +* by default only lines that contains a ':' (colon) are considered

s/contains/contain/

> +  trailers,
> +
> +* the trailer lines must all be next to each other,
> +
> +* after them it's only possible to have some lines that contain only
> +  spaces, and then a patch; the patch part is recognized using the
> +  fact that its first line starts with '---' (three minus signs),

Is that "starts with" or "consists solely of"?

> +
> +* before them there must be at least one line with only spaces.

I had little bit of trouble parsing those three points, and it seems like a
lot of text to describe something simple.  How about a single paragraph:

Existing trailers are extracted from the input message by looking for a group
of one or more lines that contain a colon (by default), where the group is
preceded by one or more empty (or whitespace-only) lines.  The group must
either be at the end of the message or be the last non-whitespace lines
before a line that starts with '---' (three minus signs).


Also, will a trailer be recognized if there is whitespace before and/or after
the separator?  Can there be whitespace before the token?  In the token?  (I
don't feel strongly about the answers to these questions, they just came to
mind as I read the documentation.)

> +
> +Note that 'trailers' do not follow and are not intended to follow many
> +rules for RFC 822 headers. For example they do not follow the line
> +folding rules, the encoding rules and probably many other rules.
> +
> +OPTIONS
> +-------
> +--trim-empty::
> +	If the <value> part of any trailer contains only whitespace,
> +	the whole trailer will be removed from the resulting message.
> +	This apply to existing trailers as well as new trailers.
> +
> +--trailer <token>[(=|:)<value>]::
> +	Specify a (<token>, <value>) pair that should be applied as a
> +	trailer to the input messages. See the description of this
> +	command.
> +
> +CONFIGURATION VARIABLES
> +-----------------------
> +
> +trailer.separators::
> +	This option tells which characters are recognized as trailer
> +	separators. By default only ':' is recognized as a trailer
> +	separator, except that '=' is always accepted on the command
> +	line for compatibility with other git commands.
> ++
> +The first character given by this option will be the default character
> +used when another separator is not specified in the config for this
> +trailer.
> ++
> +For example, if the value for this option is "%=$", then only lines
> +using the format '<token><sep><value>' with <sep> containing '%', '='
> +or '$' and then spaces will be considered trailers. And '%' will be
> +the default separator used, so by default trailers will appear like:
> +'<token>% <value>' (one percent sign and one space will appear between
> +the token and the value).
> +
> +trailer.where::
> +	This option tells where a new trailer will be added.
> ++
> +This can be `end`, which is the default, `start`, `after` or `before`.
> ++
> +If it is `end`, then each new trailer will appear at the end of the
> +existing trailers.
> ++
> +If it is `start`, then each new trailer will appear at the start,
> +instead of the end, of the existing trailers.
> ++
> +If it is `after`, then each new trailer will appear just after the
> +last trailer with the same <token>.
> ++
> +If it is `before`, then each new trailer will appear just before the
> +last trailer with the same <token>.

It seems to me that it would be more sensible to make the new trailer appear
before the *first* trailer with the same token.  Otherwise you can end up
with a somewhat unintuitive order, e.g. if we add values A B C D E (in that
order) we get
	foo: B
	foo: C
	foo: D
	foo: E
	foo: A

		M.

Re: [PATCH v13 11/11] Documentation: add documentation for 'git interpret-trailers'

[Christian Couder]

On Thu, Aug 21, 2014 at 12:05 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:
> On 14-08-16 12:06 PM, Christian Couder wrote:
>
>> +The trailers are recognized in the input message using the following
>> +rules:
>> +
>> +* by default only lines that contains a ':' (colon) are considered
>
> s/contains/contain/

Ok.

>> +  trailers,
>> +
>> +* the trailer lines must all be next to each other,
>> +
>> +* after them it's only possible to have some lines that contain only
>> +  spaces, and then a patch; the patch part is recognized using the
>> +  fact that its first line starts with '---' (three minus signs),
>
> Is that "starts with" or "consists solely of"?

It is starts with. (The starts_with() function is used.)

>> +
>> +* before them there must be at least one line with only spaces.
>
> I had little bit of trouble parsing those three points, and it seems like a
> lot of text to describe something simple.  How about a single paragraph:
>
> Existing trailers are extracted from the input message by looking for a group
> of one or more lines that contain a colon (by default), where the group is
> preceded by one or more empty (or whitespace-only) lines.  The group must
> either be at the end of the message or be the last non-whitespace lines
> before a line that starts with '---' (three minus signs).

Ok, I will use something like that, thanks.

> Also, will a trailer be recognized if there is whitespace before and/or after
> the separator?

Yes.

> Can there be whitespace before the token?

Yes.

>  In the token?

Yes.

> (I don't feel strongly about the answers to these questions, they just came to
> mind as I read the documentation.)

Ok, I will add something.

>> +Note that 'trailers' do not follow and are not intended to follow many
>> +rules for RFC 822 headers. For example they do not follow the line
>> +folding rules, the encoding rules and probably many other rules.
>> +
>> +OPTIONS
>> +-------
>> +--trim-empty::
>> +     If the <value> part of any trailer contains only whitespace,
>> +     the whole trailer will be removed from the resulting message.
>> +     This apply to existing trailers as well as new trailers.
>> +
>> +--trailer <token>[(=|:)<value>]::
>> +     Specify a (<token>, <value>) pair that should be applied as a
>> +     trailer to the input messages. See the description of this
>> +     command.
>> +
>> +CONFIGURATION VARIABLES
>> +-----------------------
>> +
>> +trailer.separators::
>> +     This option tells which characters are recognized as trailer
>> +     separators. By default only ':' is recognized as a trailer
>> +     separator, except that '=' is always accepted on the command
>> +     line for compatibility with other git commands.
>> ++
>> +The first character given by this option will be the default character
>> +used when another separator is not specified in the config for this
>> +trailer.
>> ++
>> +For example, if the value for this option is "%=$", then only lines
>> +using the format '<token><sep><value>' with <sep> containing '%', '='
>> +or '$' and then spaces will be considered trailers. And '%' will be
>> +the default separator used, so by default trailers will appear like:
>> +'<token>% <value>' (one percent sign and one space will appear between
>> +the token and the value).
>> +
>> +trailer.where::
>> +     This option tells where a new trailer will be added.
>> ++
>> +This can be `end`, which is the default, `start`, `after` or `before`.
>> ++
>> +If it is `end`, then each new trailer will appear at the end of the
>> +existing trailers.
>> ++
>> +If it is `start`, then each new trailer will appear at the start,
>> +instead of the end, of the existing trailers.
>> ++
>> +If it is `after`, then each new trailer will appear just after the
>> +last trailer with the same <token>.
>> ++
>> +If it is `before`, then each new trailer will appear just before the
>> +last trailer with the same <token>.
>
> It seems to me that it would be more sensible to make the new trailer appear
> before the *first* trailer with the same token.

Yeah, it is a copy-paste mistake that I will fix.

Thanks for the careful review,
Christian.

Re: [PATCH v13 11/11] Documentation: add documentation for 'git interpret-trailers'

[Marc Branchaud]

On 14-08-20 11:39 PM, Christian Couder wrote:
> On Thu, Aug 21, 2014 at 12:05 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:
>> On 14-08-16 12:06 PM, Christian Couder wrote:
>>> +
>>> +* after them it's only possible to have some lines that contain only
>>> +  spaces, and then a patch; the patch part is recognized using the
>>> +  fact that its first line starts with '---' (three minus signs),
>>
>> Is that "starts with" or "consists solely of"?
> 
> It is starts with. (The starts_with() function is used.)

Wouldn't it be more robust to do it the other way?  I can imagine cases when
a human might want to start a line of text with "---", whereas we can make
sure that git tools always use a plain "---" line with no extra text.

Not a big deal either way though.  Thanks for working on this!

		M.

Re: [PATCH v13 00/11] Add interpret-trailers builtin

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

>   '=' is always accepted as separator when parsing
>   "--trailer '<token><sep><value>'" command line arguments, for
>   compatibility with other git commands

Hmph.  Which of other commands take "--option foo=bar"?

Puzzled...

Re: [PATCH v13 00/11] Add interpret-trailers builtin

[Christian Couder]

From: Junio C Hamano <gitster@pobox.com>

> Christian Couder <chriscool@tuxfamily.org> writes:
> 
>>   '=' is always accepted as separator when parsing
>>   "--trailer '<token><sep><value>'" command line arguments, for
>>   compatibility with other git commands
> 
> Hmph.  Which of other commands take "--option foo=bar"?
> 
> Puzzled...

Most commands accept "--option=value". I know that it is different
from "--option foo=bar", but it could reduce user mistakes if we also
accept "--trailer foo=bar".

And if we accept it, then whatever the user has configured in the
"trailer.separators" option, "--trailer foo=bar" will always work and
add the default separator. So it is a generic way to add
"foo<separator> bar" to the trailers whatever the user want as a
separator.

But maybe I should reword the documentation to add this reason. (I did
not do that in the v14 series I just sent.)

Thanks,
Christian.