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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 23, 2024, 08:33 UTC
Message-ID: <883087b8-b013-7b30-5485-719a1c310608@gmx.de>
In-Reply-To: <xmqqplwoe5yv.fsf@gitster.g>

Hi Junio,

thank you so much for reviewing!

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

Show 14 quoted lines

> "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