{"thread":{"id":"36298","subject":"[PATCH v8 03/12] Move lower case functions into wrapper.c","startedAt":"2014-03-26T22:15:18Z","lastAt":"2014-03-28T18:50:18Z","messageCount":27,"participants":["Christian Couder","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":8,"patchTotal":12},"messages":[{"id":"237866","messageId":"20140326215858.11352.89243.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":null,"subject":"[PATCH v8 00/12] Add interpret-trailers builtin","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:18Z","receivedAt":"2014-03-26T22:15:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch series implements a new command:\n\n git interpret-trailers\n\nand an infrastructure to process trailers that can be reused,\nfor example in \"commit.c\".\n\n1) Rationale:\n\nThis command should help with RFC 822 style headers, called\n\"trailers\", that are found at the end of commit messages.\n\n(Note that these headers do not follow and are not intended to\nfollow many rules that are in RFC 822. For example they do not\nfollow the line breaking rules, the encoding rules and probably\nmany other rules.)\n\nFor a long time, these trailers have become a de facto standard\nway to add helpful information into commit messages.\n\nUntil now git commit has only supported the well known\n\"Signed-off-by: \" trailer, that is used by many projects like\nthe Linux kernel and Git.\n\nIt is better to implement features for these trailers first in a\nnew command rather than in builtin/commit.c, because this way the\nprepare-commit-msg and commit-msg hooks can reuse this command.\n \n2) Current state:\n\nCurrently the usage string of this command is:\n\ngit interpret-trailers [--trim-empty] [([(=|:)])...]\n \nThe following features are implemented:\n\n - the result is printed on stdout\n - the [[=]>] arguments are interpreted\n - a commit message read from stdin is interpreted\n - the \"trailer..key\" options in the config are interpreted\n - the \"trailer..where\" options are interpreted\n - the \"trailer..ifExist\" options are interpreted\n - the \"trailer..ifMissing\" options are interpreted\n - the \"trailer..command\" config works\n - $ARG can be used in commands\n - there are some tests\n - there is some documentation\n\nThe following features are planned but not yet implemented:\n - add more tests related to commands\n - add examples in documentation\n - integration with \"git commit\"\n\nPossible improvements:\n - support GIT_COMMIT_PROTO env variable in commands\n\n3) Changes since version 7, thanks to Junio:\n\n* improved handling of empty trailer token\n* clearer way to create 'expected' files in tests\n* other small test cleanups\n* improved commit message\n* new way to parse config keys\n* strcasecmp() is not used anymore in some config related functions\nSome values from the config file are lowercased instead.\nTo enable that a new patch (3/12) is introduced to rationalize\nlowercase related functions. I am not very happy with these\nchanges.\n\n\nChristian Couder (12):\n Add data structures and basic functions for commit trailers\n trailer: process trailers from stdin and arguments\n Move lower case functions into wrapper.c\n trailer: read and process config information\n trailer: process command line trailer arguments\n trailer: parse trailers from stdin\n trailer: put all the processing together and print\n trailer: add interpret-trailers command\n trailer: add tests for \"git interpret-trailers\"\n trailer: execute command from 'trailer..command'\n trailer: add tests for commands in config file\n Documentation: add documentation for 'git interpret-trailers'\n\n .gitignore | 1 +\n Documentation/git-interpret-trailers.txt | 123 ++++++\n Makefile | 2 +\n builtin.h | 1 +\n builtin/interpret-trailers.c | 33 ++\n config.c | 6 -\n daemon.c | 8 -\n git-compat-util.h | 4 +\n git.c | 1 +\n t/t7513-interpret-trailers.sh | 407 ++++++++++++++++++\n trailer.c | 691 +++++++++++++++++++++++++++++++\n trailer.h | 6 +\n wrapper.c | 14 +\n 13 files changed, 1283 insertions(+), 14 deletions(-)\n create mode 100644 Documentation/git-interpret-trailers.txt\n create mode 100644 builtin/interpret-trailers.c\n create mode 100755 t/t7513-interpret-trailers.sh\n create mode 100644 trailer.c\n create mode 100644 trailer.h\n\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237868","messageId":"20140326221531.11352.50524.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 01/12] Add data structures and basic functions for commit trailers","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:19Z","receivedAt":"2014-03-26T22:15:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"We will use a doubly linked list to store all information\nabout trailers and their configuration.\n\nThis way we can easily remove or add trailers to or from\ntrailer lists while traversing the lists in either direction.\n\nSigned-off-by: Christian Couder \n---\n Makefile | 1 +\n trailer.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 50 insertions(+)\n create mode 100644 trailer.c\n\ndiff --git a/Makefile b/Makefile\nindex d4ce53a..ec67ae1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -883,6 +883,7 @@ LIB_OBJS += submodule.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += trace.o\n+LIB_OBJS += trailer.o\n LIB_OBJS += transport.o\n LIB_OBJS += transport-helper.o\n LIB_OBJS += tree-diff.o\ndiff --git a/trailer.c b/trailer.c\nnew file mode 100644\nindex 0000000..db93a63\n--- /dev/null\n+++ b/trailer.c\n@@ -0,0 +1,49 @@\n+#include \"cache.h\"\n+/*\n+ * Copyright (c) 2013, 2014 Christian Couder \n+ */\n+\n+enum action_where { WHERE_AFTER, WHERE_BEFORE };\n+enum action_if_exists { EXISTS_ADD_IF_DIFFERENT, EXISTS_ADD_IF_DIFFERENT_NEIGHBOR,\n+\t\t\tEXISTS_ADD, EXISTS_OVERWRITE, EXISTS_DO_NOTHING };\n+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };\n+\n+struct conf_info {\n+\tchar *name;\n+\tchar *key;\n+\tchar *command;\n+\tenum action_where where;\n+\tenum action_if_exists if_exists;\n+\tenum action_if_missing if_missing;\n+};\n+\n+struct trailer_item {\n+\tstruct trailer_item *previous;\n+\tstruct trailer_item *next;\n+\tconst char *token;\n+\tconst char *value;\n+\tstruct conf_info conf;\n+};\n+\n+static int same_token(struct trailer_item *a, struct trailer_item *b, int alnum_len)\n+{\n+\treturn !strncasecmp(a->token, b->token, alnum_len);\n+}\n+\n+static int same_value(struct trailer_item *a, struct trailer_item *b)\n+{\n+\treturn !strcasecmp(a->value, b->value);\n+}\n+\n+static int same_trailer(struct trailer_item *a, struct trailer_item *b, int alnum_len)\n+{\n+\treturn same_token(a, b, alnum_len) && same_value(a, b);\n+}\n+\n+/* Get the length of buf from its beginning until its last alphanumeric character */\n+static size_t alnum_len(const char *buf, size_t len)\n+{\n+\twhile (len > 0 && !isalnum(buf[len - 1]))\n+\t\tlen--;\n+\treturn len;\n+}\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237865","messageId":"20140326221531.11352.46041.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 02/12] trailer: process trailers from stdin and arguments","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:20Z","receivedAt":"2014-03-26T22:15:20Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Implement the logic to process trailers from stdin and arguments.\n\nAt the beginning trailers from stdin are in their own in_tok\ndoubly linked list, and trailers from arguments are in their own\narg_tok doubly linked list.\n\nThe lists are traversed and when an arg_tok should be \"applied\",\nit is removed from its list and inserted into the in_tok list.\n\nSigned-off-by: Christian Couder \n---\n trailer.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 198 insertions(+)\n\ndiff --git a/trailer.c b/trailer.c\nindex db93a63..52108c2 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -47,3 +47,201 @@ static size_t alnum_len(const char *buf, size_t len)\n \t\tlen--;\n \treturn len;\n }\n+\n+static void free_trailer_item(struct trailer_item *item)\n+{\n+\tfree(item->conf.name);\n+\tfree(item->conf.key);\n+\tfree(item->conf.command);\n+\tfree((char *)item->token);\n+\tfree((char *)item->value);\n+\tfree(item);\n+}\n+\n+static void add_arg_to_input_list(struct trailer_item *in_tok,\n+\t\t\t\t struct trailer_item *arg_tok)\n+{\n+\tif (arg_tok->conf.where == WHERE_AFTER) {\n+\t\targ_tok->next = in_tok->next;\n+\t\tin_tok->next = arg_tok;\n+\t\targ_tok->previous = in_tok;\n+\t\tif (arg_tok->next)\n+\t\t\targ_tok->next->previous = arg_tok;\n+\t} else {\n+\t\targ_tok->previous = in_tok->previous;\n+\t\tin_tok->previous = arg_tok;\n+\t\targ_tok->next = in_tok;\n+\t\tif (arg_tok->previous)\n+\t\t\targ_tok->previous->next = arg_tok;\n+\t}\n+}\n+\n+static int check_if_different(struct trailer_item *in_tok,\n+\t\t\t struct trailer_item *arg_tok,\n+\t\t\t int alnum_len, int check_all)\n+{\n+\tenum action_where where = arg_tok->conf.where;\n+\tdo {\n+\t\tif (!in_tok)\n+\t\t\treturn 1;\n+\t\tif (same_trailer(in_tok, arg_tok, alnum_len))\n+\t\t\treturn 0;\n+\t\t/*\n+\t\t * if we want to add a trailer after another one,\n+\t\t * we have to check those before this one\n+\t\t */\n+\t\tin_tok = (where == WHERE_AFTER) ? in_tok->previous : in_tok->next;\n+\t} while (check_all);\n+\treturn 1;\n+}\n+\n+static void apply_arg_if_exists(struct trailer_item *in_tok,\n+\t\t\t\tstruct trailer_item *arg_tok,\n+\t\t\t\tint alnum_len)\n+{\n+\tswitch (arg_tok->conf.if_exists) {\n+\tcase EXISTS_DO_NOTHING:\n+\t\tfree_trailer_item(arg_tok);\n+\t\tbreak;\n+\tcase EXISTS_OVERWRITE:\n+\t\tfree((char *)in_tok->value);\n+\t\tin_tok->value = xstrdup(arg_tok->value);\n+\t\tfree_trailer_item(arg_tok);\n+\t\tbreak;\n+\tcase EXISTS_ADD:\n+\t\tadd_arg_to_input_list(in_tok, arg_tok);\n+\t\tbreak;\n+\tcase EXISTS_ADD_IF_DIFFERENT:\n+\t\tif (check_if_different(in_tok, arg_tok, alnum_len, 1))\n+\t\t\tadd_arg_to_input_list(in_tok, arg_tok);\n+\t\telse\n+\t\t\tfree_trailer_item(arg_tok);\n+\t\tbreak;\n+\tcase EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:\n+\t\tif (check_if_different(in_tok, arg_tok, alnum_len, 0))\n+\t\t\tadd_arg_to_input_list(in_tok, arg_tok);\n+\t\telse\n+\t\t\tfree_trailer_item(arg_tok);\n+\t\tbreak;\n+\t}\n+}\n+\n+static void remove_from_list(struct trailer_item *item,\n+\t\t\t struct trailer_item **first)\n+{\n+\tif (item->next)\n+\t\titem->next->previous = item->previous;\n+\tif (item->previous)\n+\t\titem->previous->next = item->next;\n+\telse\n+\t\t*first = item->next;\n+}\n+\n+static struct trailer_item *remove_first(struct trailer_item **first)\n+{\n+\tstruct trailer_item *item = *first;\n+\t*first = item->next;\n+\tif (item->next) {\n+\t\titem->next->previous = NULL;\n+\t\titem->next = NULL;\n+\t}\n+\treturn item;\n+}\n+\n+static void process_input_token(struct trailer_item *in_tok,\n+\t\t\t\tstruct trailer_item **arg_tok_first,\n+\t\t\t\tenum action_where where)\n+{\n+\tstruct trailer_item *arg_tok;\n+\tstruct trailer_item *next_arg;\n+\n+\tint after = where == WHERE_AFTER;\n+\tint tok_alnum_len = alnum_len(in_tok->token, strlen(in_tok->token));\n+\n+\tfor (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {\n+\t\tnext_arg = arg_tok->next;\n+\t\tif (!same_token(in_tok, arg_tok, tok_alnum_len))\n+\t\t\tcontinue;\n+\t\tif (arg_tok->conf.where != where)\n+\t\t\tcontinue;\n+\t\tremove_from_list(arg_tok, arg_tok_first);\n+\t\tapply_arg_if_exists(in_tok, arg_tok, tok_alnum_len);\n+\t\t/*\n+\t\t * If arg has been added to input,\n+\t\t * then we need to process it too now.\n+\t\t */\n+\t\tif ((after ? in_tok->next : in_tok->previous) == arg_tok)\n+\t\t\tin_tok = arg_tok;\n+\t}\n+}\n+\n+static void update_last(struct trailer_item **last)\n+{\n+\tif (*last)\n+\t\twhile ((*last)->next != NULL)\n+\t\t\t*last = (*last)->next;\n+}\n+\n+static void update_first(struct trailer_item **first)\n+{\n+\tif (*first)\n+\t\twhile ((*first)->previous != NULL)\n+\t\t\t*first = (*first)->previous;\n+}\n+\n+static void apply_arg_if_missing(struct trailer_item **in_tok_first,\n+\t\t\t\t struct trailer_item **in_tok_last,\n+\t\t\t\t struct trailer_item *arg_tok)\n+{\n+\tstruct trailer_item **in_tok;\n+\tenum action_where where;\n+\n+\tswitch (arg_tok->conf.if_missing) {\n+\tcase MISSING_DO_NOTHING:\n+\t\tfree_trailer_item(arg_tok);\n+\t\tbreak;\n+\tcase MISSING_ADD:\n+\t\twhere = arg_tok->conf.where;\n+\t\tin_tok = (where == WHERE_AFTER) ? in_tok_last : in_tok_first;\n+\t\tif (*in_tok) {\n+\t\t\tadd_arg_to_input_list(*in_tok, arg_tok);\n+\t\t\t*in_tok = arg_tok;\n+\t\t} else {\n+\t\t\t*in_tok_first = arg_tok;\n+\t\t\t*in_tok_last = arg_tok;\n+\t\t}\n+\t\tbreak;\n+\t}\n+}\n+\n+static void process_trailers_lists(struct trailer_item **in_tok_first,\n+\t\t\t\t struct trailer_item **in_tok_last,\n+\t\t\t\t struct trailer_item **arg_tok_first)\n+{\n+\tstruct trailer_item *in_tok;\n+\tstruct trailer_item *arg_tok;\n+\n+\tif (!*arg_tok_first)\n+\t\treturn;\n+\n+\t/* Process input from end to start */\n+\tfor (in_tok = *in_tok_last; in_tok; in_tok = in_tok->previous)\n+\t\tprocess_input_token(in_tok, arg_tok_first, WHERE_AFTER);\n+\n+\tupdate_last(in_tok_last);\n+\n+\tif (!*arg_tok_first)\n+\t\treturn;\n+\n+\t/* Process input from start to end */\n+\tfor (in_tok = *in_tok_first; in_tok; in_tok = in_tok->next)\n+\t\tprocess_input_token(in_tok, arg_tok_first, WHERE_BEFORE);\n+\n+\tupdate_first(in_tok_first);\n+\n+\t/* Process args left */\n+\twhile (*arg_tok_first) {\n+\t\targ_tok = remove_first(arg_tok_first);\n+\t\tapply_arg_if_missing(in_tok_first, in_tok_last, arg_tok);\n+\t}\n+}\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237856","messageId":"20140326221531.11352.86408.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:21Z","receivedAt":"2014-03-26T22:15:21Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The lowercase() function from config.c and the xstrdup_tolower()\nfunction from daemon.c can benefit from being moved to the same\nplace because this way the latter can use the former.\n\nAlso let's make them available globally so we can use them from\nother places like trailer.c.\n\nSigned-off-by: Christian Couder \n---\n config.c | 6 ------\n daemon.c | 8 --------\n git-compat-util.h | 4 ++++\n wrapper.c | 14 ++++++++++++++\n 4 files changed, 18 insertions(+), 14 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 314d8ee..dde128e 100644\n--- a/config.c\n+++ b/config.c\n@@ -146,12 +146,6 @@ int git_config_include(const char *var, const char *value, void *data)\n \treturn ret;\n }\n \n-static void lowercase(char *p)\n-{\n-\tfor (; *p; p++)\n-\t\t*p = tolower(*p);\n-}\n-\n void git_config_push_parameter(const char *text)\n {\n \tstruct strbuf env = STRBUF_INIT;\ndiff --git a/daemon.c b/daemon.c\nindex eba1255..f9c63e9 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -475,14 +475,6 @@ static void make_service_overridable(const char *name, int ena)\n \tdie(\"No such service %s\", name);\n }\n \n-static char *xstrdup_tolower(const char *str)\n-{\n-\tchar *p, *dup = xstrdup(str);\n-\tfor (p = dup; *p; p++)\n-\t\t*p = tolower(*p);\n-\treturn dup;\n-}\n-\n static void parse_host_and_port(char *hostport, char **host,\n \tchar **port)\n {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 614a5e9..2397706 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -727,4 +727,8 @@ void warn_on_inaccessible(const char *path);\n /* Get the passwd entry for the UID of the current process. */\n struct passwd *xgetpwuid_self(void);\n \n+/* Lowercase strings */\n+extern void lowercase(char *str);\n+extern char *xstrdup_tolower(const char *str);\n+\n #endif\ndiff --git a/wrapper.c b/wrapper.c\nindex 0cc5636..c46026a 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -455,3 +455,17 @@ struct passwd *xgetpwuid_self(void)\n \t\t errno ? strerror(errno) : _(\"no such user\"));\n \treturn pw;\n }\n+\n+void lowercase(char *p)\n+{\n+\tfor (; *p; p++)\n+\t\t*p = tolower(*p);\n+}\n+\n+char *xstrdup_tolower(const char *str)\n+{\n+\tchar *dup = xstrdup(str);\n+\tlowercase(dup);\n+\treturn dup;\n+}\n+\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237867","messageId":"20140326221531.11352.31137.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 04/12] trailer: read and process config information","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:22Z","receivedAt":"2014-03-26T22:15:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Read the configuration to get trailer information, and then process\nit and storing it in a doubly linked list.\n\nThe config information is stored in the list whose first item is\npointed to by:\n\nstatic struct trailer_item *first_conf_item;\n\nSigned-off-by: Christian Couder \n---\n trailer.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 153 insertions(+)\n\ndiff --git a/trailer.c b/trailer.c\nindex 52108c2..b5a0943 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -25,6 +25,8 @@ struct trailer_item {\n \tstruct conf_info conf;\n };\n \n+static struct trailer_item *first_conf_item;\n+\n static int same_token(struct trailer_item *a, struct trailer_item *b, int alnum_len)\n {\n \treturn !strncasecmp(a->token, b->token, alnum_len);\n@@ -245,3 +247,154 @@ static void process_trailers_lists(struct trailer_item **in_tok_first,\n \t\tapply_arg_if_missing(in_tok_first, in_tok_last, arg_tok);\n \t}\n }\n+\n+static int set_where(struct conf_info *item, const char *value)\n+{\n+\tif (!strcmp(\"after\", value))\n+\t\titem->where = WHERE_AFTER;\n+\telse if (!strcmp(\"before\", value))\n+\t\titem->where = WHERE_BEFORE;\n+\telse\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static int set_if_exists(struct conf_info *item, const char *value)\n+{\n+\tif (!strcmp(\"addifdifferent\", value))\n+\t\titem->if_exists = EXISTS_ADD_IF_DIFFERENT;\n+\telse if (!strcmp(\"addifdifferentneighbor\", value))\n+\t\titem->if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;\n+\telse if (!strcmp(\"add\", value))\n+\t\titem->if_exists = EXISTS_ADD;\n+\telse if (!strcmp(\"overwrite\", value))\n+\t\titem->if_exists = EXISTS_OVERWRITE;\n+\telse if (!strcmp(\"donothing\", value))\n+\t\titem->if_exists = EXISTS_DO_NOTHING;\n+\telse\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static int set_if_missing(struct conf_info *item, const char *value)\n+{\n+\tif (!strcmp(\"donothing\", value))\n+\t\titem->if_missing = MISSING_DO_NOTHING;\n+\telse if (!strcmp(\"add\", value))\n+\t\titem->if_missing = MISSING_ADD;\n+\telse\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static struct trailer_item *get_conf_item(const char *name)\n+{\n+\tstruct trailer_item *item;\n+\tstruct trailer_item *previous;\n+\n+\t/* Look up item with same name */\n+\tfor (previous = NULL, item = first_conf_item;\n+\t item;\n+\t previous = item, item = item->next) {\n+\t\tif (!strcasecmp(item->conf.name, name))\n+\t\t\treturn item;\n+\t}\n+\n+\t/* Item does not already exists, create it */\n+\titem = xcalloc(sizeof(struct trailer_item), 1);\n+\titem->conf.name = xstrdup(name);\n+\n+\tif (!previous)\n+\t\tfirst_conf_item = item;\n+\telse {\n+\t\tprevious->next = item;\n+\t\titem->previous = previous;\n+\t}\n+\n+\treturn item;\n+}\n+\n+enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,\n+\t\t\t TRAILER_IF_EXISTS, TRAILER_IF_MISSING };\n+\n+static struct {\n+\tconst char *name;\n+\tenum trailer_info_type type;\n+} trailer_config_items[] = {\n+\t{ \"key\", TRAILER_KEY },\n+\t{ \"command\", TRAILER_COMMAND },\n+\t{ \"where\", TRAILER_WHERE },\n+\t{ \"ifexists\", TRAILER_IF_EXISTS },\n+\t{ \"ifmissing\", TRAILER_IF_MISSING }\n+};\n+\n+static int git_trailer_config(const char *conf_key, const char *value, void *cb)\n+{\n+\tconst char *trailer_item, *variable_name;\n+\tstruct trailer_item *item;\n+\tstruct conf_info *conf;\n+\tchar *name = NULL;\n+\tenum trailer_info_type type;\n+\tchar *lowercase_value;\n+\tint i;\n+\n+\ttrailer_item = skip_prefix(conf_key, \"trailer.\");\n+\tif (!trailer_item)\n+\t\treturn 0;\n+\n+\tvariable_name = strrchr(trailer_item, '.');\n+\tif (!variable_name) {\n+\t\twarning(_(\"two level trailer config variable %s\"), conf_key);\n+\t\treturn 0;\n+\t}\n+\n+\tvariable_name++;\n+\tfor (i = 0; i < ARRAY_SIZE(trailer_config_items); i++) {\n+\t\tif (strcmp(trailer_config_items[i].name, variable_name))\n+\t\t\tcontinue;\n+\t\tname = xstrndup(trailer_item, variable_name - trailer_item -1);\n+\t\ttype = trailer_config_items[i].type;\n+\t\tbreak;\n+\t}\n+\n+\tif (!name)\n+\t\treturn 0;\n+\n+\titem = get_conf_item(name);\n+\tconf = &item->conf;\n+\tfree(name);\n+\n+\tswitch (type) {\n+\tcase TRAILER_KEY:\n+\t\tif (conf->key)\n+\t\t\twarning(_(\"more than one %s\"), conf_key);\n+\t\tconf->key = xstrdup(value);\n+\t\tbreak;\n+\tcase TRAILER_COMMAND:\n+\t\tif (conf->command)\n+\t\t\twarning(_(\"more than one %s\"), conf_key);\n+\t\tconf->command = xstrdup(value);\n+\t\tbreak;\n+\tcase TRAILER_WHERE:\n+\t\tlowercase_value = xstrdup_tolower(value);\n+\t\tif (set_where(conf, lowercase_value))\n+\t\t\twarning(_(\"unknown value '%s' for key '%s'\"), value, conf_key);\n+\t\tfree(lowercase_value);\n+\t\tbreak;\n+\tcase TRAILER_IF_EXISTS:\n+\t\tlowercase_value = xstrdup_tolower(value);\n+\t\tif (set_if_exists(conf, lowercase_value))\n+\t\t\twarning(_(\"unknown value '%s' for key '%s'\"), value, conf_key);\n+\t\tfree(lowercase_value);\n+\t\tbreak;\n+\tcase TRAILER_IF_MISSING:\n+\t\tlowercase_value = xstrdup_tolower(value);\n+\t\tif (set_if_missing(conf, lowercase_value))\n+\t\t\twarning(_(\"unknown value '%s' for key '%s'\"), value, conf_key);\n+\t\tfree(lowercase_value);\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"internal bug in trailer.c\");\n+\t}\n+\treturn 0;\n+}\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237864","messageId":"20140326221531.11352.1208.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 05/12] trailer: process command line trailer arguments","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:23Z","receivedAt":"2014-03-26T22:15:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Parse the trailer command line arguments and put\nthe result into an arg_tok doubly linked list.\n\nSigned-off-by: Christian Couder \n---\n trailer.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 103 insertions(+)\n\ndiff --git a/trailer.c b/trailer.c\nindex b5a0943..1871843 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -398,3 +398,106 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)\n \t}\n \treturn 0;\n }\n+\n+static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer)\n+{\n+\tsize_t len = strcspn(trailer, \"=:\");\n+\tif (len == 0)\n+\t\treturn error(_(\"empty trailer token in trailer '%s'\"), trailer);\n+\tif (len < strlen(trailer)) {\n+\t\tstrbuf_add(tok, trailer, len);\n+\t\tstrbuf_trim(tok);\n+\t\tstrbuf_addstr(val, trailer + len + 1);\n+\t\tstrbuf_trim(val);\n+\t} else {\n+\t\tstrbuf_addstr(tok, trailer);\n+\t\tstrbuf_trim(tok);\n+\t}\n+\treturn 0;\n+}\n+\n+\n+static void duplicate_conf(struct conf_info *dst, struct conf_info *src)\n+{\n+\t*dst = *src;\n+\tif (src->name)\n+\t\tdst->name = xstrdup(src->name);\n+\tif (src->key)\n+\t\tdst->key = xstrdup(src->key);\n+\tif (src->command)\n+\t\tdst->command = xstrdup(src->command);\n+}\n+\n+static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,\n+\t\t\t\t\t char *tok, char *val)\n+{\n+\tstruct trailer_item *new = xcalloc(sizeof(*new), 1);\n+\tnew->value = val;\n+\n+\tif (conf_item) {\n+\t\tduplicate_conf(&new->conf, &conf_item->conf);\n+\t\tnew->token = xstrdup(conf_item->conf.key);\n+\t\tfree(tok);\n+\t} else\n+\t\tnew->token = tok;\n+\n+\treturn new;\n+}\n+\n+static struct trailer_item *create_trailer_item(const char *string)\n+{\n+\tstruct strbuf tok = STRBUF_INIT;\n+\tstruct strbuf val = STRBUF_INIT;\n+\tstruct trailer_item *item;\n+\tint tok_alnum_len;\n+\n+\tif (parse_trailer(&tok, &val, string))\n+\t\treturn NULL;\n+\n+\ttok_alnum_len = alnum_len(tok.buf, tok.len);\n+\n+\t/* Lookup if the token matches something in the config */\n+\tfor (item = first_conf_item; item; item = item->next) {\n+\t\tif (!strncasecmp(tok.buf, item->conf.key, tok_alnum_len) ||\n+\t\t !strncasecmp(tok.buf, item->conf.name, tok_alnum_len)) {\n+\t\t\tstrbuf_release(&tok);\n+\t\t\treturn new_trailer_item(item,\n+\t\t\t\t\t\tNULL,\n+\t\t\t\t\t\tstrbuf_detach(&val, NULL));\n+\t\t}\n+\t}\n+\n+\treturn new_trailer_item(NULL,\n+\t\t\t\tstrbuf_detach(&tok, NULL),\n+\t\t\t\tstrbuf_detach(&val, NULL));\n+}\n+\n+static void add_trailer_item(struct trailer_item **first,\n+\t\t\t struct trailer_item **last,\n+\t\t\t struct trailer_item *new)\n+{\n+\tif (!new)\n+\t\treturn;\n+\tif (!*last) {\n+\t\t*first = new;\n+\t\t*last = new;\n+\t} else {\n+\t\t(*last)->next = new;\n+\t\tnew->previous = *last;\n+\t\t*last = new;\n+\t}\n+}\n+\n+static struct trailer_item *process_command_line_args(int argc, const char **argv)\n+{\n+\tint i;\n+\tstruct trailer_item *arg_tok_first = NULL;\n+\tstruct trailer_item *arg_tok_last = NULL;\n+\n+\tfor (i = 0; i < argc; i++) {\n+\t\tstruct trailer_item *new = create_trailer_item(argv[i]);\n+\t\tadd_trailer_item(&arg_tok_first, &arg_tok_last, new);\n+\t}\n+\n+\treturn arg_tok_first;\n+}\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237860","messageId":"20140326221531.11352.99065.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 06/12] trailer: parse trailers from stdin","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:24Z","receivedAt":"2014-03-26T22:15:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Read trailers from stdin, parse them and put the result into a doubly linked\nlist.\n\nSigned-off-by: Christian Couder \n---\n trailer.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 76 insertions(+)\n\ndiff --git a/trailer.c b/trailer.c\nindex 1871843..3993840 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -50,6 +50,14 @@ static size_t alnum_len(const char *buf, size_t len)\n \treturn len;\n }\n \n+static inline int contains_only_spaces(const char *str)\n+{\n+\tconst char *s = str;\n+\twhile (*s && isspace(*s))\n+\t\ts++;\n+\treturn !*s;\n+}\n+\n static void free_trailer_item(struct trailer_item *item)\n {\n \tfree(item->conf.name);\n@@ -501,3 +509,71 @@ static struct trailer_item *process_command_line_args(int argc, const char **arg\n \n \treturn arg_tok_first;\n }\n+\n+static struct strbuf **read_stdin(void)\n+{\n+\tstruct strbuf **lines;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (strbuf_read(&sb, fileno(stdin), 0) < 0)\n+\t\tdie_errno(_(\"could not read from stdin\"));\n+\n+\tlines = strbuf_split(&sb, '\\n');\n+\n+\tstrbuf_release(&sb);\n+\n+\treturn lines;\n+}\n+\n+/*\n+ * Return the the (0 based) index of the first trailer line\n+ * or the line count if there are no trailers.\n+ */\n+static int find_trailer_start(struct strbuf **lines)\n+{\n+\tint start, empty = 1, count = 0;\n+\n+\t/* Get the line count */\n+\twhile (lines[count])\n+\t\tcount++;\n+\n+\t/*\n+\t * Get the start of the trailers by looking starting from the end\n+\t * for a line with only spaces before lines with one ':'.\n+\t */\n+\tfor (start = count - 1; start >= 0; start--) {\n+\t\tif (contains_only_spaces(lines[start]->buf)) {\n+\t\t\tif (empty)\n+\t\t\t\tcontinue;\n+\t\t\treturn start + 1;\n+\t\t}\n+\t\tif (strchr(lines[start]->buf, ':')) {\n+\t\t\tif (empty)\n+\t\t\t\tempty = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\treturn count;\n+\t}\n+\n+\treturn empty ? count : start + 1;\n+}\n+\n+static void process_stdin(struct trailer_item **in_tok_first,\n+\t\t\t struct trailer_item **in_tok_last)\n+{\n+\tstruct strbuf **lines = read_stdin();\n+\tint start = find_trailer_start(lines);\n+\tint i;\n+\n+\t/* Print non trailer lines as is */\n+\tfor (i = 0; lines[i] && i < start; i++)\n+\t\tprintf(\"%s\", lines[i]->buf);\n+\n+\t/* Parse trailer lines */\n+\tfor (i = start; lines[i]; i++) {\n+\t\tstruct trailer_item *new = create_trailer_item(lines[i]->buf);\n+\t\tadd_trailer_item(in_tok_first, in_tok_last, new);\n+\t}\n+\n+\tstrbuf_list_free(lines);\n+}\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237861","messageId":"20140326221531.11352.11305.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 07/12] trailer: put all the processing together and print","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:25Z","receivedAt":"2014-03-26T22:15:25Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch adds the process_trailers() function that\ncalls all the previously added processing functions\nand then prints the results on the standard output.\n\nSigned-off-by: Christian Couder \n---\n trailer.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++\n trailer.h | 6 ++++++\n 2 files changed, 55 insertions(+)\n create mode 100644 trailer.h\n\ndiff --git a/trailer.c b/trailer.c\nindex 3993840..5bb29ae 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"trailer.h\"\n /*\n * Copyright (c) 2013, 2014 Christian Couder \n */\n@@ -68,6 +69,26 @@ static void free_trailer_item(struct trailer_item *item)\n \tfree(item);\n }\n \n+static void print_tok_val(const char *tok, const char *val)\n+{\n+\tchar c = tok[strlen(tok) - 1];\n+\tif (isalnum(c))\n+\t\tprintf(\"%s: %s\\n\", tok, val);\n+\telse if (isspace(c) || c == '#')\n+\t\tprintf(\"%s%s\\n\", tok, val);\n+\telse\n+\t\tprintf(\"%s %s\\n\", tok, val);\n+}\n+\n+static void print_all(struct trailer_item *first, int trim_empty)\n+{\n+\tstruct trailer_item *item;\n+\tfor (item = first; item; item = item->next) {\n+\t\tif (!trim_empty || strlen(item->value) > 0)\n+\t\t\tprint_tok_val(item->token, item->value);\n+\t}\n+}\n+\n static void add_arg_to_input_list(struct trailer_item *in_tok,\n \t\t\t\t struct trailer_item *arg_tok)\n {\n@@ -577,3 +598,31 @@ static void process_stdin(struct trailer_item **in_tok_first,\n \n \tstrbuf_list_free(lines);\n }\n+\n+static void free_all(struct trailer_item **first)\n+{\n+\twhile (*first) {\n+\t\tstruct trailer_item *item = remove_first(first);\n+\t\tfree_trailer_item(item);\n+\t}\n+}\n+\n+void process_trailers(int trim_empty, int argc, const char **argv)\n+{\n+\tstruct trailer_item *in_tok_first = NULL;\n+\tstruct trailer_item *in_tok_last = NULL;\n+\tstruct trailer_item *arg_tok_first;\n+\n+\tgit_config(git_trailer_config, NULL);\n+\n+\t/* Print the non trailer part of stdin */\n+\tprocess_stdin(&in_tok_first, &in_tok_last);\n+\n+\targ_tok_first = process_command_line_args(argc, argv);\n+\n+\tprocess_trailers_lists(&in_tok_first, &in_tok_last, &arg_tok_first);\n+\n+\tprint_all(in_tok_first, trim_empty);\n+\n+\tfree_all(&in_tok_first);\n+}\ndiff --git a/trailer.h b/trailer.h\nnew file mode 100644\nindex 0000000..9323b1e\n--- /dev/null\n+++ b/trailer.h\n@@ -0,0 +1,6 @@\n+#ifndef TRAILER_H\n+#define TRAILER_H\n+\n+void process_trailers(int trim_empty, int argc, const char **argv);\n+\n+#endif /* TRAILER_H */\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237859","messageId":"20140326221531.11352.36369.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 08/12] trailer: add interpret-trailers command","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:26Z","receivedAt":"2014-03-26T22:15:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch adds the \"git interpret-trailers\" command.\nThis command uses the previously added process_trailers()\nfunction in trailer.c.\n\nSigned-off-by: Christian Couder \n---\n .gitignore | 1 +\n Makefile | 1 +\n builtin.h | 1 +\n builtin/interpret-trailers.c | 33 +++++++++++++++++++++++++++++++++\n git.c | 1 +\n 5 files changed, 37 insertions(+)\n create mode 100644 builtin/interpret-trailers.c\n\ndiff --git a/.gitignore b/.gitignore\nindex dc600f9..c2a0b19 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -74,6 +74,7 @@\n /git-index-pack\n /git-init\n /git-init-db\n+/git-interpret-trailers\n /git-instaweb\n /git-log\n /git-ls-files\ndiff --git a/Makefile b/Makefile\nindex ec67ae1..94f7e95 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -947,6 +947,7 @@ BUILTIN_OBJS += builtin/hash-object.o\n BUILTIN_OBJS += builtin/help.o\n BUILTIN_OBJS += builtin/index-pack.o\n BUILTIN_OBJS += builtin/init-db.o\n+BUILTIN_OBJS += builtin/interpret-trailers.o\n BUILTIN_OBJS += builtin/log.o\n BUILTIN_OBJS += builtin/ls-files.o\n BUILTIN_OBJS += builtin/ls-remote.o\ndiff --git a/builtin.h b/builtin.h\nindex c47c110..8ca0065 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -73,6 +73,7 @@ extern int cmd_hash_object(int argc, const char **argv, const char *prefix);\n extern int cmd_help(int argc, const char **argv, const char *prefix);\n extern int cmd_index_pack(int argc, const char **argv, const char *prefix);\n extern int cmd_init_db(int argc, const char **argv, const char *prefix);\n+extern int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\n extern int cmd_log(int argc, const char **argv, const char *prefix);\n extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_ls_files(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nnew file mode 100644\nindex 0000000..0c8ca72\n--- /dev/null\n+++ b/builtin/interpret-trailers.c\n@@ -0,0 +1,33 @@\n+/*\n+ * Builtin \"git interpret-trailers\"\n+ *\n+ * Copyright (c) 2013, 2014 Christian Couder \n+ *\n+ */\n+\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"parse-options.h\"\n+#include \"trailer.h\"\n+\n+static const char * const git_interpret_trailers_usage[] = {\n+\tN_(\"git interpret-trailers [--trim-empty] [([(=|:)])...]\"),\n+\tNULL\n+};\n+\n+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n+{\n+\tint trim_empty = 0;\n+\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"trim-empty\", &trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t git_interpret_trailers_usage, 0);\n+\n+\tprocess_trailers(trim_empty, argc, argv);\n+\n+\treturn 0;\n+}\ndiff --git a/git.c b/git.c\nindex 7cf2953..63a03eb 100644\n--- a/git.c\n+++ b/git.c\n@@ -380,6 +380,7 @@ static struct cmd_struct commands[] = {\n \t{ \"index-pack\", cmd_index_pack, RUN_SETUP_GENTLY },\n \t{ \"init\", cmd_init_db },\n \t{ \"init-db\", cmd_init_db },\n+\t{ \"interpret-trailers\", cmd_interpret_trailers, RUN_SETUP },\n \t{ \"log\", cmd_log, RUN_SETUP },\n \t{ \"ls-files\", cmd_ls_files, RUN_SETUP },\n \t{ \"ls-remote\", cmd_ls_remote, RUN_SETUP_GENTLY },\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237857","messageId":"20140326221531.11352.93918.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 09/12] trailer: add tests for \"git interpret-trailers\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:27Z","receivedAt":"2014-03-26T22:15:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n t/t7513-interpret-trailers.sh | 336 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 336 insertions(+)\n create mode 100755 t/t7513-interpret-trailers.sh\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nnew file mode 100755\nindex 0000000..417a4f3\n--- /dev/null\n+++ b/t/t7513-interpret-trailers.sh\n@@ -0,0 +1,336 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2013 Christian Couder\n+#\n+\n+test_description='git interpret-trailers'\n+\n+. ./test-lib.sh\n+\n+# When we want one trailing space at the end of each line, let's use sed\n+# to make sure that these spaces are not removed by any automatic tool.\n+\n+test_expect_success 'setup' '\n+\tcat >basic_message <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\tEOF\n+\tcat >complex_message_body <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body which is long\n+\t\tand contains some special\n+\t\tchars like : = ? !\n+\n+\tEOF\n+\tsed -e \"s/ Z\\$/ /\" >complex_message_trailers <<-\\EOF\n+\t\tFixes: Z\n+\t\tAcked-by: Z\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+'\n+\n+test_expect_success 'without config' '\n+\tsed -e \"s/ Z\\$/ /\" >expected <<-\\EOF &&\n+\t\tack: Peff\n+\t\tReviewed-by: Z\n+\t\tAcked-by: Johan\n+\tEOF\n+\tgit interpret-trailers \"ack = Peff\" \"Reviewed-by\" \"Acked-by: Johan\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--trim-empty without config' '\n+\tcat >expected <<-\\EOF &&\n+\t\tack: Peff\n+\t\tAcked-by: Johan\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"ack = Peff\" \"Reviewed-by\" \"Acked-by: Johan\" \"sob:\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with config setup' '\n+\tgit config trailer.ack.key \"Acked-by: \" &&\n+\tcat >expected <<-\\EOF &&\n+\t\tAcked-by: Peff\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"ack = Peff\" >actual &&\n+\ttest_cmp expected actual &&\n+\tgit interpret-trailers --trim-empty \"Acked-by = Peff\" >actual &&\n+\ttest_cmp expected actual &&\n+\tgit interpret-trailers --trim-empty \"Acked-by :Peff\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with config setup and = sign' '\n+\tgit config trailer.ack.key \"Acked-by= \" &&\n+\tcat >expected <<-\\EOF &&\n+\t\tAcked-by= Peff\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"ack = Peff\" >actual &&\n+\ttest_cmp expected actual &&\n+\tgit interpret-trailers --trim-empty \"Acked-by= Peff\" >actual &&\n+\ttest_cmp expected actual &&\n+\tgit interpret-trailers --trim-empty \"Acked-by : Peff\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with config setup and # sign' '\n+\tgit config trailer.bug.key \"Bug #\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\tBug #42\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"bug = 42\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with commit basic message' '\n+\tgit interpret-trailers actual &&\n+\ttest_cmp basic_message actual\n+'\n+\n+test_expect_success 'with commit complex message' '\n+\tcat complex_message_body complex_message_trailers >complex_message &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with commit complex message and args' '\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\t\tBug #42\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"bug: 42\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with commit complex message, args and --trim-empty' '\n+\tcat complex_message_body >expected &&\n+\tcat >>expected <<-\\EOF &&\n+\t\tAcked-by= Peff\n+\t\tBug #42\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"ack: Peff\" \"bug: 42\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"where = before\"' '\n+\tgit config trailer.bug.where \"before\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"bug: 42\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"where = before\" for a token in the middle of the message' '\n+\tgit config trailer.review.key \"Reviewed-by:\" &&\n+\tgit config trailer.review.where \"before\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Johan\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"bug: 42\" \"review: Johan\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"where = before\" and --trim-empty' '\n+\tcat complex_message_body >expected &&\n+\tcat >>expected <<-\\EOF &&\n+\t\tBug #46\n+\t\tBug #42\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Johan\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"ack: Peff\" \"bug: 42\" \"review: Johan\" \"Bug: 46\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'the default is \"ifExists = addIfDifferent\"' '\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"review:\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifExists = addIfDifferent\"' '\n+\tgit config trailer.review.ifExists \"addIfDifferent\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"review:\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifExists = addIfDifferentNeighbor\"' '\n+\tgit config trailer.ack.ifExists \"addIfDifferentNeighbor\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"review:\" \"ack: Junio\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifExists = addIfDifferentNeighbor\" and --trim-empty' '\n+\tgit config trailer.ack.ifExists \"addIfDifferentNeighbor\" &&\n+\tcat complex_message_body >expected &&\n+\tcat >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tAcked-by= Peff\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\tEOF\n+\tgit interpret-trailers --trim-empty \"ack: Peff\" \"Acked-by= Peff\" \"review:\" \"ack: Junio\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifExists = add\"' '\n+\tgit config trailer.ack.ifExists \"add\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Peff\n+\t\tAcked-by= Peff\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"ack: Peff\" \"Acked-by= Peff\" \"review:\" \"ack: Junio\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifExists = overwrite\"' '\n+\tgit config trailer.fix.key \"Fixes:\" &&\n+\tgit config trailer.fix.ifExists \"overwrite\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: 22\n+\t\tAcked-by= Z\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=53\" \"ack: Junio\" \"fix=22\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifExists = doNothing\"' '\n+\tgit config trailer.fix.ifExists \"doNothing\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=53\" \"ack: Junio\" \"fix=22\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'the default is \"ifMissing = add\"' '\n+\tgit config trailer.cc.key \"Cc: \" &&\n+\tgit config trailer.cc.where \"before\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tCc: Linus\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=53\" \"cc=Linus\" \"ack: Junio\" \"fix=22\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifMissing = add\"' '\n+\tgit config trailer.cc.ifMissing \"add\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tCc: Linus\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=53\" \"ack: Junio\" \"fix=22\" \"bug: 42\" \"cc=Linus\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'using \"ifMissing = doNothing\"' '\n+\tgit config trailer.cc.ifMissing \"doNothing\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tBug #42\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tAcked-by= Junio\n+\t\tAcked-by= Peff\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=53\" \"cc=Linus\" \"ack: Junio\" \"fix=22\" \"bug: 42\" \"ack: Peff\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237863","messageId":"20140326221531.11352.20661.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 10/12] trailer: execute command from 'trailer..command'","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:28Z","receivedAt":"2014-03-26T22:15:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Let the user specify a command that will give on its standard output\nthe value to use for the specified trailer.\n\nSigned-off-by: Christian Couder \n---\n trailer.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 63 insertions(+)\n\ndiff --git a/trailer.c b/trailer.c\nindex 5bb29ae..eeb2f86 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"run-command.h\"\n #include \"trailer.h\"\n /*\n * Copyright (c) 2013, 2014 Christian Couder \n@@ -13,11 +14,14 @@ struct conf_info {\n \tchar *name;\n \tchar *key;\n \tchar *command;\n+\tunsigned command_uses_arg : 1;\n \tenum action_where where;\n \tenum action_if_exists if_exists;\n \tenum action_if_missing if_missing;\n };\n \n+#define TRAILER_ARG_STRING \"$ARG\"\n+\n struct trailer_item {\n \tstruct trailer_item *previous;\n \tstruct trailer_item *next;\n@@ -59,6 +63,13 @@ static inline int contains_only_spaces(const char *str)\n \treturn !*s;\n }\n \n+static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b)\n+{\n+\tconst char *ptr = strstr(sb->buf, a);\n+\tif (ptr)\n+\t\tstrbuf_splice(sb, ptr - sb->buf, strlen(a), b, strlen(b));\n+}\n+\n static void free_trailer_item(struct trailer_item *item)\n {\n \tfree(item->conf.name);\n@@ -403,6 +414,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)\n \t\tif (conf->command)\n \t\t\twarning(_(\"more than one %s\"), conf_key);\n \t\tconf->command = xstrdup(value);\n+\t\tconf->command_uses_arg = !!strstr(conf->command, TRAILER_ARG_STRING);\n \t\tbreak;\n \tcase TRAILER_WHERE:\n \t\tlowercase_value = xstrdup_tolower(value);\n@@ -445,6 +457,44 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra\n \treturn 0;\n }\n \n+static int read_from_command(struct child_process *cp, struct strbuf *buf)\n+{\n+\tif (run_command(cp))\n+\t\treturn error(\"running trailer command '%s' failed\", cp->argv[0]);\n+\tif (strbuf_read(buf, cp->out, 1024) < 1)\n+\t\treturn error(\"reading from trailer command '%s' failed\", cp->argv[0]);\n+\tstrbuf_trim(buf);\n+\treturn 0;\n+}\n+\n+static const char *apply_command(const char *command, const char *arg)\n+{\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct child_process cp;\n+\tconst char *argv[] = {NULL, NULL};\n+\tconst char *result = \"\";\n+\n+\tstrbuf_addstr(&cmd, command);\n+\tif (arg)\n+\t\tstrbuf_replace(&cmd, TRAILER_ARG_STRING, arg);\n+\n+\targv[0] = cmd.buf;\n+\tmemset(&cp, 0, sizeof(cp));\n+\tcp.argv = argv;\n+\tcp.env = local_repo_env;\n+\tcp.no_stdin = 1;\n+\tcp.out = -1;\n+\tcp.use_shell = 1;\n+\n+\tif (read_from_command(&cp, &buf))\n+\t\tstrbuf_release(&buf);\n+\telse\n+\t\tresult = strbuf_detach(&buf, NULL);\n+\n+\tstrbuf_release(&cmd);\n+\treturn result;\n+}\n \n static void duplicate_conf(struct conf_info *dst, struct conf_info *src)\n {\n@@ -467,6 +517,10 @@ static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,\n \t\tduplicate_conf(&new->conf, &conf_item->conf);\n \t\tnew->token = xstrdup(conf_item->conf.key);\n \t\tfree(tok);\n+\t\tif (conf_item->conf.command_uses_arg || !val) {\n+\t\t\tnew->value = apply_command(conf_item->conf.command, val);\n+\t\t\tfree(val);\n+\t\t}\n \t} else\n \t\tnew->token = tok;\n \n@@ -522,12 +576,21 @@ static struct trailer_item *process_command_line_args(int argc, const char **arg\n \tint i;\n \tstruct trailer_item *arg_tok_first = NULL;\n \tstruct trailer_item *arg_tok_last = NULL;\n+\tstruct trailer_item *item;\n \n \tfor (i = 0; i < argc; i++) {\n \t\tstruct trailer_item *new = create_trailer_item(argv[i]);\n \t\tadd_trailer_item(&arg_tok_first, &arg_tok_last, new);\n \t}\n \n+\t/* Add conf commands that don't use $ARG */\n+\tfor (item = first_conf_item; item; item = item->next) {\n+\t\tif (item->conf.command && !item->conf.command_uses_arg) {\n+\t\t\tstruct trailer_item *new = new_trailer_item(item, NULL, NULL);\n+\t\t\tadd_trailer_item(&arg_tok_first, &arg_tok_last, new);\n+\t\t}\n+\t}\n+\n \treturn arg_tok_first;\n }\n \n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237858","messageId":"20140326221531.11352.47898.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 11/12] trailer: add tests for commands in config file","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:29Z","receivedAt":"2014-03-26T22:15:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n t/t7513-interpret-trailers.sh | 71 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 71 insertions(+)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 417a4f3..0a1f3b6 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -333,4 +333,75 @@ test_expect_success 'using \"ifMissing = doNothing\"' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'with simple command' '\n+\tgit config trailer.sign.key \"Signed-off-by: \" &&\n+\tgit config trailer.sign.where \"after\" &&\n+\tgit config trailer.sign.ifExists \"addIfDifferentNeighbor\" &&\n+\tgit config trailer.sign.command \"echo \\\"A U Thor \\\"\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\t\tSigned-off-by: A U Thor \n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=22\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with command using commiter information' '\n+\tgit config trailer.sign.ifExists \"addIfDifferent\" &&\n+\tgit config trailer.sign.command \"echo \\\"\\$GIT_COMMITTER_NAME <\\$GIT_COMMITTER_EMAIL>\\\"\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\t\tSigned-off-by: C O Mitter \n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=22\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with command using author information' '\n+\tgit config trailer.sign.key \"Signed-off-by: \" &&\n+\tgit config trailer.sign.where \"after\" &&\n+\tgit config trailer.sign.ifExists \"addIfDifferentNeighbor\" &&\n+\tgit config trailer.sign.command \"echo \\\"\\$GIT_AUTHOR_NAME <\\$GIT_AUTHOR_EMAIL>\\\"\" &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\n+\t\tFixes: Z\n+\t\tAcked-by= Z\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\t\tSigned-off-by: A U Thor \n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=22\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'setup a commit' '\n+\techo \"Content of the first commit.\" > a.txt &&\n+\tgit add a.txt &&\n+\tgit commit -m \"Add file a.txt\"\n+'\n+\n+test_expect_success 'with command using $ARG' '\n+\tgit config trailer.fix.ifExists \"overwrite\" &&\n+\tgit config trailer.fix.command \"git log -1 --oneline --format=\\\"%h (%s)\\\" --abbrev-commit --abbrev=14 \\$ARG\" &&\n+\tFIXED=$(git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 HEAD) &&\n+\tcat complex_message_body >expected &&\n+\tsed -e \"s/ Z\\$/ /\" >>expected <<-EOF &&\n+\t\tFixes: $FIXED\n+\t\tAcked-by= Z\n+\t\tReviewed-by: Z\n+\t\tSigned-off-by: Z\n+\t\tSigned-off-by: A U Thor \n+\tEOF\n+\tgit interpret-trailers \"review:\" \"fix=HEAD\" actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237862","messageId":"20140326221531.11352.87454.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"[PATCH v8 12/12] Documentation: add documentation for 'git interpret-trailers'","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-26T22:15:30Z","receivedAt":"2014-03-26T22:15:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n Documentation/git-interpret-trailers.txt | 123 +++++++++++++++++++++++++++++++\n 1 file changed, 123 insertions(+)\n create mode 100644 Documentation/git-interpret-trailers.txt\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nnew file mode 100644\nindex 0000000..75ae386\n--- /dev/null\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -0,0 +1,123 @@\n+git-interpret-trailers(1)\n+=========================\n+\n+NAME\n+----\n+git-interpret-trailers - help add stuctured information into commit messages\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git interpret-trailers' [--trim-empty] [([(=|:)])...]\n+\n+DESCRIPTION\n+-----------\n+Help add RFC 822-like headers, called 'trailers', at the end of the\n+otherwise free-form part of a commit message.\n+\n+This command is a filter. It reads the standard input for a commit\n+message and applies the `token` arguments, if any, to this\n+message. The resulting message is emited on the standard output.\n+\n+Some configuration variables control the way the `token` arguments are\n+applied to the message and the way any existing trailer in the message\n+is changed. They also make it possible to automatically add some\n+trailers.\n+\n+By default, a 'token=value' or 'token:value' argument will be added\n+only if no trailer with the same (token, value) pair is already in the\n+message. The 'token' and 'value' parts will be trimmed to remove\n+starting and trailing whitespace, and the resulting trimmed 'token'\n+and 'value' will appear in the message like this:\n+\n+------------------------------------------------\n+token: value\n+------------------------------------------------\n+\n+By default, if there are already trailers with the same 'token', the\n+new trailer will appear just after the last trailer with the same\n+'token'. Otherwise it will appear at the end of the message.\n+\n+Note that 'trailers' do not follow and are not intended to follow many\n+rules that are in RFC 822. For example they do not follow the line\n+breaking rules, the encoding rules and probably many other rules.\n+\n+OPTIONS\n+-------\n+--trim-empty::\n+\tIf the 'value' part of any trailer contains only whitespace,\n+\tthe whole trailer will be removed from the resulting message.\n+\n+CONFIGURATION VARIABLES\n+-----------------------\n+\n+trailer..key::\n+\tThis 'key' will be used instead of 'token' in the\n+\ttrailer. After some alphanumeric characters, it can contain\n+\tsome non alphanumeric characters like ':', '=' or '#' that will\n+\tbe used instead of ':' to separate the token from the value in\n+\tthe trailer, though the default ':' is more standard.\n+\n+trailer..where::\n+\tThis can be either `after`, which is the default, or\n+\t`before`. If it is `before`, then a trailer with the specified\n+\ttoken, will appear before, instead of after, other trailers\n+\twith the same token, or otherwise at the beginning, instead of\n+\tat the end, of all the trailers.\n+\n+trailer..ifexist::\n+\tThis option makes it possible to choose what action will be\n+\tperformed when there is already at least one trailer with the\n+\tsame token in the message.\n++\n+The valid values for this option are: `addIfDifferent` (this is the\n+default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`.\n++\n+With `addIfDifferent`, a new trailer will be added only if no trailer\n+with the same (token, value) pair is already in the message.\n++\n+With `addIfDifferentNeighbor`, a new trailer will be added only if no\n+trailer with the same (token, value) pair is above or below the line\n+where the new trailer will be added.\n++\n+With `add`, a new trailer will be added, even if some trailers with\n+the same (token, value) pair are already in the message.\n++\n+With `overwrite`, the new trailer will overwrite an existing trailer\n+with the same token.\n++\n+With `doNothing`, nothing will be done, that is no new trailer will be\n+added if there is already one with the same token in the message.\n+\n+trailer..ifmissing::\n+\tThis option makes it possible to choose what action will be\n+\tperformed when there is not yet any trailer with the same\n+\ttoken in the message.\n++\n+The valid values for this option are: `add` (this is the default) and\n+`doNothing`.\n++\n+With `add`, a new trailer will be added.\n++\n+With `doNothing`, nothing will be done.\n+\n+trailer..command::\n+\tThis option can be used to specify a shell command that will\n+\tbe used to automatically add or modify a trailer with the\n+\tspecified 'token'.\n++\n+When this option is specified, it is like if a special 'token=value'\n+argument is added at the end of the command line, where 'value' will\n+be given by the standard output of the specified command.\n++\n+If the command contains the `$ARG` string, this string will be\n+replaced with the 'value' part of an existing trailer with the same\n+token, if any, before the command is launched.\n+\n+SEE ALSO\n+--------\n+linkgit:git-commit[1]\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\n-- \n1.9.0.164.g3aa33cd.dirty\n"},{"id":"237874","messageId":"xmqq4n2kh86k.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"20140326215858.11352.89243.chriscool@tuxfamily.org","subject":"Re: [PATCH v8 00/12] Add interpret-trailers builtin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-26T23:05:55Z","receivedAt":"2014-03-26T23:05:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Until now git commit has only supported the well known\n> \"Signed-off-by: \" trailer, that is used by many projects like\n> the Linux kernel and Git.\n>\n> It is better to implement features for these trailers first in a\n> new command rather than in builtin/commit.c, because this way the\n> prepare-commit-msg and commit-msg hooks can reuse this command.\n\nThe \"first\" is somewhat questionable.\n\nIt is better to keep builtin/commit.c uncontaminated by any more\nhard-wired logic, like what we have for the signed-off-by line. Any\nnew things can and should be doable in hooks, and this filter would\nhelp writing these hooks.\n\nAnd that is why the design goal of the filter is to make it at least\nas powerful as the built-in logic we have for signed-off-by lines;\nthat would allow us to later eject the hard-wired logic for\nsigned-off-by line from the main codepath, if/when we wanted to.\n\nAlternatively, we could build a library-ish API around this filter\ncode and replace the hard-wired logic for signed-off-by line with a\ncall into that API, if/when we wanted to, but that requires (in\naddition to the \"at least as powerful as the built-in logic\") that\nthe implementation of this stand-alone filter can be cleanly made\ninto a reusable library, so that is a bit higher bar to cross than\n\"everything can be doable with hooks\" alternative.\n\n> 3) Changes since version 7, thanks to Junio:\n>\n> * improved handling of empty trailer token\n> * clearer way to create 'expected' files in tests\n> * other small test cleanups\n> * improved commit message\n> * new way to parse config keys\n> * strcasecmp() is not used anymore in some config related functions\n\nIt is unclear which of the 12 patches are unchanged since the last\nround. Are reviewers expected to re-read all of them?\n\n> Some values from the config file are lowercased instead.\n> To enable that a new patch (3/12) is introduced to rationalize\n> lowercase related functions. I am not very happy with these\n> changes.\n\nI can see why you are not very happy. Perhaps it may make you\nhappier if you did not move lowercase() at all, did the\nxstrdup_tolower() in a cleaner and more efficient way, and only used\nthe latter in the code?\n"},{"id":"237875","messageId":"xmqqwqfgftl0.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"20140326221531.11352.50524.chriscool@tuxfamily.org","subject":"Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-26T23:06:35Z","receivedAt":"2014-03-26T23:06:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> Subject: Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers\n\nAs pointed out many times for GSoC microprojects students, limit the\nscope with \"area:\" prefix for the commit title, e.g.\n\n Subject: trailers: add data structures and basic functions\n\nPlease also refer to what has already been queued on 'pu' to avoid\nwasting review bandwidth and mark patches that are unchanged as such\n(but do send them to the list for review, so that people who haven't\nseen the previous round can also comment).\n\nAs far as I can tell, this is the same as 8d1c70e5 (trailers: add\ndata structures and basic functions, 2014-03-06), so I'll queue the\nremainder on top of that commit already on 'pu', which incidentally\nwill preserve the original author timestamp from the previous\nincarnation.\n"},{"id":"237876","messageId":"xmqqppl8ftjd.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"20140326221531.11352.86408.chriscool@tuxfamily.org","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-26T23:07:34Z","receivedAt":"2014-03-26T23:07:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> diff --git a/wrapper.c b/wrapper.c\n> index 0cc5636..c46026a 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -455,3 +455,17 @@ struct passwd *xgetpwuid_self(void)\n> \t\t errno ? strerror(errno) : _(\"no such user\"));\n> \treturn pw;\n> }\n> +\n> +void lowercase(char *p)\n> +{\n> +\tfor (; *p; p++)\n> +\t\t*p = tolower(*p);\n> +}\n> +\n> +char *xstrdup_tolower(const char *str)\n> +{\n> +\tchar *dup = xstrdup(str);\n> +\tlowercase(dup);\n> +\treturn dup;\n> +}\n> +\n\nAs a pure code-movement step, this may be OK, but I am not sure if\nboth of them want to be public functions in this shape.\n\nPerhaps\n\nchar *downcase_copy(const char *str)\n{\n\tchar *copy = xmalloc(strlen(str) + 1);\n int i;\n for (i = 0; str[i]; i++)\n \tcopy[i] = tolower(str[i]);\n\tcopy[i] = '\\0';\n return copy;\n}\n\nmay avoid having to copy things twice. Do you need the other\nfunction exposed?\n"},{"id":"237882","messageId":"20140327.084745.2069840957131666658.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"xmqqppl8ftjd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-27T07:47:45Z","receivedAt":"2014-03-27T07:47:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano \n>\n> Christian Couder writes:\n> \n>> diff --git a/wrapper.c b/wrapper.c\n>> index 0cc5636..c46026a 100644\n>> --- a/wrapper.c\n>> +++ b/wrapper.c\n>> @@ -455,3 +455,17 @@ struct passwd *xgetpwuid_self(void)\n>> \t\t errno ? strerror(errno) : _(\"no such user\"));\n>> \treturn pw;\n>> }\n>> +\n>> +void lowercase(char *p)\n>> +{\n>> +\tfor (; *p; p++)\n>> +\t\t*p = tolower(*p);\n>> +}\n>> +\n>> +char *xstrdup_tolower(const char *str)\n>> +{\n>> +\tchar *dup = xstrdup(str);\n>> +\tlowercase(dup);\n>> +\treturn dup;\n>> +}\n>> +\n> \n> As a pure code-movement step, this may be OK, but I am not sure if\n> both of them want to be public functions in this shape.\n> \n> Perhaps\n> \n> char *downcase_copy(const char *str)\n> {\n> \tchar *copy = xmalloc(strlen(str) + 1);\n> int i;\n> for (i = 0; str[i]; i++)\n> \tcopy[i] = tolower(str[i]);\n> \tcopy[i] = '\\0';\n> return copy;\n> }\n> \n> may avoid having to copy things twice.\n\nYeah, but it seems a bit wasteful to allocate memory for a new string,\nthen downcase it, then compare it with strcmp() and then free it,\ninstead of just using strcasecmp() on the original string.\n\n> Do you need the other\n> function exposed?\n\nNo, with the change you suggest, I don't.\n\nThanks,\nChristian.\n"},{"id":"237883","messageId":"20140327.085325.427007668629327512.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"xmqq4n2kh86k.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 00/12] Add interpret-trailers builtin","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-27T07:53:25Z","receivedAt":"2014-03-27T07:53:25Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano \n\n> Christian Couder writes:\n> \n>> Until now git commit has only supported the well known\n>> \"Signed-off-by: \" trailer, that is used by many projects like\n>> the Linux kernel and Git.\n>>\n>> It is better to implement features for these trailers first in a\n>> new command rather than in builtin/commit.c, because this way the\n>> prepare-commit-msg and commit-msg hooks can reuse this command.\n> \n> The \"first\" is somewhat questionable.\n> \n> It is better to keep builtin/commit.c uncontaminated by any more\n> hard-wired logic, like what we have for the signed-off-by line. Any\n> new things can and should be doable in hooks, and this filter would\n> help writing these hooks.\n> \n> And that is why the design goal of the filter is to make it at least\n> as powerful as the built-in logic we have for signed-off-by lines;\n> that would allow us to later eject the hard-wired logic for\n> signed-off-by line from the main codepath, if/when we wanted to.\n> \n> Alternatively, we could build a library-ish API around this filter\n> code and replace the hard-wired logic for signed-off-by line with a\n> call into that API, if/when we wanted to, but that requires (in\n> addition to the \"at least as powerful as the built-in logic\") that\n> the implementation of this stand-alone filter can be cleanly made\n> into a reusable library, so that is a bit higher bar to cross than\n> \"everything can be doable with hooks\" alternative.\n\nOk, I will try to improve this part of the Rationale section.\n\nThanks,\nChristian.\n"},{"id":"237884","messageId":"20140327.085508.680223484876168176.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"xmqqwqfgftl0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-27T07:55:08Z","receivedAt":"2014-03-27T07:55:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano \nSubject: Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers\nDate: Wed, 26 Mar 2014 16:06:35 -0700\n\n> Christian Couder writes:\n> \n>>> Subject: Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers\n> \n> As pointed out many times for GSoC microprojects students, limit the\n> scope with \"area:\" prefix for the commit title, e.g.\n> \n> Subject: trailers: add data structures and basic functions\n\nOk, I will fix that.\n \n> Please also refer to what has already been queued on 'pu' to avoid\n> wasting review bandwidth and mark patches that are unchanged as such\n> (but do send them to the list for review, so that people who haven't\n> seen the previous round can also comment).\n\nYeah, I forgot to do that for this version of the series, sorry.\n\n> As far as I can tell, this is the same as 8d1c70e5 (trailers: add\n> data structures and basic functions, 2014-03-06), so I'll queue the\n> remainder on top of that commit already on 'pu', which incidentally\n> will preserve the original author timestamp from the previous\n> incarnation.\n\nOk.\n\nThanks,\nChristian.\n"},{"id":"237912","messageId":"xmqq4n2jfva5.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"20140327.084745.2069840957131666658.chriscool@tuxfamily.org","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-27T16:42:10Z","receivedAt":"2014-03-27T16:42:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Yeah, but it seems a bit wasteful to allocate memory for a new string,\n> then downcase it, then compare it with strcmp() and then free it,\n> instead of just using strcasecmp() on the original string.\n\nI wasn't looking at the caller (and I haven't). I agree that, if\nyou have to compare case-insensitive user input against known set of\ntokens, using strcasecmp() would be saner than making a downcased\ncopy and the set of known tokens. I do not know however you want to\ncompare in a case-insensitive way in your application, though.\n"},{"id":"237948","messageId":"xmqqlhvvcmnj.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"xmqq4n2jfva5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-27T22:16:48Z","receivedAt":"2014-03-27T22:16:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Christian Couder writes:\n>\n>> Yeah, but it seems a bit wasteful to allocate memory for a new string,\n>> then downcase it, then compare it with strcmp() and then free it,\n>> instead of just using strcasecmp() on the original string.\n>\n> I wasn't looking at the caller (and I haven't). I agree that, if\n> you have to compare case-insensitive user input against known set of\n> tokens, using strcasecmp() would be saner than making a downcased\n> copy and the set of known tokens. I do not know however you want to\n> compare in a case-insensitive way in your application, though.\n\nIt appears that one place this \"lowercase\" is used is to allow\nrAnDOm casing in the configuration file, e.g.\n\n\t[trailer \"Signed-off-by\"]\n\t\twhere = AfTEr\n\nwhich I find is totally unnecessary. Do we churn code to accept\nsuch a nonsense input in other places?\n"},{"id":"237949","messageId":"20140327223406.GA32434@sigill.intra.peff.net","threadId":"36298","inReplyTo":"xmqqlhvvcmnj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-27T22:34:06Z","receivedAt":"2014-03-27T22:34:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 27, 2014 at 03:16:48PM -0700, Junio C Hamano wrote:\n\n> > I wasn't looking at the caller (and I haven't). I agree that, if\n> > you have to compare case-insensitive user input against known set of\n> > tokens, using strcasecmp() would be saner than making a downcased\n> > copy and the set of known tokens. I do not know however you want to\n> > compare in a case-insensitive way in your application, though.\n> \n> It appears that one place this \"lowercase\" is used is to allow\n> rAnDOm casing in the configuration file, e.g.\n> \n> \t[trailer \"Signed-off-by\"]\n> \t\twhere = AfTEr\n> \n> which I find is totally unnecessary. Do we churn code to accept\n> such a nonsense input in other places?\n\nI think we are very inconsistent.\n\nAll bool config values allow \"tRuE\". Ones that take \"auto\" often use\nstrcasecmp (e.g., diff.*.binary). \"blame.date\" and \"help.format\" choose\nfrom a fixed set of tokens, but use strcmp.\n\nCommand line parameters are of course case-sensitive, and tokens used by\nthem usually are, too (e.g., the date formats for \"blame.date\" or also\nthe same ones taken by \"--date=\").\n\nIn general I do not see any reason _not_ to use strcasecmp for config\nvalues that are matching a fixed set. It's friendlier to the user, the\nextra CPU time is negligible, and the code is no harder to read than a\nstrcmp. Just looking at the callers in patch 04/12, I think it would be\nbetter just used strcasecmp instead of making a lowercase copy. Not\nbecause the copy is wasteful (it is, but it almost certainly doesn't\nmatter here), but because avoiding the copy is shorter and easier to\nfollow (you don't have to wonder about memory ownership).\n\n-Peff\n"},{"id":"237950","messageId":"xmqqha6jcl96.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"20140327223406.GA32434@sigill.intra.peff.net","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-27T22:47:01Z","receivedAt":"2014-03-27T22:47:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> All bool config values allow \"tRuE\".\n\nI was expecting somebody will bring it up, but think about it. Bool\nis a very special case. Even among CS folks, depending on your\nbackground, true may be True may be TRUE may be 1.\n\nConflating it with some random enum does not make a good argument.\n\n> Ones that take \"auto\" often use\n> strcasecmp (e.g., diff.*.binary). \"blame.date\" and \"help.format\" choose\n> from a fixed set of tokens, but use strcmp.\n\nI would say that the latter is the right thing to do.\n\n> In general I do not see any reason _not_ to use strcasecmp for config\n> values that are matching a fixed set. It's friendlier to the user,...\n\nActually, I think it ends up being hostile to the users to accept\nrandom cases without a good reason. If you see two trailer elements\nwhose where are specified as \"after\" and \"AFTER\" in somebody's\nconfiguration file, wouldn't that give a wrong impression that a new\nline with the latter somehow has a stronger desire to come later\nthan the former?\n\nIf you consistently take only the fixed strings, you do not have to\nworry about many people writing the same things in different ways,\nconfusing each other.\n\nI would however fully agree with you that using strcasecmp() would\nbe the cleanest when reading and maintaining the code **IF** we want\nto accept values in random case, but I do not agree that accepting\nrandom cases is a good thing, so...\n"},{"id":"237953","messageId":"20140327225658.GC32434@sigill.intra.peff.net","threadId":"36298","inReplyTo":"xmqqha6jcl96.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-27T22:56:58Z","receivedAt":"2014-03-27T22:56:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 27, 2014 at 03:47:01PM -0700, Junio C Hamano wrote:\n\n> Actually, I think it ends up being hostile to the users to accept\n> random cases without a good reason. If you see two trailer elements\n> whose where are specified as \"after\" and \"AFTER\" in somebody's\n> configuration file, wouldn't that give a wrong impression that a new\n> line with the latter somehow has a stronger desire to come later\n> than the former?\n> \n> If you consistently take only the fixed strings, you do not have to\n> worry about many people writing the same things in different ways,\n> confusing each other.\n\nI do not agree with this line of reasoning at all. After all, do we have\nconfusion about the case differences between:\n\n [COLOR]\n diff = true\n\n [color]\n UI = false\n\nBut I also do not overly care. Literally zero people have complained\nthat \"[log]date = RFC822\" is not accepted, so it is probably not a big\ndeal either way.\n\n-Peff\n"},{"id":"237984","messageId":"20140328.080256.175940793917277461.chriscool@tuxfamily.org","threadId":"36298","inReplyTo":"20140327223406.GA32434@sigill.intra.peff.net","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-03-28T07:02:56Z","receivedAt":"2014-03-28T07:02:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Jeff King \nSubject: Re: [PATCH v8 03/12] Move lower case functions into wrapper.c\nDate: Thu, 27 Mar 2014 18:34:06 -0400\n\n> On Thu, Mar 27, 2014 at 03:16:48PM -0700, Junio C Hamano wrote:\n> \n>> > I wasn't looking at the caller (and I haven't). I agree that, if\n>> > you have to compare case-insensitive user input against known set of\n>> > tokens, using strcasecmp() would be saner than making a downcased\n>> > copy and the set of known tokens. I do not know however you want to\n>> > compare in a case-insensitive way in your application, though.\n>>\n>> It appears that one place this \"lowercase\" is used is to allow\n>> rAnDOm casing in the configuration file, e.g.\n>> \n>> \t[trailer \"Signed-off-by\"]\n>> \t\twhere = AfTEr\n>> \n>> which I find is totally unnecessary. Do we churn code to accept\n>> such a nonsense input in other places?\n> \n> I think we are very inconsistent.\n> \n> All bool config values allow \"tRuE\". Ones that take \"auto\" often use\n> strcasecmp (e.g., diff.*.binary). \"blame.date\" and \"help.format\" choose\n> from a fixed set of tokens, but use strcmp.\n> \n> Command line parameters are of course case-sensitive, and tokens used by\n> them usually are, too (e.g., the date formats for \"blame.date\" or also\n> the same ones taken by \"--date=\").\n> \n> In general I do not see any reason _not_ to use strcasecmp for config\n> values that are matching a fixed set. It's friendlier to the user, the\n> extra CPU time is negligible, and the code is no harder to read than a\n> strcmp.\n\nI agree with this. I think it would be better to just use strcasecmp()\nfor all the config values matching a fixed set. It is just much easier\nto explain to users how things work this way.\n\nEven if no one ever complained about this on the mailing list, many\nusers complain that Git is very inconsistent.\n\n> Just looking at the callers in patch 04/12, I think it would be\n> better just used strcasecmp instead of making a lowercase copy. Not\n> because the copy is wasteful (it is, but it almost certainly doesn't\n> matter here), but because avoiding the copy is shorter and easier to\n> follow (you don't have to wonder about memory ownership).\n\nYeah, that's also why I am not very happy to have to change things in\nthis area.\n\nThanks,\nChristian.\n"},{"id":"238018","messageId":"xmqqzjkab634.fsf@gitster.dls.corp.google.com","threadId":"36298","inReplyTo":"20140327225658.GC32434@sigill.intra.peff.net","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-28T17:12:15Z","receivedAt":"2014-03-28T17:12:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But I also do not overly care. Literally zero people have complained\n> that \"[log]date = RFC822\" is not accepted, so it is probably not a big\n> deal either way.\n\nThat is most likely because we do not advertise these enum values\nspelled in random cases in our documentation and people do not even\nattempt to upcase the examples they see. So you are right to say\nthat it does not hurt anybody in practice if the code does not\nstrcasecmp() input from them. We do not know if using strcasecmp()\nthere and allowing them to feed the enums spelled in random cases\nwould invite confusions in the longer run, so let's not risk it.\nThere is no upside in using strcasecmp() here.\n\nBy the way, that is \"rfc2822\"---do we want \"rfc822\" as its synonym\nas well as \"rfc\", I wonder ;-)\n"},{"id":"238031","messageId":"20140328185018.GB29987@sigill.intra.peff.net","threadId":"36298","inReplyTo":"xmqqzjkab634.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8 03/12] Move lower case functions into wrapper.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-28T18:50:18Z","receivedAt":"2014-03-28T18:50:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 28, 2014 at 10:12:15AM -0700, Junio C Hamano wrote:\n\n> By the way, that is \"rfc2822\"---do we want \"rfc822\" as its synonym\n> as well as \"rfc\", I wonder ;-)\n\nOops, I wrote that as I was literally looking at the code that said\nrfc2822 and didn't notice. On the other hand, I have never made the\nmistake when actually running git, so it is probably not a big deal.\n\nBesides which, isn't it 5322 these days? I do not think we need to keep\nup with the treadmill.\n\n-Peff\n"}]}