* [PATCH] tag: support mixing --sort=<spec> and -n
@ 2015-09-05 17:52 Rudy Matela
2015-09-05 22:25 ` Jacob Keller
0 siblings, 1 reply; 5+ messages in thread
From: Rudy Matela @ 2015-09-05 17:52 UTC (permalink / raw)
To: Git Mailing List; +Cc: Jeff King, Junio C Hamano
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>
---
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);
+ 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);
+ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] tag: support mixing --sort=<spec> and -n
2015-09-05 17:52 [PATCH] tag: support mixing --sort=<spec> and -n Rudy Matela
@ 2015-09-05 22:25 ` Jacob Keller
2015-09-06 3:52 ` Karthik Nayak
0 siblings, 1 reply; 5+ messages in thread
From: Jacob Keller @ 2015-09-05 22:25 UTC (permalink / raw)
To: Rudy Matela; +Cc: Git Mailing List, Jeff King, Junio C Hamano
On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela <rudy@matela.com.br> 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@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.
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] tag: support mixing --sort=<spec> and -n
2015-09-05 22:25 ` Jacob Keller
@ 2015-09-06 3:52 ` Karthik Nayak
2015-09-06 4:38 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Karthik Nayak @ 2015-09-06 3:52 UTC (permalink / raw)
To: Jacob Keller; +Cc: Rudy Matela, Git Mailing List, Jeff King, Junio C Hamano
On Sun, Sep 6, 2015 at 3:55 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela <rudy@matela.com.br> 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@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...
>
Its already resolved in my series. We use ref-filter's sorting APIs
there :)
--
Regards,
Karthik Nayak
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tag: support mixing --sort=<spec> and -n
2015-09-06 3:52 ` Karthik Nayak
@ 2015-09-06 4:38 ` Junio C Hamano
2015-09-06 6:25 ` Jacob Keller
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2015-09-06 4:38 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Jacob Keller, Rudy Matela, Git Mailing List, Jeff King
Karthik Nayak <karthik.188@gmail.com> writes:
> On Sun, Sep 6, 2015 at 3:55 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela <rudy@matela.com.br> 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@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...
>
> Its already resolved in my series. We use ref-filter's sorting APIs
> there :)
I guess Jacob needs to rebase that tag.sort thing on top of yours,
then ;-)?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tag: support mixing --sort=<spec> and -n
2015-09-06 4:38 ` Junio C Hamano
@ 2015-09-06 6:25 ` Jacob Keller
0 siblings, 0 replies; 5+ messages in thread
From: Jacob Keller @ 2015-09-06 6:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Karthik Nayak, Rudy Matela, Git Mailing List, Jeff King
On Sat, Sep 5, 2015 at 9:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Sun, Sep 6, 2015 at 3:55 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> On Sat, Sep 5, 2015 at 10:52 AM, Rudy Matela <rudy@matela.com.br> 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@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...
>>
>> Its already resolved in my series. We use ref-filter's sorting APIs
>> there :)
>
> I guess Jacob needs to rebase that tag.sort thing on top of yours,
> then ;-)?
Ya.. I wasn't sure if this was resolved, and I ended up deciding to
wait until after the tag re-write landed to look at that again.
Regards,
Jake
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-06 6:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-05 17:52 [PATCH] tag: support mixing --sort=<spec> and -n Rudy Matela
2015-09-05 22:25 ` Jacob Keller
2015-09-06 3:52 ` Karthik Nayak
2015-09-06 4:38 ` Junio C Hamano
2015-09-06 6:25 ` Jacob Keller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).