[PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jacob Keller
Add support for configuring default sort ordering for git tags. Command
line option will override this configured value, using the exact same
syntax.

Cc: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Cc: Junio C Hamano gits...@pobox.com
Signed-off-by: Jacob Keller jacob.e.kel...@intel.com
---
 Documentation/config.txt  |  7 +++
 Documentation/git-tag.txt |  3 ++-
 builtin/tag.c | 47 ---
 t/t7004-tag.sh| 21 +
 4 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bdb9662..0152582445b0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2354,6 +2354,13 @@ submodule.name.ignore::
--ignore-submodules option. The 'git submodule' commands are not
affected by this setting.
 
+tag.sort::
+   This variable is used to control the sort ordering of tags when
+   displayed via linkgit:git-tag[5]. The current supported types are
+   refname for lexicographic order (default) or version:refname or
+   v:refname for tag names treated as versions. You may prepend a - to
+   reverse the sort ordering.
+
 tar.umask::
This variable can be used to restrict the permission bits of
tar archive entries.  The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index b424a1bc48bb..b338260f9f63 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -99,7 +99,7 @@ OPTIONS
Sort in a specific order. Supported type is refname
(lexicographic order), version:refname or v:refname (tag
names are treated as versions). Prepend - to reverse sort
-   order.
+   order. See configuration variable tag.sort for more details.
 
 --column[=options]::
 --no-column::
@@ -317,6 +317,7 @@ include::date-formats.txt[]
 SEE ALSO
 
 linkgit:git-check-ref-format[1].
+linkgit:git-config[1].
 
 GIT
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index ef765563388c..e8ada8b716ee 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -18,6 +18,8 @@
 #include sha1-array.h
 #include column.h
 
+static char *configured_tag_sort;
+
 static const char * const git_tag_usage[] = {
N_(git tag [-a|-s|-u key-id] [-f] [-m msg|-F file] tagname 
[head]),
N_(git tag -d tagname...),
@@ -346,9 +348,28 @@ static const char tag_template_nocleanup[] =
Lines starting with '%c' will be kept; you may remove them
 yourself if you want to.\n);
 
+static void set_configured_tag_sort(const char *key)
+{
+   free(configured_tag_sort);
+   configured_tag_sort = xstrdup(key);
+}
+
+static const char *get_configured_tag_sort(void)
+{
+   if (configured_tag_sort)
+   return configured_tag_sort;
+   return refname;
+}
+
 static int git_tag_config(const char *var, const char *value, void *cb)
 {
-   int status = git_gpg_config(var, value, cb);
+   int status;
+
+   if (!strcmp(var, tag.sort)) {
+   set_configured_tag_sort(value);
+   }
+
+   status = git_gpg_config(var, value, cb);
if (status)
return status;
if (starts_with(var, column.))
@@ -519,9 +540,9 @@ static int parse_opt_points_at(const struct option *opt 
__attribute__((unused)),
return 0;
 }
 
-static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
+static int parse_sort_string(const char *arg)
 {
-   int *sort = opt-value;
+   int sort = 0;
int flags = 0;
 
if (*arg == '-') {
@@ -529,16 +550,26 @@ static int parse_opt_sort(const struct option *opt, const 
char *arg, int unset)
arg++;
}
if (starts_with(arg, version:)) {
-   *sort = VERCMP_SORT;
+   sort = VERCMP_SORT;
arg += 8;
} else if (starts_with(arg, v:)) {
-   *sort = VERCMP_SORT;
+   sort = VERCMP_SORT;
arg += 2;
} else
-   *sort = STRCMP_SORT;
+   sort = STRCMP_SORT;
if (strcmp(arg, refname))
die(_(unsupported sort specification %s), arg);
-   *sort |= flags;
+   sort |= flags;
+
+   return sort;
+}
+
+static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
+{
+   int *sort = opt-value;
+
+   *sort = parse_sort_string(arg);
+
return 0;
 }
 
@@ -606,6 +637,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
memset(opt, 0, sizeof(opt));
 
+   sort = parse_sort_string(get_configured_tag_sort());
+
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
if (keyid) {
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e4ab0f5b6419..1e8300f6ed7c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1423,6 +1423,27 @@ EOF
test_cmp expect actual
 '
 
+test_expect_success 'configured lexical 

Re: [PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:

 Add support for configuring default sort ordering for git tags. Command
 line option will override this configured value, using the exact same
 syntax.

This makes sense, and was something I was expecting to add once tag and
branch both learned for-each-ref's sorting code. I don't think there
will be any compatibility problems in adding it now, though; the
ultimate interface will be the same (it will just know about more types
of sorting).

 +tag.sort::
 + This variable is used to control the sort ordering of tags when
 + displayed via linkgit:git-tag[5]. The current supported types are
 + refname for lexicographic order (default) or version:refname or
 + v:refname for tag names treated as versions. You may prepend a - to
 + reverse the sort ordering.

Your link to git-tag[5] should be to git-tag[1], I think. The final
number is the manpage section.

I was going to complain that this is duplicating the sort documentation
from git-tag, but I see you actually moved it from the --sort option to
here, and refer back from there to here.

I think I'd prefer it the other way around (and explain this as behave
as if --sort=value had been given). As the sort options grow, it
seems likely that we will grow a new section in the git-tag manpage (and
eventually probably feed that content from an include that works for all
of for-each-ref, branch, and tag).

 +static char *configured_tag_sort;

When dealing with a config option that also has a command-line version,
we usually forgo this separate storage for the configured value.
Instead, we just set sort directly from the config callback (which may
require making it a global so the callback can access it), and then let
the command-line parser overwrite it.

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


Re: [PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Keller, Jacob E
On Wed, 2014-07-09 at 17:58 -0400, Jeff King wrote:
 On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:
 
  Add support for configuring default sort ordering for git tags. Command
  line option will override this configured value, using the exact same
  syntax.
 
 This makes sense, and was something I was expecting to add once tag and
 branch both learned for-each-ref's sorting code. I don't think there
 will be any compatibility problems in adding it now, though; the
 ultimate interface will be the same (it will just know about more types
 of sorting).
 

I only noticed the sort recently, and I first tried to add an alias like
tag = tag --sort=v:refname but git didn't pick up the alias of the
name already. I use a lot of version-style tags so I wanted this to be
default. I did notice that the format of the sort parameter was actually
pretty complex, so it seemed that more was intended to be added to it.

The only main issue would be the location of the sort-configure code,
but that is actually easy to move so I don't think it's a big deal. The
configuration interface should remain similar.

  +tag.sort::
  +   This variable is used to control the sort ordering of tags when
  +   displayed via linkgit:git-tag[5]. The current supported types are
  +   refname for lexicographic order (default) or version:refname or
  +   v:refname for tag names treated as versions. You may prepend a - to
  +   reverse the sort ordering.
 
 Your link to git-tag[5] should be to git-tag[1], I think. The final
 number is the manpage section.
 

Which I thought was 5 for the commands? Or is it always [1]?

 I was going to complain that this is duplicating the sort documentation
 from git-tag, but I see you actually moved it from the --sort option to
 here, and refer back from there to here.
 

I actually didn't remove anything from git-tag, I only added the
reference to git-config. But I can keep from duplicating the
documentation in there. I like the idea of using an included file
though.

 I think I'd prefer it the other way around (and explain this as behave
 as if --sort=value had been given). As the sort options grow, it
 seems likely that we will grow a new section in the git-tag manpage (and
 eventually probably feed that content from an include that works for all
 of for-each-ref, branch, and tag).

yes, I agree this makes more sense.

 
  +static char *configured_tag_sort;
 
 When dealing with a config option that also has a command-line version,
 we usually forgo this separate storage for the configured value.
 Instead, we just set sort directly from the config callback (which may
 require making it a global so the callback can access it), and then let
 the command-line parser overwrite it.
 

Alright that makes sense. I will send a v2 soon. Thanks for the
comments.

 -Peff

Regards,
Jake


Re: [PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 10:07:20PM +, Keller, Jacob E wrote:

 I only noticed the sort recently, and I first tried to add an alias like
 tag = tag --sort=v:refname but git didn't pick up the alias of the
 name already.

It was only added recently (v2.0.0). :)

As you noticed, we do not allow aliases of actual git commands. I think
it would be nice to notice and complain rather than silently ignoring
them, though.

 I use a lot of version-style tags so I wanted this to be default. I
 did notice that the format of the sort parameter was actually pretty
 complex, so it seemed that more was intended to be added to it.

Yeah, the complexity is in preparation for becoming more flexible. I
think we'd consider short options for commonly-used sorts, but were
waiting for them to prove their value in the field.

 The only main issue would be the location of the sort-configure code,
 but that is actually easy to move so I don't think it's a big deal. The
 configuration interface should remain similar.

Agreed. It's not a problem moving code around. But once a user-facing
feature is released, we try to keep compatibility with it. So as long as
the config option's format stays the same (or strictly increases in
features), that is fine.

  Your link to git-tag[5] should be to git-tag[1], I think. The final
  number is the manpage section.
 
 Which I thought was 5 for the commands? Or is it always [1]?

Commands are 1. File formats are 5 (so there are some things in section
5 in git). man man has more.

  I was going to complain that this is duplicating the sort documentation
  from git-tag, but I see you actually moved it from the --sort option to
  here, and refer back from there to here.
 
 I actually didn't remove anything from git-tag, I only added the
 reference to git-config. But I can keep from duplicating the
 documentation in there. I like the idea of using an included file
 though.

Oh, sorry, I just misread the patch. Then I'll fall back to my original
complaint. :) We should not duplicate, but just refer back to
git-tag(1).

 Alright that makes sense. I will send a v2 soon. Thanks for the
 comments.

You're welcome. Thanks for working on git (and welcome to the list, I
think).

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