From: Peijian Ju <eric.peijian@gmail.com>
Date: Mon, 16 Mar 2026 23:08:12 GMT
Subject: Re: [PATCH v2 1/1] refs: add 'preparing' phase to the reference-transaction hook
Message-ID: <CAN2LT1AeEYbCFvhUSnWPvCUtahVQP_cG8edVhURHg2N3OgMuwQ@mail.gmail.com>
In-Reply-To: <xmqqv7evpwrr.fsf@gitster.g>

On Mon, Mar 16, 2026 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Eric Ju <eric.peijian@gmail.com> writes:
>
> > +     /* Preparing checks before locking references */
> > +     ret = run_transaction_hook(transaction, "preparing");
> > +     if (ret) {
> > +             ref_transaction_abort(transaction, err);
> > +             die(_("ref updates aborted by the reference-transaction hook at its %s state"), "preparing");
> > +     }
>
> On end-user's terminal, the above should look like
>
> fatal: ref updates aborted by the reference-transaction hook at its parparing state
>
> consuming more than 80 columns and having the varying part of the
> message at the very end.  Can we shorten this and highlight the more
> important bits?  Here is my attempt
>
>                 die(_("in '%s' phase, update aborted by the reference-transaction hook"),
>                         "preparing");
>
> Enclosing the phase name in 'quotes' and moving it near the
> beginning are both my attempt to make it stand out more.
>
> Another thing you may want to consider is to extract the message to
> a separate constant, i.e.,
>
>         const char *abort_by_ref_transaction_hook[] =
>         N_("in '%s' phase, update aborted by the reference-transaction hook");
>
> and reuse at two places, perhaps?
>
>                 die(_(abort_by_ref_transaction_hook), "preparing");
>

Thank you. Fixed in V3.

- Eric