{"thread":{"id":"44643","subject":"[PATCH 0/2] A bit of ref-filter atom parsing cleanups","startedAt":"2016-12-07T16:10:20Z","lastAt":"2017-01-26T18:56:17Z","messageCount":6,"participants":["SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"307139","messageId":"20161207160923.7028-1-szeder.dev@gmail.com","threadId":"44643","inReplyTo":null,"subject":"[PATCH 0/2] A bit of ref-filter atom parsing cleanups","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2016-12-07T16:09:21Z","receivedAt":"2016-12-07T16:10:20Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The diffstat of the second patch doesn't show any benefits, but only\nbecause the first patch removed a callsite that would have benefited from\nit.\n\nIt merges cleanly with Karthik's \"port branch.c to use ref-filter's\nprinting options\" series.\n\nSZEDER Gábor (2):\n ref-filter, tag: eliminate duplicated sorting option parsing\n ref-filter: add function to parse atoms from a nul-terminated string\n\n builtin/tag.c | 24 ------------------------\n ref-filter.c | 24 +++++++++++++-----------\n ref-filter.h | 6 ++++++\n 3 files changed, 19 insertions(+), 35 deletions(-)\n\n-- \n2.11.0.78.g5a2d011\n\n"},{"id":"307140","messageId":"20161207160923.7028-3-szeder.dev@gmail.com","threadId":"44643","inReplyTo":"20161207160923.7028-1-szeder.dev@gmail.com","subject":"[PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2016-12-07T16:09:23Z","receivedAt":"2016-12-07T16:10:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"ref-filter's parse_ref_filter_atom() function parses an atom between\nthe start and end pointers it gets as arguments. This is fine for two\nof its callers, which process '%(atom)' format specifiers and the end\npointer comes directly from strchr() looking for the closing ')'.\nHowever, it's not quite so straightforward for its other two callers,\nwhich process sort specifiers given as plain nul-terminated strings.\nEspecially not for ref_default_sorting(), which has the default\nhard-coded as a string literal, but can't use it directly, because a\npointer to the end of that string literal is needed as well.\nThe next patch will add yet another caller using a string literal.\n\nAdd a helper function around parse_ref_filter_atom() to parse an atom\nup to the terminating nul, and call this helper in those two callers.\n\nSigned-off-by: SZEDER Gábor \n---\n ref-filter.c | 8 ++------\n ref-filter.h | 4 ++++\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex dfadf577c..3c6fd4ba7 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1658,19 +1658,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n /* If no sorting option is given, use refname to sort as default */\n struct ref_sorting *ref_default_sorting(void)\n {\n-\tstatic const char cstr_name[] = \"refname\";\n-\n \tstruct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));\n \n \tsorting->next = NULL;\n-\tsorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));\n+\tsorting->atom = parse_ref_filter_atom_from_string(\"refname\");\n \treturn sorting;\n }\n \n void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n {\n \tstruct ref_sorting *s;\n-\tint len;\n \n \ts = xcalloc(1, sizeof(*s));\n \ts->next = *sorting_tail;\n@@ -1683,8 +1680,7 @@ void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n \tif (skip_prefix(arg, \"version:\", &arg) ||\n \t skip_prefix(arg, \"v:\", &arg))\n \t\ts->version = 1;\n-\tlen = strlen(arg);\n-\ts->atom = parse_ref_filter_atom(arg, arg+len);\n+\ts->atom = parse_ref_filter_atom_from_string(arg);\n }\n \n int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 49466a17d..1250537cf 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -94,6 +94,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n void ref_array_clear(struct ref_array *array);\n /* Parse format string and sort specifiers */\n int parse_ref_filter_atom(const char *atom, const char *ep);\n+static inline int parse_ref_filter_atom_from_string(const char *atom)\n+{\n+\treturn parse_ref_filter_atom(atom, atom+strlen(atom));\n+}\n /* Used to verify if the given format is correct and to parse out the used atoms */\n int verify_ref_format(const char *format);\n /* Sort the given ref_array as per the ref_sorting provided */\n-- \n2.11.0.78.g5a2d011\n\n"},{"id":"307141","messageId":"20161207160923.7028-2-szeder.dev@gmail.com","threadId":"44643","inReplyTo":"20161207160923.7028-1-szeder.dev@gmail.com","subject":"[PATCH 1/2] ref-filter, tag: eliminate duplicated sorting option parsing","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2016-12-07T16:09:22Z","receivedAt":"2016-12-07T16:10:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Sorting options are either specified on the command line, which is\nhandled by parse_opt_ref_sorting() in ref-filter.c, or via the\n'tag.sort' config variable, which is handled by parse_sorting_string()\nin builtin/tag.c. These two functions are nearly identical, the\ndifference being only their signature and the former having a couple\nof extra lines at the beginning.\n\nEliminate the code duplication by making parse_sorting_string() part\nof the public ref-filter API, and turning parse_opt_ref_sorting() into\na thin wrapper around that function. This way builtin/tag.c can\ncontinue using it as before (and it might be useful if there ever will\nbe a 'branch.sort' config variable). Change its return type from int\nto void, because it always returned zero and none of its callers cared\nabout it (the actual error handling is done inside\nparse_ref_filter_atom() by die()ing on error).\n\nSigned-off-by: SZEDER Gábor \n---\n builtin/tag.c | 24 ------------------------\n ref-filter.c | 16 +++++++++++-----\n ref-filter.h | 2 ++\n 3 files changed, 13 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 50e4ae567..6fe723bee 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -122,30 +122,6 @@ static const char tag_template_nocleanup[] =\n \t\"Lines starting with '%c' will be kept; you may remove them\"\n \t\" yourself if you want to.\\n\");\n \n-/* Parse arg given and add it the ref_sorting array */\n-static int parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n-{\n-\tstruct ref_sorting *s;\n-\tint len;\n-\n-\ts = xcalloc(1, sizeof(*s));\n-\ts->next = *sorting_tail;\n-\t*sorting_tail = s;\n-\n-\tif (*arg == '-') {\n-\t\ts->reverse = 1;\n-\t\targ++;\n-\t}\n-\tif (skip_prefix(arg, \"version:\", &arg) ||\n-\t skip_prefix(arg, \"v:\", &arg))\n-\t\ts->version = 1;\n-\n-\tlen = strlen(arg);\n-\ts->atom = parse_ref_filter_atom(arg, arg+len);\n-\n-\treturn 0;\n-}\n-\n static int git_tag_config(const char *var, const char *value, void *cb)\n {\n \tint status;\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc551a752..dfadf577c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1667,15 +1667,11 @@ struct ref_sorting *ref_default_sorting(void)\n \treturn sorting;\n }\n \n-int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n+void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n {\n-\tstruct ref_sorting **sorting_tail = opt->value;\n \tstruct ref_sorting *s;\n \tint len;\n \n-\tif (!arg) /* should --no-sort void the list ? */\n-\t\treturn -1;\n-\n \ts = xcalloc(1, sizeof(*s));\n \ts->next = *sorting_tail;\n \t*sorting_tail = s;\n@@ -1689,6 +1685,16 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n \t\ts->version = 1;\n \tlen = strlen(arg);\n \ts->atom = parse_ref_filter_atom(arg, arg+len);\n+}\n+\n+int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct ref_sorting **sorting_tail = opt->value;\n+\n+\tif (!arg) /* should --no-sort void the list ? */\n+\t\treturn -1;\n+\n+\tparse_sorting_string(arg, sorting_tail);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 14d435e2c..49466a17d 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -100,6 +100,8 @@ int verify_ref_format(const char *format);\n void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);\n /* Print the ref using the given format and quote_style */\n void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style);\n+/* Parse given arg and append it to the ref_sorting list */\n+void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail);\n /* Callback function for parsing the sort option */\n int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);\n /* Default sort option based on refname */\n-- \n2.11.0.78.g5a2d011\n\n"},{"id":"307281","messageId":"CAM0VKjk1mnNzQX6LThq1t7keesBz_fjE9x2e0ywsBKSNKP9SCw@mail.gmail.com","threadId":"44643","inReplyTo":"20161207160923.7028-3-szeder.dev@gmail.com","subject":"Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2016-12-08T18:58:40Z","receivedAt":"2016-12-08T18:58:51Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor wrote:\n> ref-filter's parse_ref_filter_atom() function parses an atom between\n> the start and end pointers it gets as arguments. This is fine for two\n> of its callers, which process '%(atom)' format specifiers and the end\n> pointer comes directly from strchr() looking for the closing ')'.\n> However, it's not quite so straightforward for its other two callers,\n> which process sort specifiers given as plain nul-terminated strings.\n> Especially not for ref_default_sorting(), which has the default\n> hard-coded as a string literal, but can't use it directly, because a\n> pointer to the end of that string literal is needed as well.\n> The next patch will add yet another caller using a string literal.\n\nOops, that last sentence should be deleted, there is no third patch, sorry.\n\nGábor\n"},{"id":"310311","messageId":"CAM0VKjntATMwDTdu1fSmjeLbwVe73iTo2NQizNXjZchBzqG44w@mail.gmail.com","threadId":"44643","inReplyTo":"20161207160923.7028-3-szeder.dev@gmail.com","subject":"Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-01-26T13:15:38Z","receivedAt":"2017-01-26T13:15:46Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor wrote:\n> ref-filter's parse_ref_filter_atom() function parses an atom between\n> the start and end pointers it gets as arguments. This is fine for two\n> of its callers, which process '%(atom)' format specifiers and the end\n> pointer comes directly from strchr() looking for the closing ')'.\n> However, it's not quite so straightforward for its other two callers,\n> which process sort specifiers given as plain nul-terminated strings.\n> Especially not for ref_default_sorting(), which has the default\n> hard-coded as a string literal, but can't use it directly, because a\n> pointer to the end of that string literal is needed as well.\n> The next patch will add yet another caller using a string literal.\n>\n> Add a helper function around parse_ref_filter_atom() to parse an atom\n> up to the terminating nul, and call this helper in those two callers.\n>\n> Signed-off-by: SZEDER Gábor \n> ---\n> ref-filter.c | 8 ++------\n> ref-filter.h | 4 ++++\n> 2 files changed, 6 insertions(+), 6 deletions(-)\n\nPing?\n\nIt looks like that this little two piece cleanup series fell on the floor.\n\n\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index dfadf577c..3c6fd4ba7 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1658,19 +1658,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n> /* If no sorting option is given, use refname to sort as default */\n> struct ref_sorting *ref_default_sorting(void)\n> {\n> - static const char cstr_name[] = \"refname\";\n> -\n> struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));\n>\n> sorting->next = NULL;\n> - sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));\n> + sorting->atom = parse_ref_filter_atom_from_string(\"refname\");\n> return sorting;\n> }\n>\n> void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n> {\n> struct ref_sorting *s;\n> - int len;\n>\n> s = xcalloc(1, sizeof(*s));\n> s->next = *sorting_tail;\n> @@ -1683,8 +1680,7 @@ void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n> if (skip_prefix(arg, \"version:\", &arg) ||\n> skip_prefix(arg, \"v:\", &arg))\n> s->version = 1;\n> - len = strlen(arg);\n> - s->atom = parse_ref_filter_atom(arg, arg+len);\n> + s->atom = parse_ref_filter_atom_from_string(arg);\n> }\n>\n> int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n> diff --git a/ref-filter.h b/ref-filter.h\n> index 49466a17d..1250537cf 100644\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -94,6 +94,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n> void ref_array_clear(struct ref_array *array);\n> /* Parse format string and sort specifiers */\n> int parse_ref_filter_atom(const char *atom, const char *ep);\n> +static inline int parse_ref_filter_atom_from_string(const char *atom)\n> +{\n> + return parse_ref_filter_atom(atom, atom+strlen(atom));\n> +}\n> /* Used to verify if the given format is correct and to parse out the used atoms */\n> int verify_ref_format(const char *format);\n> /* Sort the given ref_array as per the ref_sorting provided */\n> --\n> 2.11.0.78.g5a2d011\n>\n"},{"id":"310349","messageId":"xmqqvat14poe.fsf@gitster.mtv.corp.google.com","threadId":"44643","inReplyTo":"CAM0VKjntATMwDTdu1fSmjeLbwVe73iTo2NQizNXjZchBzqG44w@mail.gmail.com","subject":"Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-26T18:54:57Z","receivedAt":"2017-01-26T18:56:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor wrote:\n>> ref-filter's parse_ref_filter_atom() function parses an atom between\n>> the start and end pointers it gets as arguments. This is fine for two\n>> of its callers, which process '%(atom)' format specifiers and the end\n>> pointer comes directly from strchr() looking for the closing ')'.\n>> However, it's not quite so straightforward for its other two callers,\n>> which process sort specifiers given as plain nul-terminated strings.\n>> Especially not for ref_default_sorting(), which has the default\n>> hard-coded as a string literal, but can't use it directly, because a\n>> pointer to the end of that string literal is needed as well.\n>> The next patch will add yet another caller using a string literal.\n>>\n>> Add a helper function around parse_ref_filter_atom() to parse an atom\n>> up to the terminating nul, and call this helper in those two callers.\n>>\n>> Signed-off-by: SZEDER Gábor \n>> ---\n>> ref-filter.c | 8 ++------\n>> ref-filter.h | 4 ++++\n>> 2 files changed, 6 insertions(+), 6 deletions(-)\n>\n> Ping?\n>\n> It looks like that this little two piece cleanup series fell on the floor.\n\nI am still waiting for somebody else to comment on the changes, and\nthe final reroll after such a review discussion to address your own\ncomment in \n\n\n\n"}]}