{"thread":{"id":"28935","subject":"[PATCH, v2] tag: implement --[no-]strip option","startedAt":"2011-11-14T21:43:39Z","lastAt":"2011-11-15T07:17:36Z","messageCount":6,"participants":["Kirill A. Shutemov","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"179448","messageId":"1321307019-5557-1-git-send-email-kirill@shutemov.name","threadId":"28935","inReplyTo":null,"subject":"[PATCH, v2] tag: implement --[no-]strip option","fromName":"Kirill A. Shutemov","fromEmail":"kirill@shutemov.name","sentAt":"2011-11-14T21:43:39Z","receivedAt":"2011-11-14T21:43:39Z","isPatch":true,"sender":{"key":"kirill@shutemov.name","avatar":"https://avatars.githubusercontent.com/u/588661?v=4"},"body":"From: \"Kirill A. Shutemov\" <kirill@shutemov.name>\n\n--strip::\n\tRemove from tag message lines staring with '#', trailing spaces\n\tfrom every line and empty lines from the beginning and end.\n\tEnabled by default. Use --no-strip to overwrite the behaviour.\n\n--no-strip is useful if you want to take a tag message as-is, without\nany stripping.\n\nSigned-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n---\n Documentation/git-tag.txt |    5 +++++\n builtin/tag.c             |   41 ++++++++++++++++++++++++++++-------------\n 2 files changed, 33 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex c83cb13..dbb76a6 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -99,6 +99,11 @@ OPTIONS\n \tImplies `-a` if none of `-a`, `-s`, or `-u <key-id>`\n \tis given.\n \n+--strip::\n+\tRemove from tag message lines staring with '#', trailing spaces\n+\tfrom every line and empty lines from the beginning and end.\n+\tEnabled by default. Use --no-strip to overwrite the behaviour.\n+\n <tagname>::\n \tThe name of the tag to create, delete, or describe.\n \tThe new tag name must pass all checks defined by\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 9b6fd95..05a1fd4 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -319,8 +319,14 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)\n \treturn 0;\n }\n \n+struct create_tag_options {\n+\tunsigned int message;\n+\tunsigned int sign;\n+\tunsigned int strip;\n+};\n+\n static void create_tag(const unsigned char *object, const char *tag,\n-\t\t       struct strbuf *buf, int message, int sign,\n+\t\t       struct strbuf *buf, struct create_tag_options *opt,\n \t\t       unsigned char *prev, unsigned char *result)\n {\n \tenum object_type type;\n@@ -345,7 +351,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n \tif (header_len > sizeof(header_buf) - 1)\n \t\tdie(_(\"tag header too big.\"));\n \n-\tif (!message) {\n+\tif (!opt->message) {\n \t\tint fd;\n \n \t\t/* write the template message before editing: */\n@@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n \n \t\tif (!is_null_sha1(prev))\n \t\t\twrite_tag_body(fd, prev);\n-\t\telse\n+\t\telse if (opt->strip)\n \t\t\twrite_or_die(fd, _(tag_template), strlen(_(tag_template)));\n \t\tclose(fd);\n \n@@ -367,14 +373,15 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t\t}\n \t}\n \n-\tstripspace(buf, 1);\n+\tif (opt->strip)\n+\t\tstripspace(buf, 1);\n \n-\tif (!message && !buf->len)\n+\tif (opt->message && !buf->len)\n \t\tdie(_(\"no tag message?\"));\n \n \tstrbuf_insert(buf, 0, header_buf, header_len);\n \n-\tif (build_tag_object(buf, sign, result) < 0) {\n+\tif (build_tag_object(buf, opt->sign, result) < 0) {\n \t\tif (path)\n \t\t\tfprintf(stderr, _(\"The tag message has been left in %s\\n\"),\n \t\t\t\tpath);\n@@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tconst char *object_ref, *tag;\n \tstruct ref_lock *lock;\n \n-\tint annotate = 0, sign = 0, force = 0, lines = -1,\n-\t\tlist = 0, delete = 0, verify = 0;\n+\tstruct create_tag_options opt = {\n+\t\t.sign = 0,\n+\t\t.strip = 1,\n+\t};\n+\n+\tint annotate = 0, force = 0, lines = -1, list = 0,\n+\t    delete = 0, verify = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct commit_list *with_commit = NULL;\n@@ -442,7 +454,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK('m', \"message\", &msg, \"message\",\n \t\t\t     \"tag message\", parse_msg_arg),\n \t\tOPT_FILENAME('F', \"file\", &msgfile, \"read message from file\"),\n-\t\tOPT_BOOLEAN('s', \"sign\", &sign, \"annotated and GPG-signed tag\"),\n+\t\tOPT_BOOLEAN('s', \"sign\", &opt.sign, \"annotated and GPG-signed tag\"),\n+\t\tOPT_BOOLEAN(0, \"strip\", &opt.strip,\n+\t\t\t\t\t\"turn off tag message stripping\"),\n \t\tOPT_STRING('u', \"local-user\", &keyid, \"key-id\",\n \t\t\t\t\t\"use another key to sign the tag\"),\n \t\tOPT__FORCE(&force, \"replace the tag if exists\"),\n@@ -462,10 +476,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n \tif (keyid) {\n-\t\tsign = 1;\n+\t\topt.sign = 1;\n \t\tset_signingkey(keyid);\n \t}\n-\tif (sign)\n+\tif (opt.sign)\n \t\tannotate = 1;\n \tif (argc == 0 && !(delete || verify))\n \t\tlist = 1;\n@@ -523,9 +537,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse if (!force)\n \t\tdie(_(\"tag '%s' already exists\"), tag);\n \n+\topt.message = msg.given || msgfile;\n+\n \tif (annotate)\n-\t\tcreate_tag(object, tag, &buf, msg.given || msgfile,\n-\t\t\t   sign, prev, object);\n+\t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n \n \tlock = lock_any_ref_for_update(ref.buf, prev, 0);\n \tif (!lock)\n-- \n1.7.7.2\n"},{"id":"179452","messageId":"7vipmmibx4.fsf@alter.siamese.dyndns.org","threadId":"28935","inReplyTo":"1321307019-5557-1-git-send-email-kirill@shutemov.name","subject":"Re: [PATCH, v2] tag: implement --[no-]strip option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-14T22:20:23Z","receivedAt":"2011-11-14T22:20:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kirill A. Shutemov\" <kirill@shutemov.name> writes:\n\n> From: \"Kirill A. Shutemov\" <kirill@shutemov.name>\n>\n> --strip::\n> \tRemove from tag message lines staring with '#', trailing spaces\n> \tfrom every line and empty lines from the beginning and end.\n> \tEnabled by default. Use --no-strip to overwrite the behaviour.\n>\n> --no-strip is useful if you want to take a tag message as-is, without\n> any stripping.\n\nThat is not a commit log message ;-)\n\n> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index c83cb13..dbb76a6 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -99,6 +99,11 @@ OPTIONS\n>  \tImplies `-a` if none of `-a`, `-s`, or `-u <key-id>`\n>  \tis given.\n>  \n> +--strip::\n> +\tRemove from tag message lines staring with '#', trailing spaces\n> +\tfrom every line and empty lines from the beginning and end.\n> +\tEnabled by default. Use --no-strip to overwrite the behaviour.\n\ns/overwrite/override/; or replace the last sentence with \"With\n`--no-strip`, the tag message given by the user is used as-is\".\n\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 9b6fd95..05a1fd4 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> ...\n> @@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n>  \n>  \t\tif (!is_null_sha1(prev))\n>  \t\t\twrite_tag_body(fd, prev);\n> -\t\telse\n> +\t\telse if (opt->strip)\n>  \t\t\twrite_or_die(fd, _(tag_template), strlen(_(tag_template)));\n\nWhy are you not writing template when no strip is done? (Not an objection\ndisguised as a rhetorical question, but a question).\n\nThe user who typed \"tag -a v1.2.3 HEAD\" that spawns an editor would still\nfind it useful to have commented instructions, once we start filling the\ntemplate with more useful information that is customized for the\nsituation (e.g. \"git show -s --oneline\" output), no?\n\n> @@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \tconst char *object_ref, *tag;\n>  \tstruct ref_lock *lock;\n>  \n> -\tint annotate = 0, sign = 0, force = 0, lines = -1,\n> -\t\tlist = 0, delete = 0, verify = 0;\n> +\tstruct create_tag_options opt = {\n> +\t\t.sign = 0,\n> +\t\t.strip = 1,\n> +\t};\n\nAvoid doing this.  Even though these C99 initializers are nicer and more\nreadable way to write this, we try to be gentle to people with older\ncompilers that do not grok the syntax.\n\nExcept for the above minor nits, the patch basically looks good. Please\nhold onto it and resubmit after 1.7.8 final ships.\n\nThanks.\n"},{"id":"179453","messageId":"20111114223903.GA5751@shutemov.name","threadId":"28935","inReplyTo":"7vipmmibx4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH, v2] tag: implement --[no-]strip option","fromName":"Kirill A. Shutemov","fromEmail":"kirill@shutemov.name","sentAt":"2011-11-14T22:39:03Z","receivedAt":"2011-11-14T22:39:03Z","isPatch":true,"sender":{"key":"kirill@shutemov.name","avatar":"https://avatars.githubusercontent.com/u/588661?v=4"},"body":"On Mon, Nov 14, 2011 at 02:20:23PM -0800, Junio C Hamano wrote:\n> \"Kirill A. Shutemov\" <kirill@shutemov.name> writes:\n> \n> > From: \"Kirill A. Shutemov\" <kirill@shutemov.name>\n> >\n> > --strip::\n> > \tRemove from tag message lines staring with '#', trailing spaces\n> > \tfrom every line and empty lines from the beginning and end.\n> > \tEnabled by default. Use --no-strip to overwrite the behaviour.\n> >\n> > --no-strip is useful if you want to take a tag message as-is, without\n> > any stripping.\n> \n> That is not a commit log message ;-)\n\nOk, I'll fix.\n\n> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n> \n> > diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> > index c83cb13..dbb76a6 100644\n> > --- a/Documentation/git-tag.txt\n> > +++ b/Documentation/git-tag.txt\n> > @@ -99,6 +99,11 @@ OPTIONS\n> >  \tImplies `-a` if none of `-a`, `-s`, or `-u <key-id>`\n> >  \tis given.\n> >  \n> > +--strip::\n> > +\tRemove from tag message lines staring with '#', trailing spaces\n> > +\tfrom every line and empty lines from the beginning and end.\n> > +\tEnabled by default. Use --no-strip to overwrite the behaviour.\n> \n> s/overwrite/override/; or replace the last sentence with \"With\n> `--no-strip`, the tag message given by the user is used as-is\".\n\nOk.\n\n> > diff --git a/builtin/tag.c b/builtin/tag.c\n> > index 9b6fd95..05a1fd4 100644\n> > --- a/builtin/tag.c\n> > +++ b/builtin/tag.c\n> > ...\n> > @@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n> >  \n> >  \t\tif (!is_null_sha1(prev))\n> >  \t\t\twrite_tag_body(fd, prev);\n> > -\t\telse\n> > +\t\telse if (opt->strip)\n> >  \t\t\twrite_or_die(fd, _(tag_template), strlen(_(tag_template)));\n> \n> Why are you not writing template when no strip is done? (Not an objection\n> disguised as a rhetorical question, but a question).\n> \n> The user who typed \"tag -a v1.2.3 HEAD\" that spawns an editor would still\n> find it useful to have commented instructions, once we start filling the\n> template with more useful information that is customized for the\n> situation (e.g. \"git show -s --oneline\" output), no?\n\nYes. But in this case commented instructions will not be stripped and they\nwill go to the message. I think user will be confused.\n\nWe can show show some instructions before spawning the editor. What do\nyou think?\n\n> > @@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> >  \tconst char *object_ref, *tag;\n> >  \tstruct ref_lock *lock;\n> >  \n> > -\tint annotate = 0, sign = 0, force = 0, lines = -1,\n> > -\t\tlist = 0, delete = 0, verify = 0;\n> > +\tstruct create_tag_options opt = {\n> > +\t\t.sign = 0,\n> > +\t\t.strip = 1,\n> > +\t};\n> \n> Avoid doing this.  Even though these C99 initializers are nicer and more\n> readable way to write this, we try to be gentle to people with older\n> compilers that do not grok the syntax.\n\nIt's sad. Do you have a list of compilers which are important for the\nproject?\n\n> Except for the above minor nits, the patch basically looks good. Please\n> hold onto it and resubmit after 1.7.8 final ships.\n\nThanks.\n\n-- \n Kirill A. Shutemov\n"},{"id":"179459","messageId":"7vehxai9w5.fsf@alter.siamese.dyndns.org","threadId":"28935","inReplyTo":"20111114223903.GA5751@shutemov.name","subject":"Re: [PATCH, v2] tag: implement --[no-]strip option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-14T23:04:10Z","receivedAt":"2011-11-14T23:04:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kirill A. Shutemov\" <kirill@shutemov.name> writes:\n\n>> > @@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n>> >  \n>> >  \t\tif (!is_null_sha1(prev))\n>> >  \t\t\twrite_tag_body(fd, prev);\n>> > -\t\telse\n>> > +\t\telse if (opt->strip)\n>> >  \t\t\twrite_or_die(fd, _(tag_template), strlen(_(tag_template)));\n>> \n>> Why are you not writing template when no strip is done? (Not an objection\n>> disguised as a rhetorical question, but a question).\n>> \n>> The user who typed \"tag -a v1.2.3 HEAD\" that spawns an editor would still\n>> find it useful to have commented instructions, once we start filling the\n>> template with more useful information that is customized for the\n>> situation (e.g. \"git show -s --oneline\" output), no?\n>\n> Yes. But in this case commented instructions will not be stripped and they\n> will go to the message. I think user will be confused.\n>\n> We can show show some instructions before spawning the editor. What do\n> you think?\n\nMy knee-jerk reaction is that it would be worse than what your patch\ndoes. I'd say we'd start from your patch and see how users of 'next'\nreacts while the topic is cooking.\n"},{"id":"179478","messageId":"4EC209C7.6090805@viscovery.net","threadId":"28935","inReplyTo":"1321307019-5557-1-git-send-email-kirill@shutemov.name","subject":"Re: [PATCH, v2] tag: implement --[no-]strip option","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-11-15T06:42:15Z","receivedAt":"2011-11-15T06:42:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/14/2011 22:43, schrieb Kirill A. Shutemov:\n> From: \"Kirill A. Shutemov\" <kirill@shutemov.name>\n> \n> --strip::\n> \tRemove from tag message lines staring with '#', trailing spaces\n\ns/staring/starting/\n\n> \tfrom every line and empty lines from the beginning and end.\n> \tEnabled by default. Use --no-strip to overwrite the behaviour.\n> \n> --no-strip is useful if you want to take a tag message as-is, without\n> any stripping.\n\nI would like to know why this is useful. Tag messages are for human\nconsumption. What benefit is it that whitespace is not stripped? Why are\nlines starting with '#' so important that they need to stay in the tag\nmessage?\n\n-- Hannes\n"},{"id":"179480","messageId":"7vy5vhx3an.fsf@alter.siamese.dyndns.org","threadId":"28935","inReplyTo":"4EC209C7.6090805@viscovery.net","subject":"Re: [PATCH, v2] tag: implement --[no-]strip option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-15T07:17:36Z","receivedAt":"2011-11-15T07:17:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Am 11/14/2011 22:43, schrieb Kirill A. Shutemov:\n>> From: \"Kirill A. Shutemov\" <kirill@shutemov.name>\n>> \n>> --strip::\n>> \tRemove from tag message lines staring with '#', trailing spaces\n>\n> s/staring/starting/\n>\n>> \tfrom every line and empty lines from the beginning and end.\n>> \tEnabled by default. Use --no-strip to overwrite the behaviour.\n>> \n>> --no-strip is useful if you want to take a tag message as-is, without\n>> any stripping.\n>\n> I would like to know why this is useful. Tag messages are for human\n> consumption. What benefit is it that whitespace is not stripped? Why are\n> lines starting with '#' so important that they need to stay in the tag\n> message?\n\nA conversion from foreign SCM comes to mind, but that is somewhat an\nunfair comment, given that I know by heart how stripspace works and the\nabove documentation patch incorrectly describes what it does.\n\nBesides stripping \"# comment\", stripspace collapses consecutive blank\nlines, removes trailing blank lines and trailing whitespaces at the end of\nlines.\n"}]}