Re: [PATCH] tag: support mixing --sort=<spec> and -n
From: Jacob Keller <hidden>
Date: 2016-06-15 23:06:28
On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela [off-list ref] wrote:
Allow -n and --sort=version:refname to be used together instead of failing with: fatal: --sort and -n are incompatible Signed-off-by: Rudy Matela <redacted>
Nice! I've been wondering about this one for a while. Especially since implementing tag.sort configuration which made -n not work at all. Note that it may be worth rebasing this on top of Karthik's part tag to use ref-filter series, since I think there will be plenty of merge conflicts there... I also suggest adding some tests for this, as sort and -n didn't even have a test before, but now we could add a test that shows it works.
quoted hunk ↗ jump to hunk
--- builtin/tag.c | 64 ++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 29 deletions(-)diff --git a/builtin/tag.c b/builtin/tag.c index cccca99..cdcb373 100644 --- a/builtin/tag.c +++ b/builtin/tag.c@@ -176,13 +176,19 @@ static enum contains_result contains(struct commit *candidate, return contains_test(candidate, want); } -static void show_tag_lines(const struct object_id *oid, int lines) +static char *get_tag_lines(const struct object_id *oid, int lines) { int i; unsigned long size; enum object_type type; char *buf, *sp, *eol; size_t len; + struct strbuf sb; + + if (!lines) + return NULL; + + strbuf_init(&sb,0); buf = read_sha1_file(oid->hash, &type, &size); if (!buf)@@ -203,20 +209,21 @@ static void show_tag_lines(const struct object_id *oid, int lines) size = parse_signature(buf, size); for (i = 0, sp += 2; i < lines && sp < buf + size; i++) { if (i) - printf("\n "); + strbuf_addstr(&sb,"\n "); eol = memchr(sp, '\n', size - (sp - buf)); len = eol ? eol - sp : size - (sp - buf); - fwrite(sp, len, 1, stdout); + strbuf_add(&sb, sp, len); if (!eol) break; sp = eol + 1; } free_return: free(buf); + return strbuf_detach(&sb, NULL); } -static int show_reference(const char *refname, const struct object_id *oid, - int flag, void *cb_data) +static int get_reference_and_tag_lines(const char *refname, const struct object_id *oid, + int flag, void *cb_data) { struct tag_filter *filter = cb_data;@@ -234,16 +241,8 @@ static int show_reference(const char *refname, const struct object_id *oid, if (points_at.nr && !match_points_at(refname, oid->hash)) return 0; - if (!filter->lines) { - if (filter->sort) - string_list_append(&filter->tags, refname); - else - printf("%s\n", refname); - return 0; - } - printf("%-15s ", refname); - show_tag_lines(oid, filter->lines); - putchar('\n'); + string_list_append(&filter->tags, refname)->util = + get_tag_lines(oid, filter->lines); } return 0;@@ -260,6 +259,7 @@ static int list_tags(const char **patterns, int lines, struct commit_list *with_commit, int sort) { struct tag_filter filter; + int i; filter.patterns = patterns; filter.lines = lines;@@ -268,20 +268,28 @@ static int list_tags(const char **patterns, int lines, memset(&filter.tags, 0, sizeof(filter.tags)); filter.tags.strdup_strings = 1; - for_each_tag_ref(show_reference, (void *)&filter); - if (sort) { - int i; - if ((sort & SORT_MASK) == VERCMP_SORT) - qsort(filter.tags.items, filter.tags.nr, - sizeof(struct string_list_item), sort_by_version); - if (sort & REVERSE_SORT) - for (i = filter.tags.nr - 1; i >= 0; i--) + for_each_tag_ref(get_reference_and_tag_lines, (void *)&filter); + if ((sort & SORT_MASK) == VERCMP_SORT) + qsort(filter.tags.items, filter.tags.nr, + sizeof(struct string_list_item), sort_by_version);
Nice. So we store the items and sort by the lines.
+ if (sort & REVERSE_SORT)
+ for (i = filter.tags.nr - 1; i >= 0; i--)
+ if (lines)
+ printf("%-15s %s\n",
+ filter.tags.items[i].string,
+ (char*)filter.tags.items[i].util);
+ else
printf("%s\n", filter.tags.items[i].string);
- else
- for (i = 0; i < filter.tags.nr; i++)
+ else
+ for (i = 0; i < filter.tags.nr; i++)
+ if (lines)
+ printf("%-15s %s\n",
+ filter.tags.items[i].string,
+ (char*)filter.tags.items[i].util);I see we print them here (or above depending on whether we reverse sort or not.. Nice! I would maybe suggest if we can rename util to something else so it is more clear? Maybe I am not understanding why it has to be named such.
quoted hunk ↗ jump to hunk
+ else printf("%s\n", filter.tags.items[i].string); - string_list_clear(&filter.tags, 0); - } + string_list_clear(&filter.tags, 1); + return 0; }@@ -665,8 +673,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix) copts.padding = 2; run_column_filter(colopts, &copts); } - if (lines != -1 && tag_sort) - die(_("--sort and -n are incompatible")); ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit, tag_sort); if (column_active(colopts)) stop_column_filter(); --2.5.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html