Re: [PATCH v2 1/2] format-patch: Add a config option format.from to set the default for --from
- From
Josh Triplett <josh@joshtriplett.org>
- Date
- Aug 7, 2016, 22:57 UTC
- Message-ID
- <20160807225701.ucv2xunq5vs4uedk@x>
- In-Reply-To
- <20160801173847.qph2tora75h6ebsk@sigill.intra.peff.net>
On Mon, Aug 01, 2016 at 01:38:47PM -0400, Jeff King wrote:
Show 39 quoted lines
> On Sat, Jul 30, 2016 at 12:11:11PM -0700, Josh Triplett wrote:
>
> > +enum from {
> > + FROM_AUTHOR,
> > + FROM_USER,
> > + FROM_VALUE,
> > +};
> > +
> > +static void set_from(enum from type, const char *value)
> > +{
> > + free(from);
> > + switch (type) {
> > + case FROM_AUTHOR:
> > + from = NULL;
> > + break;
> > + case FROM_USER:
> > + from = xstrdup(git_committer_info(IDENT_NO_DATE));
> > + break;
> > + case FROM_VALUE:
> > + from = xstrdup(value);
> > + break;
> > + }
> > +}
>
> Thanks for looking into reducing the duplication. TBH, I am not sure it
> is really an improvement, just because of the amount of boilerplate (and
> this function interface is kind of weird, because of the rules for when
> "value" should or should not be NULL).
>
> I guess another way to do it would be:
>
> #define FROM_AUTO_IDENT ((const char *)(intptr_t)1))
> void set_from(const char *value)
> {
> if (value == FROM_AUTO_IDENT)
> value = git_committer_info(IDENT_NO_DATE);
> free(from);
> from = xstrdup_or_null(value);
> }I'd actually seriously considered this exact approach, which I preferred as well, but I'd discarded it because I figured it'd get rejected. Given your suggestion, and Junio's comment, I'll go with this version.
- Josh Triplett