Re: [PATCH] tag: support --sort=version
On Fri, Feb 21, 2014 at 06:58:16PM +0700, Duy Nguyen wrote: --sort=[-][comparison:]field [...] Why not reversed order? So its syntax could be [ - ] FIELD [ : [ version | v ] ] It fits better to current f-e-r syntax where modifiers are after the colon. And it avoids the possibility that someone adds field version and we can't tell what version is what. I find my version a bit more obvious, for two reasons: 1. version here is not a modifier of the field name, it is a modifier of the sort. You cannot use it in non-sort contexts (like --format), and you cannot order it like other modifiers (you cannot say refname:version:short, only refname:short:version). 2. There are actually two sort-modifiers: - for ordering, and then a comparator. In your proposal, they are split, whereas in mine, they are next to each other. -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 --sort=version
On Sat, Feb 22, 2014 at 2:59 PM, Jeff King p...@peff.net wrote: On Fri, Feb 21, 2014 at 06:58:16PM +0700, Duy Nguyen wrote: --sort=[-][comparison:]field [...] Why not reversed order? So its syntax could be [ - ] FIELD [ : [ version | v ] ] It fits better to current f-e-r syntax where modifiers are after the colon. And it avoids the possibility that someone adds field version and we can't tell what version is what. I find my version a bit more obvious, for two reasons: 1. version here is not a modifier of the field name, it is a modifier of the sort. You cannot use it in non-sort contexts (like --format), and you cannot order it like other modifiers (you cannot say refname:version:short, only refname:short:version). Or you can read it like type cast this field as a version, where sorting is affected but formatting, not so much. So you can specify it with --format (but it's no-op, unless we find a fancy way to color versions). I don't see a problem with accepting both refname:version:short and refname:short:version in future for-each-ref either. It will be the first time we accept multiple modifiers though. 2. There are actually two sort-modifiers: - for ordering, and then a comparator. In your proposal, they are split, whereas in mine, they are next to each other. -- Duy -- 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 --sort=version
On Fri, Feb 21, 2014 at 3:43 AM, Jeff King p...@peff.net wrote: I think I actually prefer the full word version, as you have already. It's clear what it means, and we can extend the syntax generally to: Agreed. It's hard to find a letter that reminds you about version. --sort=[-][comparison:]field like: --sort=-version:subject for descending version-sort by subject. And then as a special-case convenience, make version without a field default to version:refname. There's no ambiguity because the set of comparison names and field-names is fixed, and we know there is no overlap. If want to, we can _also_ give a one-letter abbreviation to the comparison field, like: --sort=v:subject but that is not necessary. Why not reversed order? So its syntax could be [ - ] FIELD [ : [ version | v ] ] It fits better to current f-e-r syntax where modifiers are after the colon. And it avoids the possibility that someone adds field version and we can't tell what version is what. -- Duy -- 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 --sort=version
Duy Nguyen pclo...@gmail.com writes: Why not reversed order? So its syntax could be [ - ] FIELD [ : [ version | v ] ] It fits better to current f-e-r syntax where modifiers are after the colon. And it avoids the possibility that someone adds field version and we can't tell what version is what. I think that reads well. -- 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 --sort=version
On Wed, Feb 19, 2014 at 09:19:24PM +0700, Duy Nguyen wrote: We don't need to do any of that immediately. This is mostly just me thinking aloud, to make sure we do not paint ourselves into a corner compatibility-wise. Good thinking. If you plan to use the exact same sort syntax f-e-r uses now, pick a letter (the dot I used above is probably not the best), I'll rewrite this patch to use the same syntax. I think I actually prefer the full word version, as you have already. It's clear what it means, and we can extend the syntax generally to: --sort=[-][comparison:]field like: --sort=-version:subject for descending version-sort by subject. And then as a special-case convenience, make version without a field default to version:refname. There's no ambiguity because the set of comparison names and field-names is fixed, and we know there is no overlap. If want to, we can _also_ give a one-letter abbreviation to the comparison field, like: --sort=v:subject but that is not necessary. So I think your patch is fine as-is. It is perhaps a little funny to start with the special case and only implement the general case later, but: 1. We would want the special case eventually, because it is the most natural thing to type, and pretty clearly the most common use-case. 2. We may not ever even end up with the general case. This is just about making sure that if we _do_ add it, that it fits syntactically with --sort=version. -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
[PATCH] tag: support --sort=version
--sort=version sorts tags as versions. GNU extension's strverscmp is
used and no real compat implementation is provided so this is Linux only.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
I didn't know that coreutils' sort is simply a wrapper of strverscmp.
With that GNU extension, implementing --sort is easy. Mac and Windows
are welcome to reimplement strverscmp (of rip it off glibc).
Documentation/git-tag.txt | 4
builtin/tag.c | 49 ++-
git-compat-util.h | 7 +++
3 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 404257d..fc23dc0 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -95,6 +95,10 @@ OPTIONS
using fnmatch(3)). Multiple patterns may be given; if any of
them matches, the tag is shown.
+--sort=type::
+ Sort in a specific order. Supported type is version. Prepend
+ - to revert sort order.
+
--column[=options]::
--no-column::
Display tag listing in columns. See configuration variable
diff --git a/builtin/tag.c b/builtin/tag.c
index 74d3780..db3567b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,8 @@ static const char * const git_tag_usage[] = {
struct tag_filter {
const char **patterns;
int lines;
+ int version_sort;
+ struct string_list tags;
struct commit_list *with_commit;
};
@@ -166,7 +168,10 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
return 0;
if (!filter-lines) {
- printf(%s\n, refname);
+ if (filter-version_sort)
+ string_list_append(filter-tags, refname);
+ else
+ printf(%s\n, refname);
return 0;
}
printf(%-15s , refname);
@@ -177,17 +182,38 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
return 0;
}
+static int sort_by_version(const void *a_, const void *b_)
+{
+ const struct string_list_item *a = a_;
+ const struct string_list_item *b = b_;
+ return strverscmp(a-string, b-string);
+}
+
static int list_tags(const char **patterns, int lines,
- struct commit_list *with_commit)
+struct commit_list *with_commit, int version_sort)
{
struct tag_filter filter;
filter.patterns = patterns;
filter.lines = lines;
+ filter.version_sort = version_sort;
filter.with_commit = with_commit;
+ memset(filter.tags, 0, sizeof(filter.tags));
+ filter.tags.strdup_strings = 1;
for_each_tag_ref(show_reference, (void *) filter);
-
+ if (version_sort) {
+ int i;
+ qsort(filter.tags.items, filter.tags.nr,
+ sizeof(struct string_list_item), sort_by_version);
+ if (version_sort 0)
+ for (i = 0; i filter.tags.nr; i++)
+ printf(%s\n, filter.tags.items[i].string);
+ else
+ for (i = filter.tags.nr - 1; i = 0; i--)
+ printf(%s\n, filter.tags.items[i].string);
+ string_list_clear(filter.tags, 0);
+ }
return 0;
}
@@ -437,7 +463,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
- int cmdmode = 0;
+ int cmdmode = 0, version_sort = 0;
+ const char *sort = NULL;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
@@ -462,6 +489,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
N_(use another key to sign the tag)),
OPT__FORCE(force, N_(replace the tag if exists)),
OPT_COLUMN(0, column, colopts, N_(show tag list in
columns)),
+ OPT_STRING(0, sort, sort, N_(type), N_(sort tags)),
OPT_GROUP(N_(Tag listing options)),
{
@@ -509,7 +537,18 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
copts.padding = 2;
run_column_filter(colopts, copts);
}
- ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit);
+ if (sort) {
+ if (!strcmp(sort, version))
+ version_sort = 1;
+ else if (!strcmp(sort, -version))
+ version_sort = -1;
+ else
+ die(_(unsupported sort type %s), sort);
+
Re: [PATCH] tag: support --sort=version
On Wed, Feb 19, 2014 at 08:39:27PM +0700, Nguyễn Thái Ngọc Duy wrote: --sort=version sorts tags as versions. GNU extension's strverscmp is used and no real compat implementation is provided so this is Linux only. Sounds like a good goal. I wonder, if we were to merge the for-each-ref and tag implementations[1], how this would integrate with for-each-ref's sorting. It can sort on arbitrary fields like --sort=-*authordate. I think given the syntax you provide, this would fall out naturally as just another key (albeit a slightly magical one, as it is building on the %(refname:short) field but using a different comparator). Would we ever want to version-sort any other field? Perhaps %(content:subject) for a tag? I'm not sure what would be the most natural way to specify that. Maybe --sort=version:content:subject, with just --version as a synonym for version:refname:short. We don't need to do any of that immediately. This is mostly just me thinking aloud, to make sure we do not paint ourselves into a corner compatibility-wise. -Peff [1] I have patches which I really need to polish and send out that combine the ref-selection code (so tag, branch, and for-each-ref all know --merged, --contains, etc). I'd really like to combine the sorting and formatting code, too, so everybody can use --sort and --format with all of the associated fields. -- 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 --sort=version
On Wed, Feb 19, 2014 at 9:09 PM, Jeff King p...@peff.net wrote: On Wed, Feb 19, 2014 at 08:39:27PM +0700, Nguyễn Thái Ngọc Duy wrote: --sort=version sorts tags as versions. GNU extension's strverscmp is used and no real compat implementation is provided so this is Linux only. Sounds like a good goal. I wonder, if we were to merge the for-each-ref and tag implementations[1], how this would integrate with for-each-ref's sorting. It can sort on arbitrary fields like --sort=-*authordate. I think given the syntax you provide, this would fall out naturally as just another key (albeit a slightly magical one, as it is building on the %(refname:short) field but using a different comparator). Would we ever want to version-sort any other field? Perhaps %(content:subject) for a tag? I'm not sure what would be the most natural way to specify that. Maybe --sort=version:content:subject, with just --version as a synonym for version:refname:short. If f-e-r and tag share code and syntax then we could use another letter for this sort type (like we use '-' to reverse sort order). I think that would make the implementation easier as well. So we could have --sort=.refname for version sorting refname, --sort=-.refname for reversed version sort, and sort=-refname for reverse alphabetical sort. We don't need to do any of that immediately. This is mostly just me thinking aloud, to make sure we do not paint ourselves into a corner compatibility-wise. Good thinking. If you plan to use the exact same sort syntax f-e-r uses now, pick a letter (the dot I used above is probably not the best), I'll rewrite this patch to use the same syntax. -Peff [1] I have patches which I really need to polish and send out that combine the ref-selection code (so tag, branch, and for-each-ref all know --merged, --contains, etc). I'd really like to combine the sorting and formatting code, too, so everybody can use --sort and --format with all of the associated fields. -- Duy -- 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 --sort=version
On Wed, Feb 19, 2014 at 8:39 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: --sort=version sorts tags as versions. GNU extension's strverscmp is used and no real compat implementation is provided so this is Linux only. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 404257d..fc23dc0 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -95,6 +95,10 @@ OPTIONS using fnmatch(3)). Multiple patterns may be given; if any of them matches, the tag is shown. +--sort=type:: + Sort in a specific order. Supported type is version. Prepend + - to revert sort order. s/revert/reverse/ + --column[=options]:: --no-column:: Display tag listing in columns. See configuration variable -- 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
