diff mbox series

[v2,2/2] object-name: make ambiguous object output translatable

Message ID patch-v2-2.2-c0e873543f5-20211004T142523Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series i18n: improve translatability of ambiguous object output | expand

Commit Message

Ævar Arnfjörð Bjarmason Oct. 4, 2021, 2:27 p.m. UTC
Change the output of show_ambiguous_object() added in [1] and last
tweaked in [2] to be more friendly to translators. By being able to
customize the sprintf formats we're even ready for RTL languages.

The "unknown type" message here is unreachable, and has been since
[1], i.e. that code has never worked. If we craft an object of a bogus
type with a conflicting prefix we'll just die:

    $ git rev-parse 8315
    error: short object ID 8315 is ambiguous
    hint: The candidates are:
    fatal: invalid object type

But let's continue to pretend that this works, we can eventually use
the API improvements in my ab/fsck-unexpected-type (once it lands) to
inspect these objects and emit the actual type here, or at least not
die as we emit "unknown type".

1. 1ffa26c461 (get_short_sha1: list ambiguous objects on error,
   2016-09-26)
2. 5cc044e0257 (get_short_oid: sort ambiguous objects by type,
   then SHA-1, 2018-05-10)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 object-name.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 66 insertions(+), 6 deletions(-)

Comments

Jeff King Oct. 6, 2021, 7:11 p.m. UTC | #1
On Mon, Oct 04, 2021 at 04:27:02PM +0200, Ævar Arnfjörð Bjarmason wrote:

> +	abbrev = repo_find_unique_abbrev(ds->repo, oid, DEFAULT_ABBREV);
> +	if (type == OBJ_COMMIT) {
> +		/*
> +		 * TRANSLATORS: This is a line of ambiguous commit
> +		 * object output. E.g.:
> +		 *
> +		 *    "deadbeef commit 2021-01-01 - Some Commit Message"
> +		 *
> +		 * The second argument is the "commit" string from
> +		 * object.c, it should (hopefully) already be
> +		 * translated.
> +		 */
> +		strbuf_addf(&desc, _("%s %s %s - %s"), abbrev, ci_ad.buf,
> +			    _(type_name(type)), ci_s.buf);
> +	} else if (tag_desc) {
> [...]

OK, this all looks reasonable to me. I'd probably have ditched "desc"
altogether in favor of just calling advise(), to give translators even
more information about what we're trying to output, but I admit I don't
care that much either way.

I'm still not sure if translating the object types is a good idea or
not, per my other response.

> +	/*
> +	 * TRANSLATORS: This is line item of ambiguous object output,
> +	 * translated above.
> +	 */
> +	advise(_("  %s\n"), desc.buf);

The "\n" here isn't necessary (and wasn't present in the original, but
it doesn't hurt, as advise()'s algorithm gobbles any newlines as it
splits). I guess it helps making this otherwise un-notable string more
unique for translation, but just stuffing the indentation into the
earlier calls would do an even better job of that.

-Peff
diff mbox series

Patch

diff --git a/object-name.c b/object-name.c
index fdff4601b2c..73c946f1117 100644
--- a/object-name.c
+++ b/object-name.c
@@ -355,7 +355,11 @@  static int show_ambiguous_object(const struct object_id *oid, void *data)
 {
 	const struct disambiguate_state *ds = data;
 	struct strbuf desc = STRBUF_INIT;
+	struct strbuf ci_ad = STRBUF_INIT;
+	struct strbuf ci_s = STRBUF_INIT;
 	int type;
+	const char *tag_desc = NULL;
+	const char *abbrev;
 
 	if (ds->fn && !ds->fn(ds->repo, oid, ds->cb_data))
 		return 0;
@@ -366,20 +370,76 @@  static int show_ambiguous_object(const struct object_id *oid, void *data)
 		if (commit) {
 			struct pretty_print_context pp = {0};
 			pp.date_mode.type = DATE_SHORT;
-			format_commit_message(commit, " %ad - %s", &desc, &pp);
+			format_commit_message(commit, "%ad", &ci_ad, &pp);
+			format_commit_message(commit, "%s", &ci_s, &pp);
 		}
 	} else if (type == OBJ_TAG) {
 		struct tag *tag = lookup_tag(ds->repo, oid);
 		if (!parse_tag(tag) && tag->tag)
-			strbuf_addf(&desc, " %s", tag->tag);
+			tag_desc = tag->tag;
 	}
 
-	advise("  %s %s%s",
-	       repo_find_unique_abbrev(ds->repo, oid, DEFAULT_ABBREV),
-	       type_name(type) ? type_name(type) : "unknown type",
-	       desc.buf);
+	abbrev = repo_find_unique_abbrev(ds->repo, oid, DEFAULT_ABBREV);
+	if (type == OBJ_COMMIT) {
+		/*
+		 * TRANSLATORS: This is a line of ambiguous commit
+		 * object output. E.g.:
+		 *
+		 *    "deadbeef commit 2021-01-01 - Some Commit Message"
+		 *
+		 * The second argument is the "commit" string from
+		 * object.c, it should (hopefully) already be
+		 * translated.
+		 */
+		strbuf_addf(&desc, _("%s %s %s - %s"), abbrev, ci_ad.buf,
+			    _(type_name(type)), ci_s.buf);
+	} else if (tag_desc) {
+		/*
+		 * TRANSLATORS: This is a line of
+		 * ambiguous tag object output. E.g.:
+		 *
+		 *    "deadbeef tag Some Tag Message"
+		 *
+		 * The second argument is the "tag" string from
+		 * object.c, it should (hopefully) already be
+		 * translated.
+		 */
+		strbuf_addf(&desc, _("%s %s %s"), abbrev, _(type_name(type)),
+			    tag_desc);
+	} else {
+		const char *tname = type_name(type) ? _(type_name(type)) :
+			_(unknown_type);
+		/*
+		 * TRANSLATORS: This is a line of ambiguous <type>
+		 * object output. Where <type> is one of the object
+		 * types of "tree", "blob", "tag" ("commit" is handled
+		 * above).
+		 *
+		 *    "deadbeef tree"
+		 *    "deadbeef blob"
+		 *    "deadbeef tag"
+		 *    "deadbeef unknown type"
+		 *
+		 * Note that annotated tags use a separate format
+		 * outlined above.
+		 *
+		 * The second argument is the "tree", "blob" or "tag"
+		 * string from object.c, or the "unknown type" string
+		 * in the case of an unknown type. All of them should
+		 * (hopefully) already be translated.
+		 */
+		strbuf_addf(&desc, _("%s %s"), abbrev, tname);
+	}
+
+	/*
+	 * TRANSLATORS: This is line item of ambiguous object output,
+	 * translated above.
+	 */
+	advise(_("  %s\n"), desc.buf);
 
 	strbuf_release(&desc);
+	strbuf_release(&ci_ad);
+	strbuf_release(&ci_s);
 	return 0;
 }