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

From: Jeff King <peff@peff.net>
Date: Dec 12, 2023, 01:37 UTC
Message-ID: <20231212013702.GG376323@coredump.intra.peff.net>
In-Reply-To: <ZXOfrKYsmOjOHGmj@nand.local>

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

Show 19 quoted lines

> 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