{"thread":{"id":"61190","subject":"[PATCH 1/2] factor out strbuf_expand_bad_format()","startedAt":"2024-03-24T08:00:34Z","lastAt":"2024-03-24T16:06:54Z","messageCount":7,"participants":["René Scharfe","Chris Torek","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"491343","messageId":"27cdcde7-74bc-4ee8-bc84-9a6046292cae@web.de","threadId":"61190","inReplyTo":null,"subject":"[PATCH 1/2] factor out strbuf_expand_bad_format()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-03-24T07:55:15Z","receivedAt":"2024-03-24T08:00:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Extract a function for reporting placeholders that are not enclosed in a\nparenthesis or are unknown.  This reduces the number of strings to\ntranslate and improves consistency across commands.  Call it at the end\nof the if/else chain, after exhausting all accepted possibilities.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/ls-files.c | 10 +---------\n builtin/ls-tree.c  | 10 +---------\n strbuf.c           | 20 ++++++++++++++++++++\n strbuf.h           |  5 +++++\n 4 files changed, 27 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 92f94e65bf..89e7e726c0 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -266,7 +266,6 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \tstruct strbuf sb = STRBUF_INIT;\n\n \twhile (strbuf_expand_step(&sb, &format)) {\n-\t\tconst char *end;\n \t\tsize_t len;\n \t\tstruct stat st;\n\n@@ -274,12 +273,6 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \t\t\tstrbuf_addch(&sb, '%');\n \t\telse if ((len = strbuf_expand_literal(&sb, format)))\n \t\t\tformat += len;\n-\t\telse if (*format != '(')\n-\t\t\tdie(_(\"bad ls-files format: element '%s' \"\n-\t\t\t      \"does not start with '('\"), format);\n-\t\telse if (!(end = strchr(format + 1, ')')))\n-\t\t\tdie(_(\"bad ls-files format: element '%s' \"\n-\t\t\t      \"does not end in ')'\"), format);\n \t\telse if (skip_prefix(format, \"(objectmode)\", &format))\n \t\t\tstrbuf_addf(&sb, \"%06o\", ce->ce_mode);\n \t\telse if (skip_prefix(format, \"(objectname)\", &format))\n@@ -308,8 +301,7 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \t\telse if (skip_prefix(format, \"(path)\", &format))\n \t\t\twrite_name_to_buf(&sb, fullname);\n \t\telse\n-\t\t\tdie(_(\"bad ls-files format: %%%.*s\"),\n-\t\t\t    (int)(end - format + 1), format);\n+\t\t\tstrbuf_expand_bad_format(format, \"ls-format\");\n \t}\n \tstrbuf_addch(&sb, line_terminator);\n \tfwrite(sb.buf, sb.len, 1, stdout);\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex e4a891337c..bd803ace03 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -100,19 +100,12 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \t\treturn 0;\n\n \twhile (strbuf_expand_step(&sb, &format)) {\n-\t\tconst char *end;\n \t\tsize_t len;\n\n \t\tif (skip_prefix(format, \"%\", &format))\n \t\t\tstrbuf_addch(&sb, '%');\n \t\telse if ((len = strbuf_expand_literal(&sb, format)))\n \t\t\tformat += len;\n-\t\telse if (*format != '(')\n-\t\t\tdie(_(\"bad ls-tree format: element '%s' \"\n-\t\t\t      \"does not start with '('\"), format);\n-\t\telse if (!(end = strchr(format + 1, ')')))\n-\t\t\tdie(_(\"bad ls-tree format: element '%s' \"\n-\t\t\t      \"does not end in ')'\"), format);\n \t\telse if (skip_prefix(format, \"(objectmode)\", &format))\n \t\t\tstrbuf_addf(&sb, \"%06o\", mode);\n \t\telse if (skip_prefix(format, \"(objecttype)\", &format))\n@@ -135,8 +128,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \t\t\tstrbuf_setlen(base, baselen);\n \t\t\tstrbuf_release(&sbuf);\n \t\t} else\n-\t\t\tdie(_(\"bad ls-tree format: %%%.*s\"),\n-\t\t\t    (int)(end - format + 1), format);\n+\t\t\tstrbuf_expand_bad_format(format, \"ls-tree\");\n \t}\n \tstrbuf_addch(&sb, options->null_termination ? '\\0' : '\\n');\n \tfwrite(sb.buf, sb.len, 1, stdout);\ndiff --git a/strbuf.c b/strbuf.c\nindex 7827178d8e..449eb610f1 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -442,6 +442,26 @@ size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder)\n \treturn 0;\n }\n\n+void strbuf_expand_bad_format(const char *format, const char *command)\n+{\n+\tconst char *end;\n+\n+\tif (*format != '(')\n+\t\t/* TRANSLATORS: The first %s is a command like \"ls-tree\". */\n+\t\tdie(_(\"bad %s format: element '%s' does not start with '('\"),\n+\t\t    command, format);\n+\n+\tend = strchr(format + 1, ')');\n+\tif (!end)\n+\t\t/* TRANSLATORS: The first %s is a command like \"ls-tree\". */\n+\t\tdie(_(\"bad %s format: element '%s' does not end in ')'\"),\n+\t\t    command, format);\n+\n+\t/* TRANSLATORS: %s is a command like \"ls-tree\". */\n+\tdie(_(\"bad %s format: %%%.*s\"),\n+\t    command, (int)(end - format + 1), format);\n+}\n+\n void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src)\n {\n \tsize_t i, len = src->len;\ndiff --git a/strbuf.h b/strbuf.h\nindex e959caca87..c758de3729 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -337,6 +337,11 @@ size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder);\n  */\n int strbuf_expand_step(struct strbuf *sb, const char **formatp);\n\n+/**\n+ * Used with `strbuf_expand_step` to report unknown placeholders.\n+ */\n+void strbuf_expand_bad_format(const char *format, const char *command);\n+\n /**\n  * Append the contents of one strbuf to another, quoting any\n  * percent signs (\"%\") into double-percents (\"%%\") in the\n--\n2.44.0\n"},{"id":"491344","messageId":"62c7ec20-4c1f-4a17-9deb-30699215251a@web.de","threadId":"61190","inReplyTo":"27cdcde7-74bc-4ee8-bc84-9a6046292cae@web.de","subject":"[PATCH 2/2] cat-file: use strbuf_expand_bad_format()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-03-24T08:04:21Z","receivedAt":"2024-03-24T08:04:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Report unknown format elements and missing closing parentheses with\nconsistent and translated messages by calling strbuf_expand_bad_format()\nat the very end of the combined if/else chain of expand_format() and\nexpand_atom().\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/cat-file.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex bbf851138e..fadf2da2f0 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -310,8 +310,8 @@ static int is_atom(const char *atom, const char *s, int slen)\n \treturn alen == slen && !memcmp(atom, s, alen);\n }\n\n-static void expand_atom(struct strbuf *sb, const char *atom, int len,\n-\t\t\tstruct expand_data *data)\n+static int expand_atom(struct strbuf *sb, const char *atom, int len,\n+\t\t       struct expand_data *data)\n {\n \tif (is_atom(\"objectname\", atom, len)) {\n \t\tif (!data->mark_query)\n@@ -343,7 +343,8 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,\n \t\t\tstrbuf_addstr(sb,\n \t\t\t\t      oid_to_hex(&data->delta_base_oid));\n \t} else\n-\t\tdie(\"unknown format element: %.*s\", len, atom);\n+\t\treturn 0;\n+\treturn 1;\n }\n\n static void expand_format(struct strbuf *sb, const char *start,\n@@ -354,12 +355,11 @@ static void expand_format(struct strbuf *sb, const char *start,\n\n \t\tif (skip_prefix(start, \"%\", &start) || *start != '(')\n \t\t\tstrbuf_addch(sb, '%');\n-\t\telse if (!(end = strchr(start + 1, ')')))\n-\t\t\tdie(\"format element '%s' does not end in ')'\", start);\n-\t\telse {\n-\t\t\texpand_atom(sb, start + 1, end - start - 1, data);\n+\t\telse if ((end = strchr(start + 1, ')')) &&\n+\t\t\t expand_atom(sb, start + 1, end - start - 1, data))\n \t\t\tstart = end + 1;\n-\t\t}\n+\t\telse\n+\t\t\tstrbuf_expand_bad_format(start, \"cat-file\");\n \t}\n }\n\n--\n2.44.0\n"},{"id":"491345","messageId":"CAPx1GveKUL1nexRQDYueAVU7G0vp+JzB=5yUf4r4eoRjU49Jcg@mail.gmail.com","threadId":"61190","inReplyTo":"27cdcde7-74bc-4ee8-bc84-9a6046292cae@web.de","subject":"Re: [PATCH 1/2] factor out strbuf_expand_bad_format()","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2024-03-24T09:00:35Z","receivedAt":"2024-03-24T09:00:48Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"Minor:\n\nOn Sun, Mar 24, 2024 at 1:00 AM René Scharfe <l.s.r@web.de> wrote:\n> @@ -308,8 +301,7 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n>                 else if (skip_prefix(format, \"(path)\", &format))\n>                         write_name_to_buf(&sb, fullname);\n>                 else\n> -                       die(_(\"bad ls-files format: %%%.*s\"),\n> -                           (int)(end - format + 1), format);\n> +                       strbuf_expand_bad_format(format, \"ls-format\");\n\nThis last string constant is clearly supposed to be \"ls-files\".\n\nChris\n"},{"id":"491348","messageId":"67aa076b-1a73-45a6-b727-6416963a1bd0@web.de","threadId":"61190","inReplyTo":"CAPx1GveKUL1nexRQDYueAVU7G0vp+JzB=5yUf4r4eoRjU49Jcg@mail.gmail.com","subject":"Re: [PATCH 1/2] factor out strbuf_expand_bad_format()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-03-24T11:17:11Z","receivedAt":"2024-03-24T11:17:15Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.03.24 um 10:00 schrieb Chris Torek:\n> Minor:\n>\n> On Sun, Mar 24, 2024 at 1:00 AM René Scharfe <l.s.r@web.de> wrote:\n>> @@ -308,8 +301,7 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n>>                 else if (skip_prefix(format, \"(path)\", &format))\n>>                         write_name_to_buf(&sb, fullname);\n>>                 else\n>> -                       die(_(\"bad ls-files format: %%%.*s\"),\n>> -                           (int)(end - format + 1), format);\n>> +                       strbuf_expand_bad_format(format, \"ls-format\");\n>\n> This last string constant is clearly supposed to be \"ls-files\".\n\nLOL.  Seems I can only hold one f-word in my mind at a time.\n\nThank you for catching this!\n\nRené\n"},{"id":"491349","messageId":"cf8f2256-d954-4a3e-bc2a-31b2de4c8e1d@web.de","threadId":"61190","inReplyTo":"27cdcde7-74bc-4ee8-bc84-9a6046292cae@web.de","subject":"[PATCH v2 1/2] factor out strbuf_expand_bad_format()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-03-24T11:19:40Z","receivedAt":"2024-03-24T11:19:42Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Extract a function for reporting placeholders that are not enclosed in a\nparenthesis or are unknown.  This reduces the number of strings to\ntranslate and improves consistency across commands.  Call it at the end\nof the if/else chain, after exhausting all accepted possibilities.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\nChanges since v1: s/ls-format/ls-files/\n\n builtin/ls-files.c | 10 +---------\n builtin/ls-tree.c  | 10 +---------\n strbuf.c           | 20 ++++++++++++++++++++\n strbuf.h           |  5 +++++\n 4 files changed, 27 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 92f94e65bf..6eeb5cba78 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -266,7 +266,6 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \tstruct strbuf sb = STRBUF_INIT;\n\n \twhile (strbuf_expand_step(&sb, &format)) {\n-\t\tconst char *end;\n \t\tsize_t len;\n \t\tstruct stat st;\n\n@@ -274,12 +273,6 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \t\t\tstrbuf_addch(&sb, '%');\n \t\telse if ((len = strbuf_expand_literal(&sb, format)))\n \t\t\tformat += len;\n-\t\telse if (*format != '(')\n-\t\t\tdie(_(\"bad ls-files format: element '%s' \"\n-\t\t\t      \"does not start with '('\"), format);\n-\t\telse if (!(end = strchr(format + 1, ')')))\n-\t\t\tdie(_(\"bad ls-files format: element '%s' \"\n-\t\t\t      \"does not end in ')'\"), format);\n \t\telse if (skip_prefix(format, \"(objectmode)\", &format))\n \t\t\tstrbuf_addf(&sb, \"%06o\", ce->ce_mode);\n \t\telse if (skip_prefix(format, \"(objectname)\", &format))\n@@ -308,8 +301,7 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \t\telse if (skip_prefix(format, \"(path)\", &format))\n \t\t\twrite_name_to_buf(&sb, fullname);\n \t\telse\n-\t\t\tdie(_(\"bad ls-files format: %%%.*s\"),\n-\t\t\t    (int)(end - format + 1), format);\n+\t\t\tstrbuf_expand_bad_format(format, \"ls-files\");\n \t}\n \tstrbuf_addch(&sb, line_terminator);\n \tfwrite(sb.buf, sb.len, 1, stdout);\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex e4a891337c..bd803ace03 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -100,19 +100,12 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \t\treturn 0;\n\n \twhile (strbuf_expand_step(&sb, &format)) {\n-\t\tconst char *end;\n \t\tsize_t len;\n\n \t\tif (skip_prefix(format, \"%\", &format))\n \t\t\tstrbuf_addch(&sb, '%');\n \t\telse if ((len = strbuf_expand_literal(&sb, format)))\n \t\t\tformat += len;\n-\t\telse if (*format != '(')\n-\t\t\tdie(_(\"bad ls-tree format: element '%s' \"\n-\t\t\t      \"does not start with '('\"), format);\n-\t\telse if (!(end = strchr(format + 1, ')')))\n-\t\t\tdie(_(\"bad ls-tree format: element '%s' \"\n-\t\t\t      \"does not end in ')'\"), format);\n \t\telse if (skip_prefix(format, \"(objectmode)\", &format))\n \t\t\tstrbuf_addf(&sb, \"%06o\", mode);\n \t\telse if (skip_prefix(format, \"(objecttype)\", &format))\n@@ -135,8 +128,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \t\t\tstrbuf_setlen(base, baselen);\n \t\t\tstrbuf_release(&sbuf);\n \t\t} else\n-\t\t\tdie(_(\"bad ls-tree format: %%%.*s\"),\n-\t\t\t    (int)(end - format + 1), format);\n+\t\t\tstrbuf_expand_bad_format(format, \"ls-tree\");\n \t}\n \tstrbuf_addch(&sb, options->null_termination ? '\\0' : '\\n');\n \tfwrite(sb.buf, sb.len, 1, stdout);\ndiff --git a/strbuf.c b/strbuf.c\nindex 7827178d8e..449eb610f1 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -442,6 +442,26 @@ size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder)\n \treturn 0;\n }\n\n+void strbuf_expand_bad_format(const char *format, const char *command)\n+{\n+\tconst char *end;\n+\n+\tif (*format != '(')\n+\t\t/* TRANSLATORS: The first %s is a command like \"ls-tree\". */\n+\t\tdie(_(\"bad %s format: element '%s' does not start with '('\"),\n+\t\t    command, format);\n+\n+\tend = strchr(format + 1, ')');\n+\tif (!end)\n+\t\t/* TRANSLATORS: The first %s is a command like \"ls-tree\". */\n+\t\tdie(_(\"bad %s format: element '%s' does not end in ')'\"),\n+\t\t    command, format);\n+\n+\t/* TRANSLATORS: %s is a command like \"ls-tree\". */\n+\tdie(_(\"bad %s format: %%%.*s\"),\n+\t    command, (int)(end - format + 1), format);\n+}\n+\n void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src)\n {\n \tsize_t i, len = src->len;\ndiff --git a/strbuf.h b/strbuf.h\nindex e959caca87..c758de3729 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -337,6 +337,11 @@ size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder);\n  */\n int strbuf_expand_step(struct strbuf *sb, const char **formatp);\n\n+/**\n+ * Used with `strbuf_expand_step` to report unknown placeholders.\n+ */\n+void strbuf_expand_bad_format(const char *format, const char *command);\n+\n /**\n  * Append the contents of one strbuf to another, quoting any\n  * percent signs (\"%\") into double-percents (\"%%\") in the\n--\n2.44.0\n"},{"id":"491350","messageId":"c6f915a9-0788-4492-b71a-37171a8bb13b@web.de","threadId":"61190","inReplyTo":"27cdcde7-74bc-4ee8-bc84-9a6046292cae@web.de","subject":"[PATCH v2 2/2] cat-file: use strbuf_expand_bad_format()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-03-24T11:21:15Z","receivedAt":"2024-03-24T11:21:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Report unknown format elements and missing closing parentheses with\nconsistent and translated messages by calling strbuf_expand_bad_format()\nat the very end of the combined if/else chain of expand_format() and\nexpand_atom().\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\nSame as v1.\n\n builtin/cat-file.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex bbf851138e..fadf2da2f0 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -310,8 +310,8 @@ static int is_atom(const char *atom, const char *s, int slen)\n \treturn alen == slen && !memcmp(atom, s, alen);\n }\n\n-static void expand_atom(struct strbuf *sb, const char *atom, int len,\n-\t\t\tstruct expand_data *data)\n+static int expand_atom(struct strbuf *sb, const char *atom, int len,\n+\t\t       struct expand_data *data)\n {\n \tif (is_atom(\"objectname\", atom, len)) {\n \t\tif (!data->mark_query)\n@@ -343,7 +343,8 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,\n \t\t\tstrbuf_addstr(sb,\n \t\t\t\t      oid_to_hex(&data->delta_base_oid));\n \t} else\n-\t\tdie(\"unknown format element: %.*s\", len, atom);\n+\t\treturn 0;\n+\treturn 1;\n }\n\n static void expand_format(struct strbuf *sb, const char *start,\n@@ -354,12 +355,11 @@ static void expand_format(struct strbuf *sb, const char *start,\n\n \t\tif (skip_prefix(start, \"%\", &start) || *start != '(')\n \t\t\tstrbuf_addch(sb, '%');\n-\t\telse if (!(end = strchr(start + 1, ')')))\n-\t\t\tdie(\"format element '%s' does not end in ')'\", start);\n-\t\telse {\n-\t\t\texpand_atom(sb, start + 1, end - start - 1, data);\n+\t\telse if ((end = strchr(start + 1, ')')) &&\n+\t\t\t expand_atom(sb, start + 1, end - start - 1, data))\n \t\t\tstart = end + 1;\n-\t\t}\n+\t\telse\n+\t\t\tstrbuf_expand_bad_format(start, \"cat-file\");\n \t}\n }\n\n--\n2.44.0\n"},{"id":"491358","messageId":"xmqqa5mnty06.fsf@gitster.g","threadId":"61190","inReplyTo":"cf8f2256-d954-4a3e-bc2a-31b2de4c8e1d@web.de","subject":"Re: [PATCH v2 1/2] factor out strbuf_expand_bad_format()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-24T16:06:49Z","receivedAt":"2024-03-24T16:06:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> @@ -274,12 +273,6 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n>  \t\t\tstrbuf_addch(&sb, '%');\n>  \t\telse if ((len = strbuf_expand_literal(&sb, format)))\n>  \t\t\tformat += len;\n> -\t\telse if (*format != '(')\n> -\t\t\tdie(_(\"bad ls-files format: element '%s' \"\n> -\t\t\t      \"does not start with '('\"), format);\n> -\t\telse if (!(end = strchr(format + 1, ')')))\n> -\t\t\tdie(_(\"bad ls-files format: element '%s' \"\n> -\t\t\t      \"does not end in ')'\"), format);\n\nWe used to do these two checks upfront, before the cascade of checks\nthat follow from here to the \"else die()\".\n\nBut we do not even take advantage of the fact that we already\nchecked these two in the following tests.  We do not take advantage\nof the fact that we know where the placeholder ends by computing\n\"end\" early, and all the checks in the \"else if\" cascade check that\nthe placeholder is enclosed inside a pair of parentheses again and\nagain.\n\nSo there is no point in optimizing to fail fast by having these two\ntests early.\n\n> +void strbuf_expand_bad_format(const char *format, const char *command)\n> +{\n> +\tconst char *end;\n> +\n> +\tif (*format != '(')\n> +\t\t/* TRANSLATORS: The first %s is a command like \"ls-tree\". */\n> +\t\tdie(_(\"bad %s format: element '%s' does not start with '('\"),\n> +\t\t    command, format);\n> +\n> +\tend = strchr(format + 1, ')');\n> +\tif (!end)\n> +\t\t/* TRANSLATORS: The first %s is a command like \"ls-tree\". */\n> +\t\tdie(_(\"bad %s format: element '%s' does not end in ')'\"),\n> +\t\t    command, format);\n> +\n> +\t/* TRANSLATORS: %s is a command like \"ls-tree\". */\n> +\tdie(_(\"bad %s format: %%%.*s\"),\n> +\t    command, (int)(end - format + 1), format);\n> +}\n> +\n\nNow these \"pair of parentheses\" checks are removed from the \"else\nif\" cascade, and we check them only to give a different message that\ntells _how_ the format was bad in expand_bad_format().\n\nLooks good.\n"}]}