On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela <rudy@xxxxxxxxxxxxx> 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 <rudy@xxxxxxxxxxxxx>
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.
> ---
> 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.
> + 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html