Re: [PATCH v2 1/2] format-patch: Add a config option format.from to set the default for --from

From: Jeff King <peff@peff.net>
Date: Aug 1, 2016, 17:38 UTC
Message-ID: <20160801173847.qph2tora75h6ebsk@sigill.intra.peff.net>
In-Reply-To: <20160730191111.cd6ay3l4hweyjf7f@x>

On Sat, Jul 30, 2016 at 12:11:11PM -0700, Josh Triplett wrote:

Show 21 quoted lines

> +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);
  }

but I think the effort to polish further here is outweighing the magnitude of the patch itself. So I offer that as "how I would have done it" in case you like it, but again, I am fine with either this version or the previous.

-Peff