[1/2] factor out strbuf_expand_bad_format()

Commit Message

René Scharfe March 24, 2024, 7:55 a.m. UTC

Extract a function for reporting placeholders that are not enclosed in a
parenthesis or are unknown.  This reduces the number of strings to
translate and improves consistency across commands.  Call it at the end
of the if/else chain, after exhausting all accepted possibilities.

Signed-off-by: René Scharfe <[e-mail]>
---
 builtin/ls-files.c | 10 +---------
 builtin/ls-tree.c  | 10 +---------
 strbuf.c           | 20 ++++++++++++++++++++
 strbuf.h           |  5 +++++
 4 files changed, 27 insertions(+), 18 deletions(-)

--
2.44.0

Comments

Chris Torek March 24, 2024, 9 a.m. UTC | #1

Minor:

On Sun, Mar 24, 2024 at 1:00 AM René Scharfe <[e-mail]> wrote:
> @@ -308,8 +301,7 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
>                 else if (skip_prefix(format, "(path)", &format))
>                         write_name_to_buf(&sb, fullname);
>                 else
> -                       die(_("bad ls-files format: %%%.*s"),
> -                           (int)(end - format + 1), format);
> +                       strbuf_expand_bad_format(format, "ls-format");

This last string constant is clearly supposed to be "ls-files".

Chris

René Scharfe March 24, 2024, 11:17 a.m. UTC | #2

Am 24.03.24 um 10:00 schrieb Chris Torek:
> Minor:
>
> On Sun, Mar 24, 2024 at 1:00 AM René Scharfe <[e-mail]> wrote:
>> @@ -308,8 +301,7 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
>>                 else if (skip_prefix(format, "(path)", &format))
>>                         write_name_to_buf(&sb, fullname);
>>                 else
>> -                       die(_("bad ls-files format: %%%.*s"),
>> -                           (int)(end - format + 1), format);
>> +                       strbuf_expand_bad_format(format, "ls-format");
>
> This last string constant is clearly supposed to be "ls-files".

LOL.  Seems I can only hold one f-word in my mind at a time.

Thank you for catching this!

René

Patch

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 92f94e65bf..89e7e726c0 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -266,7 +266,6 @@  static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 	struct strbuf sb = STRBUF_INIT;

 	while (strbuf_expand_step(&sb, &format)) {
-		const char *end;
 		size_t len;
 		struct stat st;

@@ -274,12 +273,6 @@  static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 			strbuf_addch(&sb, '%');
 		else if ((len = strbuf_expand_literal(&sb, format)))
 			format += len;
-		else if (*format != '(')
-			die(_("bad ls-files format: element '%s' "
-			      "does not start with '('"), format);
-		else if (!(end = strchr(format + 1, ')')))
-			die(_("bad ls-files format: element '%s' "
-			      "does not end in ')'"), format);
 		else if (skip_prefix(format, "(objectmode)", &format))
 			strbuf_addf(&sb, "%06o", ce->ce_mode);
 		else if (skip_prefix(format, "(objectname)", &format))
@@ -308,8 +301,7 @@  static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 		else if (skip_prefix(format, "(path)", &format))
 			write_name_to_buf(&sb, fullname);
 		else
-			die(_("bad ls-files format: %%%.*s"),
-			    (int)(end - format + 1), format);
+			strbuf_expand_bad_format(format, "ls-format");
 	}
 	strbuf_addch(&sb, line_terminator);
 	fwrite(sb.buf, sb.len, 1, stdout);
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index e4a891337c..bd803ace03 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -100,19 +100,12 @@  static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 		return 0;

 	while (strbuf_expand_step(&sb, &format)) {
-		const char *end;
 		size_t len;

 		if (skip_prefix(format, "%", &format))
 			strbuf_addch(&sb, '%');
 		else if ((len = strbuf_expand_literal(&sb, format)))
 			format += len;
-		else if (*format != '(')
-			die(_("bad ls-tree format: element '%s' "
-			      "does not start with '('"), format);
-		else if (!(end = strchr(format + 1, ')')))
-			die(_("bad ls-tree format: element '%s' "
-			      "does not end in ')'"), format);
 		else if (skip_prefix(format, "(objectmode)", &format))
 			strbuf_addf(&sb, "%06o", mode);
 		else if (skip_prefix(format, "(objecttype)", &format))
@@ -135,8 +128,7 @@  static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 			strbuf_setlen(base, baselen);
 			strbuf_release(&sbuf);
 		} else
-			die(_("bad ls-tree format: %%%.*s"),
-			    (int)(end - format + 1), format);
+			strbuf_expand_bad_format(format, "ls-tree");
 	}
 	strbuf_addch(&sb, options->null_termination ? '\0' : '\n');
 	fwrite(sb.buf, sb.len, 1, stdout);
diff --git a/strbuf.c b/strbuf.c
index 7827178d8e..449eb610f1 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -442,6 +442,26 @@  size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder)
 	return 0;
 }

+void strbuf_expand_bad_format(const char *format, const char *command)
+{
+	const char *end;
+
+	if (*format != '(')
+		/* TRANSLATORS: The first %s is a command like "ls-tree". */
+		die(_("bad %s format: element '%s' does not start with '('"),
+		    command, format);
+
+	end = strchr(format + 1, ')');
+	if (!end)
+		/* TRANSLATORS: The first %s is a command like "ls-tree". */
+		die(_("bad %s format: element '%s' does not end in ')'"),
+		    command, format);
+
+	/* TRANSLATORS: %s is a command like "ls-tree". */
+	die(_("bad %s format: %%%.*s"),
+	    command, (int)(end - format + 1), format);
+}
+
 void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src)
 {
 	size_t i, len = src->len;
diff --git a/strbuf.h b/strbuf.h
index e959caca87..c758de3729 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -337,6 +337,11 @@  size_t strbuf_expand_literal(struct strbuf *sb, const char *placeholder);
  */
 int strbuf_expand_step(struct strbuf *sb, const char **formatp);

+/**
+ * Used with `strbuf_expand_step` to report unknown placeholders.
+ */
+void strbuf_expand_bad_format(const char *format, const char *command);
+
 /**
  * Append the contents of one strbuf to another, quoting any
  * percent signs ("%") into double-percents ("%%") in the