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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx> writes:

> From: "Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx>
>
> --strip::
> 	Remove from tag message lines staring with '#', trailing spaces
> 	from every line and empty lines from the beginning and end.
> 	Enabled by default. Use --no-strip to overwrite the behaviour.
>
> --no-strip is useful if you want to take a tag message as-is, without
> any stripping.

That is not a commit log message ;-)

> Signed-off-by: Kirill A. Shutemov <kirill@xxxxxxxxxxxxx>

> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index c83cb13..dbb76a6 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -99,6 +99,11 @@ OPTIONS
>  	Implies `-a` if none of `-a`, `-s`, or `-u <key-id>`
>  	is given.
>  
> +--strip::
> +	Remove from tag message lines staring with '#', trailing spaces
> +	from every line and empty lines from the beginning and end.
> +	Enabled by default. Use --no-strip to overwrite the behaviour.

s/overwrite/override/; or replace the last sentence with "With
`--no-strip`, the tag message given by the user is used as-is".

> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9b6fd95..05a1fd4 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> ...
> @@ -356,7 +362,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 (opt->strip)
>  			write_or_die(fd, _(tag_template), strlen(_(tag_template)));

Why are you not writing template when no strip is done? (Not an objection
disguised as a rhetorical question, but a question).

The user who typed "tag -a v1.2.3 HEAD" that spawns an editor would still
find it useful to have commented instructions, once we start filling the
template with more useful information that is customized for the
situation (e.g. "git show -s --oneline" output), no?

> @@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  	const char *object_ref, *tag;
>  	struct ref_lock *lock;
>  
> -	int annotate = 0, sign = 0, force = 0, lines = -1,
> -		list = 0, delete = 0, verify = 0;
> +	struct create_tag_options opt = {
> +		.sign = 0,
> +		.strip = 1,
> +	};

Avoid doing this.  Even though these C99 initializers are nicer and more
readable way to write this, we try to be gentle to people with older
compilers that do not grok the syntax.

Except for the above minor nits, the patch basically looks good. Please
hold onto it and resubmit after 1.7.8 final ships.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]