Re: [PATCH] tag: support mixing --sort=<spec> and -n
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Sep 5, 2015, 22:25 UTC
- Message-ID
- <CA+P7+xo=4mC0fKkbtL+fkMXGxdUNcdUGUYHdaJ58MF9jVUUziw@mail.gmail.com>
- In-Reply-To
- <20150905175202.GC7050@zero.home>
On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela <rudy@matela.com.br> wrote:
Show 7 quoted lines
> > 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@matela.com.br>
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.
Show 98 quoted lines
> ---
> 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.
Show 16 quoted 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.
Show 25 quoted lines
> + 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