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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

thank you so much for reviewing!

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

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> 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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux