Re: [PATCH v2] format-patch --signature-file <file>

From: Jeff King <peff@peff.net>
Date: May 17, 2014, 10:00 UTC
Message-ID: <20140517100013.GA18087@sigill.intra.peff.net>
In-Reply-To: <20140517085911.GA18862@hudson.localdomain>

On Sat, May 17, 2014 at 01:59:11AM -0700, Jeremiah Mahler wrote:

Show 15 quoted lines

> >   if (signature) {
> > 	if (signature_file)
> > 		die("you cannot specify both a signature and a signature-file");
> > 	/* otherwise, we already have the value */
> >   } else if (signature_file) {
> > 	struct strbuf buf = STRBUF_INIT;
> > 	strbuf_read(&buf, signature_file, 128);
> > 	signature = strbuf_detach(&buf);
> >   } else
> > 	signature = git_version_string;
> > 
> 
> Before, --no-signature would clear the &signature.
> With this code it sees it as not being set and assigns
> the default version string.

Ah, you're right. Thanks for catching it.

If you wanted to know whether it was set, I guess you'd have to compare it to the default, like:

  if (signature_file) {
	if (signature && signature != git_version_string)
		die("you cannot specify both a signature and a signature-file");
	... read signature file ...
  }

though it's a bit ugly that this code has to know what the default is. Having signature-file take precedence is OK with me, but it feels somewhat arbitrary to me from the user's perspective.

-Peff