From: Jeff King Date: Sat, 17 May 2014 10:00:14 GMT Subject: Re: [PATCH v2] format-patch --signature-file 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: > > 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