{"thread":{"id":"9735","subject":"[PATCH] git-tag: Fix -l option to use better shell style globs.","startedAt":"2007-09-01T05:10:09Z","lastAt":"2007-09-01T14:46:30Z","messageCount":6,"participants":["Carlos Rica","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"52123","messageId":"46D8F431.70801@gmail.com","threadId":"9735","inReplyTo":null,"subject":"[PATCH] git-tag: Fix -l option to use better shell style globs.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-09-01T05:10:09Z","receivedAt":"2007-09-01T05:10:09Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"This patch removes certain behaviour of \"git tag -l foo\", currently\nlisting every tag name having \"foo\" as a substring. The same\nthing now could be achieved doing \"git tag -l '*foo*'\".\n\nThis feature was added recently when git-tag.sh got the -n option\nfor showing tag annotations, because that commit also replaced the\nold \"grep pattern\" behaviour with a more preferable \"shell pattern\"\nbehaviour (although slightly modified as you can see).\nThus, the following builtin-tag.c implemented it in order to\nensure that tests were passing unchanged with both programs.\n\nSince common \"shell patterns\" match names with a given substring\n_only_ when * is inserted before and after (as in \"*substring*\"), and\nthe \"plain\" behaviour cannot be achieved easily with the current\nimplementation, this is mostly the right thing to do, in order to\nmake it more flexible and consistent.\n\nTests for \"git tag\" were also changed to reflect this.\n\nSigned-off-by: Carlos Rica \n---\n builtin-tag.c | 11 ++---------\n t/t7004-tag.sh | 20 +++++++++-----------\n 2 files changed, 11 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex d6d38ad..348919c 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -123,22 +123,15 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n static int list_tags(const char *pattern, int lines)\n {\n \tstruct tag_filter filter;\n-\tchar *newpattern;\n\n \tif (pattern == NULL)\n-\t\tpattern = \"\";\n+\t\tpattern = \"*\";\n\n-\t/* prepend/append * to the shell pattern: */\n-\tnewpattern = xmalloc(strlen(pattern) + 3);\n-\tsprintf(newpattern, \"*%s*\", pattern);\n-\n-\tfilter.pattern = newpattern;\n+\tfilter.pattern = pattern;\n \tfilter.lines = lines;\n\n \tfor_each_tag_ref(show_reference, (void *) &filter);\n\n-\tfree(newpattern);\n-\n \treturn 0;\n }\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex c4fa446..606d4f2 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -185,18 +185,17 @@ cba\n EOF\n test_expect_success \\\n \t'listing tags with substring as pattern must print those matching' '\n-\tgit-tag -l a > actual &&\n+\tgit-tag -l \"*a*\" > actual &&\n \tgit diff expect actual\n '\n\n cat >expect < actual &&\n+\t'listing tags with a suffix as pattern must print those matching' '\n+\tgit-tag -l \"*.1\" > actual &&\n \tgit diff expect actual\n '\n\n@@ -205,37 +204,36 @@ t210\n t211\n EOF\n test_expect_success \\\n-\t'listing tags with substring as pattern must print those matching' '\n-\tgit-tag -l t21 > actual &&\n+\t'listing tags with a prefix as pattern must print those matching' '\n+\tgit-tag -l \"t21*\" > actual &&\n \tgit diff expect actual\n '\n\n cat >expect < actual &&\n \tgit diff expect actual\n '\n\n cat >expect < actual &&\n \tgit diff expect actual\n '\n\n cat >expect < actual &&\n+\tgit-tag -l \"v1.?.?\" > actual &&\n \tgit diff expect actual\n '\n\n-- \n1.5.0\n"},{"id":"52124","messageId":"20070901053158.GF18160@spearce.org","threadId":"9735","inReplyTo":"46D8F431.70801@gmail.com","subject":"Re: [PATCH] git-tag: Fix -l option to use better shell style globs.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-01T05:31:58Z","receivedAt":"2007-09-01T05:31:58Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Carlos Rica wrote:\n> This patch removes certain behaviour of \"git tag -l foo\", currently\n> listing every tag name having \"foo\" as a substring. The same\n> thing now could be achieved doing \"git tag -l '*foo*'\".\n\nEven though this is a behavior change, I think its the right thing\nto do. The current behavior of searching \"*$arg*\" is downright\nannoying and not what most users would expect I think, especially\nwhen tools like for-each-ref don't do that.\n\nThen again, I do \"*$arg*\" in git-gui's revision selection widget.\nBut there its immediately obvious what is happening and anyone I\nhave talked[*1*] to prefers it that way.\n\n\n*1*: Disclaimer: people I talked to has thus far been limited to\n day-job coworkers.\n \n-- \nShawn.\n"},{"id":"52125","messageId":"7vd4x3uep7.fsf@gitster.siamese.dyndns.org","threadId":"9735","inReplyTo":"20070901053158.GF18160@spearce.org","subject":"Re: [PATCH] git-tag: Fix -l option to use better shell style globs.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-01T05:39:32Z","receivedAt":"2007-09-01T05:39:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Carlos Rica wrote:\n>> This patch removes certain behaviour of \"git tag -l foo\", currently\n>> listing every tag name having \"foo\" as a substring. The same\n>> thing now could be achieved doing \"git tag -l '*foo*'\".\n>\n> Even though this is a behavior change, I think its the right thing\n> to do. The current behavior of searching \"*$arg*\" is downright\n> annoying.\n\nYes, I concur. It is very annoying that \"git tag -l gui\"\nmatches \"gitgui-0.7.0\".\n\nLet's fix this.\n"},{"id":"52128","messageId":"7v8x7qvrka.fsf@gitster.siamese.dyndns.org","threadId":"9735","inReplyTo":"46D8F431.70801@gmail.com","subject":"Re: [PATCH] git-tag: Fix -l option to use better shell style globs.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-01T06:16:21Z","receivedAt":"2007-09-01T06:16:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Rica writes:\n\n> \tif (pattern == NULL)\n> -\t\tpattern = \"\";\n> +\t\tpattern = \"*\";\n>\n> -\t/* prepend/append * to the shell pattern: */\n> -\tnewpattern = xmalloc(strlen(pattern) + 3);\n> -\tsprintf(newpattern, \"*%s*\", pattern);\n> -\n> -\tfilter.pattern = newpattern;\n> +\tfilter.pattern = pattern;\n> \tfilter.lines = lines;\n>\n> \tfor_each_tag_ref(show_reference, (void *) &filter);\n\nI think it is conceptually simpler on the show_reference side to\nallow (filter.pattern == NULL) and say:\n\n\tif (!filter->pattern || !fnmatch(filter->pattern, refname, 0)) {\n \t... show that ref ...\n\t}\n\nIt is not such a big deal now you do not do newpattern\nallocation anymore, so I'll apply the patch as is.\n"},{"id":"52145","messageId":"1b46aba20709010733x45960f00g8732f6a1af363768@mail.gmail.com","threadId":"9735","inReplyTo":"7v8x7qvrka.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-tag: Fix -l option to use better shell style globs.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-09-01T14:33:50Z","receivedAt":"2007-09-01T14:33:50Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"2007/9/1, Junio C Hamano :\n> I think it is conceptually simpler on the show_reference side to\n> allow (filter.pattern == NULL) and say:\n>\n> if (!filter->pattern || !fnmatch(filter->pattern, refname, 0)) {\n> ... show that ref ...\n> }\n>\n> It is not such a big deal now you do not do newpattern\n> allocation anymore, so I'll apply the patch as is.\n>\n\nYou are right. I changed the patch also to reflect this.\nI cannot send a reply using my email client now, so I send it\nattached to this response to avoid gmail breaks in long lines.\n\n\nFrom d10170c5a2c3cf1fc6ad270e9a2c82ff29a98871 Mon Sep 17 00:00:00 2001\nFrom: Carlos Rica \nDate: Sat, 1 Sep 2007 06:58:40 +0200\nSubject: [PATCH] git-tag: Fix -l option to use better shell style globs.\n\nThis patch removes the behaviour of \"git tag -l foo\", currently\nlisting every tag name having \"foo\" as a substring. The same\nthing now could be achieved doing \"git tag -l '*foo*'\".\n\nThe \"feature\" was added recently when git-tag.sh got the -n option\nfor showing tag annotations, because that commit also replaced the\nold \"grep pattern\" behaviour with a more preferable \"shell pattern\"\nbehaviour (although slightly modified as you can see).\nThus, the following builtin-tag.c implemented it in order to\nensure that tests were passing unchanged with both programs.\n\nSince common \"shell patterns\" match names with a given substring\n_only_ when * is inserted before and after (as in \"*substr*\"), and\nthe \"plain\" behaviour cannot be achieved easily with the current\nimplementation, this is mostly the right thing to do, in order to\nmake it more flexible and consistent.\n\nTests for \"git tag\" were also changed to reflect this.\n\nSigned-off-by: Carlos Rica \n---\n builtin-tag.c | 14 ++------------\n t/t7004-tag.sh | 20 +++++++++-----------\n 2 files changed, 11 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex d6d38ad..0e01b98 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -75,7 +75,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n {\n \tstruct tag_filter *filter = cb_data;\n \n-\tif (!fnmatch(filter->pattern, refname, 0)) {\n+\tif (!filter->pattern || !fnmatch(filter->pattern, refname, 0)) {\n \t\tint i;\n \t\tunsigned long size;\n \t\tenum object_type type;\n@@ -123,22 +123,12 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n static int list_tags(const char *pattern, int lines)\n {\n \tstruct tag_filter filter;\n-\tchar *newpattern;\n \n-\tif (pattern == NULL)\n-\t\tpattern = \"\";\n-\n-\t/* prepend/append * to the shell pattern: */\n-\tnewpattern = xmalloc(strlen(pattern) + 3);\n-\tsprintf(newpattern, \"*%s*\", pattern);\n-\n-\tfilter.pattern = newpattern;\n+\tfilter.pattern = pattern;\n \tfilter.lines = lines;\n \n \tfor_each_tag_ref(show_reference, (void *) &filter);\n \n-\tfree(newpattern);\n-\n \treturn 0;\n }\n \ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex c4fa446..606d4f2 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -185,18 +185,17 @@ cba\n EOF\n test_expect_success \\\n \t'listing tags with substring as pattern must print those matching' '\n-\tgit-tag -l a > actual &&\n+\tgit-tag -l \"*a*\" > actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n+\t'listing tags with a suffix as pattern must print those matching' '\n+\tgit-tag -l \"*.1\" > actual &&\n \tgit diff expect actual\n '\n \n@@ -205,37 +204,36 @@ t210\n t211\n EOF\n test_expect_success \\\n-\t'listing tags with substring as pattern must print those matching' '\n-\tgit-tag -l t21 > actual &&\n+\t'listing tags with a prefix as pattern must print those matching' '\n+\tgit-tag -l \"t21*\" > actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n+\tgit-tag -l \"v1.?.?\" > actual &&\n \tgit diff expect actual\n '\n \n-- \n1.5.0\n\n"},{"id":"52146","messageId":"1b46aba20709010746h204bfbbbtc0d83336f76570c3@mail.gmail.com","threadId":"9735","inReplyTo":"1b46aba20709010733x45960f00g8732f6a1af363768@mail.gmail.com","subject":"Re: [PATCH] git-tag: Fix -l option to use better shell style globs.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-09-01T14:46:30Z","receivedAt":"2007-09-01T14:46:30Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"2007/9/1, Carlos Rica :\n> You are right. I changed the patch also to reflect this.\n\nSorry, it seems that it also has a different \"version\" of\nmy comments for the commit, I have the good/bad habit of\nreading again the comments in patches (and changing them)\nbefore send it out to the list...\n\n6c6\nThis patch removes certain behaviour of \"git tag -l foo\", currently\nThis patch removes the behaviour of \"git tag -l foo\", currently\n\n10c10\nThis feature was added recently when git-tag.sh got the -n option\nThe \"feature\" was added recently when git-tag.sh got the -n option\n\n18c18\n_only_ when * is inserted before and after (as in \"*substring*\"), and\n_only_ when * is inserted before and after (as in \"*substr*\"), and\n\n\nFrom ea881fdfd39ce5fb5aaa273d90c6bf8ed5f8d9b0 Mon Sep 17 00:00:00 2001\nFrom: Carlos Rica \nDate: Sat, 1 Sep 2007 06:58:40 +0200\nSubject: [PATCH] git-tag: Fix -l option to use better shell style globs.\n\nThis patch removes certain behaviour of \"git tag -l foo\", currently\nlisting every tag name having \"foo\" as a substring. The same\nthing now could be achieved doing \"git tag -l '*foo*'\".\n\nThis feature was added recently when git-tag.sh got the -n option\nfor showing tag annotations, because that commit also replaced the\nold \"grep pattern\" behaviour with a more preferable \"shell pattern\"\nbehaviour (although slightly modified as you can see).\nThus, the following builtin-tag.c implemented it in order to\nensure that tests were passing unchanged with both programs.\n\nSince common \"shell patterns\" match names with a given substring\n_only_ when * is inserted before and after (as in \"*substring*\"), and\nthe \"plain\" behaviour cannot be achieved easily with the current\nimplementation, this is mostly the right thing to do, in order to\nmake it more flexible and consistent.\n\nTests for \"git tag\" were also changed to reflect this.\n\nSigned-off-by: Carlos Rica \n---\n builtin-tag.c | 14 ++------------\n t/t7004-tag.sh | 20 +++++++++-----------\n 2 files changed, 11 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex d6d38ad..0e01b98 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -75,7 +75,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n {\n \tstruct tag_filter *filter = cb_data;\n \n-\tif (!fnmatch(filter->pattern, refname, 0)) {\n+\tif (!filter->pattern || !fnmatch(filter->pattern, refname, 0)) {\n \t\tint i;\n \t\tunsigned long size;\n \t\tenum object_type type;\n@@ -123,22 +123,12 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n static int list_tags(const char *pattern, int lines)\n {\n \tstruct tag_filter filter;\n-\tchar *newpattern;\n \n-\tif (pattern == NULL)\n-\t\tpattern = \"\";\n-\n-\t/* prepend/append * to the shell pattern: */\n-\tnewpattern = xmalloc(strlen(pattern) + 3);\n-\tsprintf(newpattern, \"*%s*\", pattern);\n-\n-\tfilter.pattern = newpattern;\n+\tfilter.pattern = pattern;\n \tfilter.lines = lines;\n \n \tfor_each_tag_ref(show_reference, (void *) &filter);\n \n-\tfree(newpattern);\n-\n \treturn 0;\n }\n \ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex c4fa446..606d4f2 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -185,18 +185,17 @@ cba\n EOF\n test_expect_success \\\n \t'listing tags with substring as pattern must print those matching' '\n-\tgit-tag -l a > actual &&\n+\tgit-tag -l \"*a*\" > actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n+\t'listing tags with a suffix as pattern must print those matching' '\n+\tgit-tag -l \"*.1\" > actual &&\n \tgit diff expect actual\n '\n \n@@ -205,37 +204,36 @@ t210\n t211\n EOF\n test_expect_success \\\n-\t'listing tags with substring as pattern must print those matching' '\n-\tgit-tag -l t21 > actual &&\n+\t'listing tags with a prefix as pattern must print those matching' '\n+\tgit-tag -l \"t21*\" > actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n \tgit diff expect actual\n '\n \n cat >expect < actual &&\n+\tgit-tag -l \"v1.?.?\" > actual &&\n \tgit diff expect actual\n '\n \n-- \n1.5.0\n\n"}]}