Re: [PATCH 3/9] imap-send: don't use git_die_config() inside callback

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

 



On Fri, Dec 08, 2023 at 05:58:52PM -0500, Taylor Blau wrote:

> On Thu, Dec 07, 2023 at 09:57:55AM +0100, Patrick Steinhardt wrote:
> > On Thu, Dec 07, 2023 at 02:24:58AM -0500, Jeff King wrote:
> > [snip]
> > > diff --git a/imap-send.c b/imap-send.c
> > > index 996651e4f8..5b0fe4f95a 100644
> > > --- a/imap-send.c
> > > +++ b/imap-send.c
> > > @@ -1346,7 +1346,7 @@ static int git_imap_config(const char *var, const char *val,
> > >  		server.port = git_config_int(var, val, ctx->kvi);
> > >  	else if (!strcmp("imap.host", var)) {
> > >  		if (!val) {
> > > -			git_die_config("imap.host", "Missing value for 'imap.host'");
> > > +			return error("Missing value for 'imap.host'");
> >
> > Nit: while at it we might also mark this error for translation. Not
> > worth a reroll on its own though.
> 
> This string goes away entirely in the next patch, so I don't think we
> need to mark it here.

Right. It's a little confusing because it is converted along with some
other spots in the next patch. But in one of those other spots, we
earlier switched it (in patch 2) from die() to error(), and we _did_
mark it for translation as we did so.

I did it there because in patch 2 we touch multiple messages, and the
other ones don't end up as config_error_nonbool(), so we do want them
translated.

I'm not sure if there would have been an easier ordering to the series.
I could have pulled the "mark for translation" bits from patch 2 into
their own patch (after this one makes some of the messages go away), but
then I'd expect somebody would review patch 2 and say "why not mark them
for translation while we're here?". :)

-Peff




[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