Re: [PATCH] tag: implement --no-strip option

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2011, 20:26 UTC
Message-ID: <7v7h32jvrq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1321268902-2170-1-git-send-email-kirill@shutemov.name>

"Kirill A. Shutemov" <kirill@shutemov.name> writes:

> +-S::
> +--no-strip::
> +	Take tag message as-is. Do not strip any comments or empty lines.

Wrong naming convention. Just introduce a boolean --strip option which is on by default, without adding -S; use of parse-options will allow the user to negate it from the command line with --no-strip for free.

Show 10 quoted lines

> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9b6fd95..427d646 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -320,7 +320,7 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
>  }
>  
>  static void create_tag(const unsigned char *object, const char *tag,
> -		       struct strbuf *buf, int message, int sign,
> +		       struct strbuf *buf, int message, int sign, int nostrip,

Again, wrong naming convention. "int strip" would be fine but I think at this point "message, sign, strip" tuple should become fields of "struct create_tag_option" that is passed to this funciton.

Show 9 quoted lines

>  		       unsigned char *prev, unsigned char *result)
>  {
>  	enum object_type type;
> @@ -356,7 +356,7 @@ static void create_tag(const unsigned char *object, const char *tag,
>  
>  		if (!is_null_sha1(prev))
>  			write_tag_body(fd, prev);
> -		else
> +		else if (!nostrip)

This double negation comes only because the argument is misnamed with negation.