Re: [PATCH v3 4/5] Always check `parse_tree*()`'s return value

Johannes Schindelin
2024-02-23T08:33:48Z

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

Hi Junio,

thank you so much for reviewing!

On Thu, 22 Feb 2024, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > + } else {
> > new_tree = repo_get_commit_tree(the_repository,
> > new_branch_info->commit);
> > + if (!new_tree)
> > + return error(_("unable to read tree %s"),
> > + oid_to_hex(&new_branch_info->commit->object.oid));
>
> We can help translators by enclosing %s inside a pair of parentheses.
>
> $ git grep -h 'msgid .*unable to read tree' po | sort | uniq -c
> 18 msgid "unable to read tree (%s)"

Right. I had used

$ git grep 'unable to read tree .*%s' | sed -n 's/.*_("\([^"]*\).*/\1/p' | sort | uniq -c
11 unable to read tree %s
3 unable to read tree (%s)

only to realize that the 11 were the ones I added.🤦 Re-running the same
command on v2.43.0 reports only the 3 parenthesized ones.

I've fixed those error messages in v4, and also added a patch to adjust
the one error message that I imitated (and to mark it for translation).
This patch might be slightly controversial because, in what is probably
only a hypothetical scenario, users might have scripted around `git
bisect` to parse error messages _and_ special-cased the "unable to read
tree" message to be able to deal with the missing tree in a programmatical
manner. I might be misjudging the likelihood for something like that, but
I vividly remember the, ahem, "exciting times" I had after 7560f547e61
(treewide: correct several "up-to-date" to "up to date", 2017-08-23). If
that patch is too concerning, I am open to dropping it.

Ciao,
Johannes