{"thread":{"id":"37087","subject":"[PATCH] tag: support configuring --sort via .gitconfig","startedAt":"2014-07-09T21:21:00Z","lastAt":"2014-07-09T22:15:06Z","messageCount":4,"participants":["Jacob Keller","Jeff King","Keller, Jacob E"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"245658","messageId":"1404940860-4004-1-git-send-email-jacob.e.keller@intel.com","threadId":"37087","inReplyTo":null,"subject":"[PATCH] tag: support configuring --sort via .gitconfig","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2014-07-09T21:21:00Z","receivedAt":"2014-07-09T21:21:00Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"Add support for configuring default sort ordering for git tags. Command\nline option will override this configured value, using the exact same\nsyntax.\n\nCc: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nCc: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jacob Keller <jacob.e.keller@intel.com>\n---\n Documentation/config.txt  |  7 +++++++\n Documentation/git-tag.txt |  3 ++-\n builtin/tag.c             | 47 ++++++++++++++++++++++++++++++++++++++++-------\n t/t7004-tag.sh            | 21 +++++++++++++++++++++\n 4 files changed, 70 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1d718bdb9662..0152582445b0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2354,6 +2354,13 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.sort::\n+\tThis variable is used to control the sort ordering of tags when\n+\tdisplayed via linkgit:git-tag[5]. The current supported types are\n+\t\"refname\" for lexicographic order (default) or \"version:refname\" or\n+\t\"v:refname\" for tag names treated as versions. You may prepend a \"-\" to\n+\treverse the sort ordering.\n+\n tar.umask::\n \tThis variable can be used to restrict the permission bits of\n \ttar archive entries.  The default is 0002, which turns off the\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex b424a1bc48bb..b338260f9f63 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -99,7 +99,7 @@ OPTIONS\n \tSort in a specific order. Supported type is \"refname\"\n \t(lexicographic order), \"version:refname\" or \"v:refname\" (tag\n \tnames are treated as versions). Prepend \"-\" to reverse sort\n-\torder.\n+\torder. See configuration variable tag.sort for more details.\n \n --column[=<options>]::\n --no-column::\n@@ -317,6 +317,7 @@ include::date-formats.txt[]\n SEE ALSO\n --------\n linkgit:git-check-ref-format[1].\n+linkgit:git-config[1].\n \n GIT\n ---\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ef765563388c..e8ada8b716ee 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -18,6 +18,8 @@\n #include \"sha1-array.h\"\n #include \"column.h\"\n \n+static char *configured_tag_sort;\n+\n static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]\"),\n \tN_(\"git tag -d <tagname>...\"),\n@@ -346,9 +348,28 @@ static const char tag_template_nocleanup[] =\n \t\"Lines starting with '%c' will be kept; you may remove them\"\n \t\" yourself if you want to.\\n\");\n \n+static void set_configured_tag_sort(const char *key)\n+{\n+\tfree(configured_tag_sort);\n+\tconfigured_tag_sort = xstrdup(key);\n+}\n+\n+static const char *get_configured_tag_sort(void)\n+{\n+\tif (configured_tag_sort)\n+\t\treturn configured_tag_sort;\n+\treturn \"refname\";\n+}\n+\n static int git_tag_config(const char *var, const char *value, void *cb)\n {\n-\tint status = git_gpg_config(var, value, cb);\n+\tint status;\n+\n+\tif (!strcmp(var, \"tag.sort\")) {\n+\t\tset_configured_tag_sort(value);\n+\t}\n+\n+\tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n \tif (starts_with(var, \"column.\"))\n@@ -519,9 +540,9 @@ static int parse_opt_points_at(const struct option *opt __attribute__((unused)),\n \treturn 0;\n }\n \n-static int parse_opt_sort(const struct option *opt, const char *arg, int unset)\n+static int parse_sort_string(const char *arg)\n {\n-\tint *sort = opt->value;\n+\tint sort = 0;\n \tint flags = 0;\n \n \tif (*arg == '-') {\n@@ -529,16 +550,26 @@ static int parse_opt_sort(const struct option *opt, const char *arg, int unset)\n \t\targ++;\n \t}\n \tif (starts_with(arg, \"version:\")) {\n-\t\t*sort = VERCMP_SORT;\n+\t\tsort = VERCMP_SORT;\n \t\targ += 8;\n \t} else if (starts_with(arg, \"v:\")) {\n-\t\t*sort = VERCMP_SORT;\n+\t\tsort = VERCMP_SORT;\n \t\targ += 2;\n \t} else\n-\t\t*sort = STRCMP_SORT;\n+\t\tsort = STRCMP_SORT;\n \tif (strcmp(arg, \"refname\"))\n \t\tdie(_(\"unsupported sort specification %s\"), arg);\n-\t*sort |= flags;\n+\tsort |= flags;\n+\n+\treturn sort;\n+}\n+\n+static int parse_opt_sort(const struct option *opt, const char *arg, int unset)\n+{\n+\tint *sort = opt->value;\n+\n+\t*sort = parse_sort_string(arg);\n+\n \treturn 0;\n }\n \n@@ -606,6 +637,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \tmemset(&opt, 0, sizeof(opt));\n \n+\tsort = parse_sort_string(get_configured_tag_sort());\n+\n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n \tif (keyid) {\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex e4ab0f5b6419..1e8300f6ed7c 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1423,6 +1423,27 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'configured lexical sort' '\n+\tgit config tag.sort \"v:refname\" &&\n+\tgit tag -l \"foo*\" >actual &&\n+\tcat >expect <<EOF &&\n+foo1.3\n+foo1.6\n+foo1.10\n+EOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'option override configured sort' '\n+\tgit tag -l --sort=-refname \"foo*\" >actual &&\n+\tcat >expect <<EOF &&\n+foo1.6\n+foo1.3\n+foo1.10\n+EOF\n+\ttest_cmp expect actual\n+'\n+\n run_with_limited_stack () {\n \t(ulimit -s 64 && \"$@\")\n }\n-- \n2.0.0\n"},{"id":"245665","messageId":"20140709215816.GE25854@sigill.intra.peff.net","threadId":"37087","inReplyTo":"1404940860-4004-1-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH] tag: support configuring --sort via .gitconfig","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-09T21:58:16Z","receivedAt":"2014-07-09T21:58:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:\n\n> Add support for configuring default sort ordering for git tags. Command\n> line option will override this configured value, using the exact same\n> syntax.\n\nThis makes sense, and was something I was expecting to add once tag and\nbranch both learned for-each-ref's sorting code. I don't think there\nwill be any compatibility problems in adding it now, though; the\nultimate interface will be the same (it will just know about more types\nof sorting).\n\n> +tag.sort::\n> +\tThis variable is used to control the sort ordering of tags when\n> +\tdisplayed via linkgit:git-tag[5]. The current supported types are\n> +\t\"refname\" for lexicographic order (default) or \"version:refname\" or\n> +\t\"v:refname\" for tag names treated as versions. You may prepend a \"-\" to\n> +\treverse the sort ordering.\n\nYour link to git-tag[5] should be to git-tag[1], I think. The final\nnumber is the manpage section.\n\nI was going to complain that this is duplicating the sort documentation\nfrom git-tag, but I see you actually moved it from the --sort option to\nhere, and refer back from there to here.\n\nI think I'd prefer it the other way around (and explain this as \"behave\nas if --sort=<value> had been given\"). As the sort options grow, it\nseems likely that we will grow a new section in the git-tag manpage (and\neventually probably feed that content from an include that works for all\nof for-each-ref, branch, and tag).\n\n> +static char *configured_tag_sort;\n\nWhen dealing with a config option that also has a command-line version,\nwe usually forgo this separate storage for the configured value.\nInstead, we just set \"sort\" directly from the config callback (which may\nrequire making it a global so the callback can access it), and then let\nthe command-line parser overwrite it.\n\n-Peff\n"},{"id":"245668","messageId":"1404943640.23510.40.camel@jekeller-desk1.amr.corp.intel.com","threadId":"37087","inReplyTo":"20140709215816.GE25854@sigill.intra.peff.net","subject":"Re: [PATCH] tag: support configuring --sort via .gitconfig","fromName":"Keller, Jacob E","fromEmail":"jacob.e.keller@intel.com","sentAt":"2014-07-09T22:07:20Z","receivedAt":"2014-07-09T22:07:20Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, 2014-07-09 at 17:58 -0400, Jeff King wrote:\n> On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:\n> \n> > Add support for configuring default sort ordering for git tags. Command\n> > line option will override this configured value, using the exact same\n> > syntax.\n> \n> This makes sense, and was something I was expecting to add once tag and\n> branch both learned for-each-ref's sorting code. I don't think there\n> will be any compatibility problems in adding it now, though; the\n> ultimate interface will be the same (it will just know about more types\n> of sorting).\n> \n\nI only noticed the sort recently, and I first tried to add an alias like\n\"tag = tag --sort=v:refname\" but git didn't pick up the alias of the\nname already. I use a lot of version-style tags so I wanted this to be\ndefault. I did notice that the format of the sort parameter was actually\npretty complex, so it seemed that more was intended to be added to it.\n\nThe only main issue would be the location of the sort-configure code,\nbut that is actually easy to move so I don't think it's a big deal. The\nconfiguration interface should remain similar.\n\n> > +tag.sort::\n> > +\tThis variable is used to control the sort ordering of tags when\n> > +\tdisplayed via linkgit:git-tag[5]. The current supported types are\n> > +\t\"refname\" for lexicographic order (default) or \"version:refname\" or\n> > +\t\"v:refname\" for tag names treated as versions. You may prepend a \"-\" to\n> > +\treverse the sort ordering.\n> \n> Your link to git-tag[5] should be to git-tag[1], I think. The final\n> number is the manpage section.\n> \n\nWhich I thought was 5 for the commands? Or is it always [1]?\n\n> I was going to complain that this is duplicating the sort documentation\n> from git-tag, but I see you actually moved it from the --sort option to\n> here, and refer back from there to here.\n> \n\nI actually didn't remove anything from git-tag, I only added the\nreference to git-config. But I can keep from duplicating the\ndocumentation in there. I like the idea of using an included file\nthough.\n\n> I think I'd prefer it the other way around (and explain this as \"behave\n> as if --sort=<value> had been given\"). As the sort options grow, it\n> seems likely that we will grow a new section in the git-tag manpage (and\n> eventually probably feed that content from an include that works for all\n> of for-each-ref, branch, and tag).\n\nyes, I agree this makes more sense.\n\n> \n> > +static char *configured_tag_sort;\n> \n> When dealing with a config option that also has a command-line version,\n> we usually forgo this separate storage for the configured value.\n> Instead, we just set \"sort\" directly from the config callback (which may\n> require making it a global so the callback can access it), and then let\n> the command-line parser overwrite it.\n> \n\nAlright that makes sense. I will send a v2 soon. Thanks for the\ncomments.\n\n> -Peff\n\nRegards,\nJake\n"},{"id":"245669","messageId":"20140709221506.GA27388@sigill.intra.peff.net","threadId":"37087","inReplyTo":"1404943640.23510.40.camel@jekeller-desk1.amr.corp.intel.com","subject":"Re: [PATCH] tag: support configuring --sort via .gitconfig","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-09T22:15:06Z","receivedAt":"2014-07-09T22:15:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 09, 2014 at 10:07:20PM +0000, Keller, Jacob E wrote:\n\n> I only noticed the sort recently, and I first tried to add an alias like\n> \"tag = tag --sort=v:refname\" but git didn't pick up the alias of the\n> name already.\n\nIt was only added recently (v2.0.0). :)\n\nAs you noticed, we do not allow aliases of actual git commands. I think\nit would be nice to notice and complain rather than silently ignoring\nthem, though.\n\n> I use a lot of version-style tags so I wanted this to be default. I\n> did notice that the format of the sort parameter was actually pretty\n> complex, so it seemed that more was intended to be added to it.\n\nYeah, the complexity is in preparation for becoming more flexible. I\nthink we'd consider short options for commonly-used sorts, but were\nwaiting for them to prove their value in the field.\n\n> The only main issue would be the location of the sort-configure code,\n> but that is actually easy to move so I don't think it's a big deal. The\n> configuration interface should remain similar.\n\nAgreed. It's not a problem moving code around. But once a user-facing\nfeature is released, we try to keep compatibility with it. So as long as\nthe config option's format stays the same (or strictly increases in\nfeatures), that is fine.\n\n> > Your link to git-tag[5] should be to git-tag[1], I think. The final\n> > number is the manpage section.\n> \n> Which I thought was 5 for the commands? Or is it always [1]?\n\nCommands are 1. File formats are 5 (so there are some things in section\n5 in git). \"man man\" has more.\n\n> > I was going to complain that this is duplicating the sort documentation\n> > from git-tag, but I see you actually moved it from the --sort option to\n> > here, and refer back from there to here.\n> \n> I actually didn't remove anything from git-tag, I only added the\n> reference to git-config. But I can keep from duplicating the\n> documentation in there. I like the idea of using an included file\n> though.\n\nOh, sorry, I just misread the patch. Then I'll fall back to my original\ncomplaint. :) We should not duplicate, but just refer back to\ngit-tag(1).\n\n> Alright that makes sense. I will send a v2 soon. Thanks for the\n> comments.\n\nYou're welcome. Thanks for working on git (and welcome to the list, I\nthink).\n\n-Peff\n"}]}