* [PATCH v16 01/14] ref-filter: move `struct atom_value` to ref-filter.c
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 02/14] ref-filter: introduce ref_formatting_state and ref_formatting_stack Karthik Nayak
` (14 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Since atom_value is only required for the internal working of
ref-filter it doesn't belong in the public header.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
ref-filter.c | 5 +++++
ref-filter.h | 5 +----
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 46963a5..e53c77e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,6 +55,11 @@ static struct {
{ "color" },
};
+struct atom_value {
+ const char *s;
+ unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
/*
* An atom is a valid field atom listed above, possibly prefixed with
* a "*" to denote deref_tag().
diff --git a/ref-filter.h b/ref-filter.h
index 6bf27d8..45026d0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -16,10 +16,7 @@
#define FILTER_REFS_INCLUDE_BROKEN 0x1
#define FILTER_REFS_ALL 0x2
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
+struct atom_value;
struct ref_sorting {
struct ref_sorting *next;
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 02/14] ref-filter: introduce ref_formatting_state and ref_formatting_stack
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 01/14] ref-filter: move `struct atom_value` to ref-filter.c Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 03/14] utf8: add function to align a string into given strbuf Karthik Nayak
` (13 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Introduce ref_formatting_state which will hold the formatted output
strbuf instead of directly printing to stdout. This will help us in
creating modifier atoms which modify the format specified before
printing to stdout.
Implement a stack machinery for ref_formatting_state, this allows us
to push and pop elements onto the stack. Whenever we pop an element
from the stack, the strbuf from that element is appended to the strbuf
of the next element on the stack, this will allow us to support
nesting of modifier atoms.
Rename some functions to reflect the changes made:
print_value() -> append_atom()
emit() -> append_literal()
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
ref-filter.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 19 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index e53c77e..432cea0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,6 +55,18 @@ static struct {
{ "color" },
};
+#define REF_FORMATTING_STATE_INIT { 0, NULL }
+
+struct ref_formatting_stack {
+ struct ref_formatting_stack *prev;
+ struct strbuf output;
+};
+
+struct ref_formatting_state {
+ int quote_style;
+ struct ref_formatting_stack *stack;
+};
+
struct atom_value {
const char *s;
unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -129,6 +141,27 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
return at;
}
+static void push_stack_element(struct ref_formatting_stack **stack)
+{
+ struct ref_formatting_stack *s = xcalloc(1, sizeof(struct ref_formatting_stack));
+
+ strbuf_init(&s->output, 0);
+ s->prev = *stack;
+ *stack = s;
+}
+
+static void pop_stack_element(struct ref_formatting_stack **stack)
+{
+ struct ref_formatting_stack *current = *stack;
+ struct ref_formatting_stack *prev = current->prev;
+
+ if (prev)
+ strbuf_addbuf(&prev->output, ¤t->output);
+ strbuf_release(¤t->output);
+ free(current);
+ *stack = prev;
+}
+
/*
* In a format string, find the next occurrence of %(atom).
*/
@@ -1195,30 +1228,27 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
}
-static void print_value(struct atom_value *v, int quote_style)
+static void append_atom(struct atom_value *v, struct ref_formatting_state *state)
{
- struct strbuf sb = STRBUF_INIT;
- switch (quote_style) {
+ struct strbuf *s = &state->stack->output;
+
+ switch (state->quote_style) {
case QUOTE_NONE:
- fputs(v->s, stdout);
+ strbuf_addstr(s, v->s);
break;
case QUOTE_SHELL:
- sq_quote_buf(&sb, v->s);
+ sq_quote_buf(s, v->s);
break;
case QUOTE_PERL:
- perl_quote_buf(&sb, v->s);
+ perl_quote_buf(s, v->s);
break;
case QUOTE_PYTHON:
- python_quote_buf(&sb, v->s);
+ python_quote_buf(s, v->s);
break;
case QUOTE_TCL:
- tcl_quote_buf(&sb, v->s);
+ tcl_quote_buf(s, v->s);
break;
}
- if (quote_style != QUOTE_NONE) {
- fputs(sb.buf, stdout);
- strbuf_release(&sb);
- }
}
static int hex1(char ch)
@@ -1239,8 +1269,10 @@ static int hex2(const char *cp)
return -1;
}
-static void emit(const char *cp, const char *ep)
+static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)
{
+ struct strbuf *s = &state->stack->output;
+
while (*cp && (!ep || cp < ep)) {
if (*cp == '%') {
if (cp[1] == '%')
@@ -1248,13 +1280,13 @@ static void emit(const char *cp, const char *ep)
else {
int ch = hex2(cp + 1);
if (0 <= ch) {
- putchar(ch);
+ strbuf_addch(s, ch);
cp += 3;
continue;
}
}
}
- putchar(*cp);
+ strbuf_addch(s, *cp);
cp++;
}
}
@@ -1262,19 +1294,24 @@ static void emit(const char *cp, const char *ep)
void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
{
const char *cp, *sp, *ep;
+ struct strbuf *final_buf;
+ struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+
+ state.quote_style = quote_style;
+ push_stack_element(&state.stack);
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
struct atom_value *atomv;
ep = strchr(sp, ')');
if (cp < sp)
- emit(cp, sp);
+ append_literal(cp, sp, &state);
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
- print_value(atomv, quote_style);
+ append_atom(atomv, &state);
}
if (*cp) {
sp = cp + strlen(cp);
- emit(cp, sp);
+ append_literal(cp, sp, &state);
}
if (need_color_reset_at_eol) {
struct atom_value resetv;
@@ -1283,8 +1320,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
if (color_parse("reset", color) < 0)
die("BUG: couldn't parse 'reset' as a color");
resetv.s = color;
- print_value(&resetv, quote_style);
+ append_atom(&resetv, &state);
}
+ final_buf = &state.stack->output;
+ fwrite(final_buf->buf, 1, final_buf->len, stdout);
+ pop_stack_element(&state.stack);
putchar('\n');
}
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 03/14] utf8: add function to align a string into given strbuf
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 01/14] ref-filter: move `struct atom_value` to ref-filter.c Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 02/14] ref-filter: introduce ref_formatting_state and ref_formatting_stack Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 04/14] ref-filter: introduce handler function for each atom Karthik Nayak
` (12 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Add strbuf_utf8_align() which will align a given string into a strbuf
as per given align_type and width. If the width is greater than the
string length then no alignment is performed.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
utf8.c | 21 +++++++++++++++++++++
utf8.h | 15 +++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/utf8.c b/utf8.c
index 28e6d76..00e10c8 100644
--- a/utf8.c
+++ b/utf8.c
@@ -644,3 +644,24 @@ int skip_utf8_bom(char **text, size_t len)
*text += strlen(utf8_bom);
return 1;
}
+
+void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,
+ const char *s)
+{
+ int slen = strlen(s);
+ int display_len = utf8_strnwidth(s, slen, 0);
+ int utf8_compensation = slen - display_len;
+
+ if (display_len >= width) {
+ strbuf_addstr(buf, s);
+ return;
+ }
+
+ if (position == ALIGN_LEFT)
+ strbuf_addf(buf, "%-*s", width + utf8_compensation, s);
+ else if (position == ALIGN_MIDDLE) {
+ int left = (width - display_len) / 2;
+ strbuf_addf(buf, "%*s%-*s", left, "", width - left + utf8_compensation, s);
+ } else if (position == ALIGN_RIGHT)
+ strbuf_addf(buf, "%*s", width + utf8_compensation, s);
+}
diff --git a/utf8.h b/utf8.h
index 5a9e94b..7930b44 100644
--- a/utf8.h
+++ b/utf8.h
@@ -55,4 +55,19 @@ int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
*/
int is_hfs_dotgit(const char *path);
+typedef enum {
+ ALIGN_LEFT,
+ ALIGN_MIDDLE,
+ ALIGN_RIGHT
+} align_type;
+
+/*
+ * Align the string given and store it into a strbuf as per the
+ * 'position' and 'width'. If the given string length is larger than
+ * 'width' than then the input string is not truncated and no
+ * alignment is done.
+ */
+void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,
+ const char *s);
+
#endif
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 04/14] ref-filter: introduce handler function for each atom
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (2 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 03/14] utf8: add function to align a string into given strbuf Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 05/14] ref-filter: introduce match_atom_name() Karthik Nayak
` (11 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Introduce a handler function for each atom, which is called when the
atom is processed in show_ref_array_item().
In this context make append_atom() as the default handler function and
extract quote_formatting() out of append_atom(). Bump this to the top.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
ref-filter.c | 54 ++++++++++++++++++++++++++++++------------------------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 432cea0..a993216 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -69,6 +69,7 @@ struct ref_formatting_state {
struct atom_value {
const char *s;
+ void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);
unsigned long ul; /* used for sorting when not FIELD_STR */
};
@@ -141,6 +142,32 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
return at;
}
+static void quote_formatting(struct strbuf *s, const char *str, int quote_style)
+{
+ switch (quote_style) {
+ case QUOTE_NONE:
+ strbuf_addstr(s, str);
+ break;
+ case QUOTE_SHELL:
+ sq_quote_buf(s, str);
+ break;
+ case QUOTE_PERL:
+ perl_quote_buf(s, str);
+ break;
+ case QUOTE_PYTHON:
+ python_quote_buf(s, str);
+ break;
+ case QUOTE_TCL:
+ tcl_quote_buf(s, str);
+ break;
+ }
+}
+
+static void append_atom(struct atom_value *v, struct ref_formatting_state *state)
+{
+ quote_formatting(&state->stack->output, v->s, state->quote_style);
+}
+
static void push_stack_element(struct ref_formatting_stack **stack)
{
struct ref_formatting_stack *s = xcalloc(1, sizeof(struct ref_formatting_stack));
@@ -662,6 +689,8 @@ static void populate_value(struct ref_array_item *ref)
const char *formatp;
struct branch *branch = NULL;
+ v->handler = append_atom;
+
if (*name == '*') {
deref = 1;
name++;
@@ -1228,29 +1257,6 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
}
-static void append_atom(struct atom_value *v, struct ref_formatting_state *state)
-{
- struct strbuf *s = &state->stack->output;
-
- switch (state->quote_style) {
- case QUOTE_NONE:
- strbuf_addstr(s, v->s);
- break;
- case QUOTE_SHELL:
- sq_quote_buf(s, v->s);
- break;
- case QUOTE_PERL:
- perl_quote_buf(s, v->s);
- break;
- case QUOTE_PYTHON:
- python_quote_buf(s, v->s);
- break;
- case QUOTE_TCL:
- tcl_quote_buf(s, v->s);
- break;
- }
-}
-
static int hex1(char ch)
{
if ('0' <= ch && ch <= '9')
@@ -1307,7 +1313,7 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
if (cp < sp)
append_literal(cp, sp, &state);
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
- append_atom(atomv, &state);
+ atomv->handler(atomv, &state);
}
if (*cp) {
sp = cp + strlen(cp);
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 05/14] ref-filter: introduce match_atom_name()
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (3 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 04/14] ref-filter: introduce handler function for each atom Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-06 19:52 ` Eric Sunshine
2015-09-05 18:52 ` [PATCH v16 06/14] ref-filter: implement an `align` atom Karthik Nayak
` (10 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Introduce match_atom_name() which helps in checking if a particular
atom is the atom we're looking for and if it has a value attached to
it or not.
Use it instead of starts_with() for checking the value of %(color:...)
atom. Write a test for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Thanks-to: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
ref-filter.c | 23 +++++++++++++++++++++--
t/t6302-for-each-ref-filter.sh | 4 ++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index a993216..e99c342 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -189,6 +189,22 @@ static void pop_stack_element(struct ref_formatting_stack **stack)
*stack = prev;
}
+static int match_atom_name(const char *name, const char *atom_name, const char **val)
+{
+ const char *body;
+
+ if (!skip_prefix(name, atom_name, &body))
+ return 0; /* doesn't even begin with "atom_name" */
+ if (!body[0] || !body[1]) {
+ *val = NULL; /* %(atom_name) and no customization */
+ return 1;
+ }
+ if (body[0] != ':')
+ return 0; /* "atom_namefoo" is not "atom_name" or "atom_name:..." */
+ *val = body + 1; /* "atomname:val" */
+ return 1;
+}
+
/*
* In a format string, find the next occurrence of %(atom).
*/
@@ -687,6 +703,7 @@ static void populate_value(struct ref_array_item *ref)
int deref = 0;
const char *refname;
const char *formatp;
+ const char *valp;
struct branch *branch = NULL;
v->handler = append_atom;
@@ -721,10 +738,12 @@ static void populate_value(struct ref_array_item *ref)
refname = branch_get_push(branch, NULL);
if (!refname)
continue;
- } else if (starts_with(name, "color:")) {
+ } else if (match_atom_name(name, "color", &valp)) {
char color[COLOR_MAXLEN] = "";
- if (color_parse(name + 6, color) < 0)
+ if (!valp)
+ die(_("expected format: %%(color:<color>)"));
+ if (color_parse(valp, color) < 0)
die(_("unable to parse format"));
v->s = xstrdup(color);
continue;
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 505a360..c4f0378 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -81,4 +81,8 @@ test_expect_success 'filtering with --contains' '
test_cmp expect actual
'
+test_expect_success '%(color) must fail' '
+ test_must_fail git for-each-ref --format="%(color)%(refname)"
+'
+
test_done
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v16 05/14] ref-filter: introduce match_atom_name()
2015-09-05 18:52 ` [PATCH v16 05/14] ref-filter: introduce match_atom_name() Karthik Nayak
@ 2015-09-06 19:52 ` Eric Sunshine
2015-09-07 18:07 ` Karthik Nayak
0 siblings, 1 reply; 30+ messages in thread
From: Eric Sunshine @ 2015-09-06 19:52 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Git List, Christian Couder, Matthieu Moy, Junio C Hamano
On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> Introduce match_atom_name() which helps in checking if a particular
> atom is the atom we're looking for and if it has a value attached to
> it or not.
>
> Use it instead of starts_with() for checking the value of %(color:...)
> atom. Write a test for the same.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index a993216..e99c342 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> +static int match_atom_name(const char *name, const char *atom_name, const char **val)
> +{
> + const char *body;
> +
> + if (!skip_prefix(name, atom_name, &body))
> + return 0; /* doesn't even begin with "atom_name" */
> + if (!body[0] || !body[1]) {
> + *val = NULL; /* %(atom_name) and no customization */
> + return 1;
If this is invoked as match_atom_name("colors", "color", ...), then it
will return true (matched, but no value), which is not correct at all;
"colors" is not a match for atom %(color). Maybe you meant?
if (!body[0] || (body[0] == ':' && !body[1])) {
But, that's getting ugly and complicated, and would be bettered
handled by reordering the logic of this function for dealing with the
various valid and invalid cases. However...
> + }
> + if (body[0] != ':')
> + return 0; /* "atom_namefoo" is not "atom_name" or "atom_name:..." */
> + *val = body + 1; /* "atomname:val" */
> + return 1;
> +}
It's not clear why this function exists in the first place. It's only
purpose seems to be to specially recognize instances of atoms which
should have a ":" but lack it, so that the caller can then report an
error.
But why is this better than the more generic solution of merely
reporting an error for *any* unrecognized atom? How does it help to
single out these special cases?
There is a further inconsistency in that you are only specially
recognizing %(color) and %(align) lacking ":", but not
%(content:lines) lacking "=" in patch 8/14. Why do %(color:...) and
%(align:...) get special treatment but %(content:lines=...) does not?
Such inconsistency again argues in favor of a generic "unrecognized
atom" detection and reporting over special case handling.
> /*
> * In a format string, find the next occurrence of %(atom).
> */
> @@ -721,10 +738,12 @@ static void populate_value(struct ref_array_item *ref)
> refname = branch_get_push(branch, NULL);
> if (!refname)
> continue;
> - } else if (starts_with(name, "color:")) {
> + } else if (match_atom_name(name, "color", &valp)) {
> char color[COLOR_MAXLEN] = "";
>
> - if (color_parse(name + 6, color) < 0)
> + if (!valp)
> + die(_("expected format: %%(color:<color>)"));
> + if (color_parse(valp, color) < 0)
> die(_("unable to parse format"));
> v->s = xstrdup(color);
> continue;
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> index 505a360..c4f0378 100755
> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -81,4 +81,8 @@ test_expect_success 'filtering with --contains' '
> +test_expect_success '%(color) must fail' '
> + test_must_fail git for-each-ref --format="%(color)%(refname)"
> +'
> +
> test_done
> --
> 2.5.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 05/14] ref-filter: introduce match_atom_name()
2015-09-06 19:52 ` Eric Sunshine
@ 2015-09-07 18:07 ` Karthik Nayak
0 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-07 18:07 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List, Christian Couder, Matthieu Moy, Junio C Hamano
On Mon, Sep 7, 2015 at 1:22 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> Introduce match_atom_name() which helps in checking if a particular
>> atom is the atom we're looking for and if it has a value attached to
>> it or not.
>>
>> Use it instead of starts_with() for checking the value of %(color:...)
>> atom. Write a test for the same.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> index a993216..e99c342 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> +static int match_atom_name(const char *name, const char *atom_name, const char **val)
>> +{
>> + const char *body;
>> +
>> + if (!skip_prefix(name, atom_name, &body))
>> + return 0; /* doesn't even begin with "atom_name" */
>> + if (!body[0] || !body[1]) {
>> + *val = NULL; /* %(atom_name) and no customization */
>> + return 1;
>
> If this is invoked as match_atom_name("colors", "color", ...), then it
> will return true (matched, but no value), which is not correct at all;
> "colors" is not a match for atom %(color). Maybe you meant?
>
> if (!body[0] || (body[0] == ':' && !body[1])) {
>
> But, that's getting ugly and complicated, and would be bettered
> handled by reordering the logic of this function for dealing with the
> various valid and invalid cases. However...
>
Well as you infered the logic is to check so that there is something
existing after the ':'. About why I didn't do something like this
"(body[0] == ':' && !body[1])" is because ref-filter already
checks for invalid atom names.
So before even match_atom_name() is called, the check is done in
parse_ref_filter_atom().
>> + }
>> + if (body[0] != ':')
>> + return 0; /* "atom_namefoo" is not "atom_name" or "atom_name:..." */
>> + *val = body + 1; /* "atomname:val" */
>> + return 1;
>> +}
>
> It's not clear why this function exists in the first place. It's only
> purpose seems to be to specially recognize instances of atoms which
> should have a ":" but lack it, so that the caller can then report an
> error.
>
> But why is this better than the more generic solution of merely
> reporting an error for *any* unrecognized atom? How does it help to
> single out these special cases?
>
> There is a further inconsistency in that you are only specially
> recognizing %(color) and %(align) lacking ":", but not
> %(content:lines) lacking "=" in patch 8/14. Why do %(color:...) and
> %(align:...) get special treatment but %(content:lines=...) does not?
> Such inconsistency again argues in favor of a generic "unrecognized
> atom" detection and reporting over special case handling.
>
This is from http://article.gmane.org/gmane.comp.version-control.git/277099
Junio suggested to have this to check for ":" rather than rely on
skip_prefix() and check manually after that.
Agreed, a more generic solution would be better, and If I remember I said,
I'll work on that after this entire series.
About contents:lines, we declare contents:lines itself as an atom, this was to
keep it similar to how the other contents atoms are declared, so the testing
for this again is already done by parse_ref_filter_atom().
--
Regards,
Karthik Nayak
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v16 06/14] ref-filter: implement an `align` atom
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (4 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 05/14] ref-filter: introduce match_atom_name() Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-06 20:10 ` Eric Sunshine
2015-09-05 18:52 ` [PATCH v16 07/14] ref-filter: add option to filter out tags, branches and remotes Karthik Nayak
` (9 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Implement an `align` atom which left-, middle-, or right-aligns the
content between %(align:...) and %(end).
It is followed by `:<width>,<position>`, where the `<position>` is
either left, right or middle and `<width>` is the size of the area
into which the content will be placed. If the content between
%(align:...) and %(end) is more than the width then no alignment is
performed. e.g. to align a refname atom to the middle with a total
width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
We introduce an `at_end` function for each element of the stack which
is to be called when the `end` atom is encountered. Using this we
implement end_align_handler() for the `align` atom, this aligns the
final strbuf by calling `strbuf_utf8_align()` from utf8.c.
Ensure that quote formatting is performed on the whole of
%(align:...)...%(end) rather than individual atoms inside. We skip
quote formatting for individual atoms when the current stack element
is handling an %(align:...) atom and perform quote formatting at the
end when we encounter the %(end) atom of the second element of then
stack.
Add documentation and tests for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-for-each-ref.txt | 10 ++++
ref-filter.c | 112 ++++++++++++++++++++++++++++++++++++-
t/t6302-for-each-ref-filter.sh | 82 +++++++++++++++++++++++++++
3 files changed, 203 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index e49d578..b23412d 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,6 +127,16 @@ color::
Change output color. Followed by `:<colorname>`, where names
are described in `color.branch.*`.
+align::
+ Left-, middle-, or right-align the content between %(align:...)
+ and %(end). Followed by `:<width>,<position>`, where the
+ `<position>` is either left, right or middle and `<width>` is
+ the total length of the content with alignment. If the
+ contents length is more than the width then no alignment is
+ performed. If used with '--quote' everything in between
+ %(align:...) and %(end) is quoted, but if nested then only the
+ topmost level performs quoting.
+
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index e99c342..6c9ef08 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -10,6 +10,7 @@
#include "quote.h"
#include "ref-filter.h"
#include "revision.h"
+#include "utf8.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -53,13 +54,22 @@ static struct {
{ "flag" },
{ "HEAD" },
{ "color" },
+ { "align" },
+ { "end" },
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
+struct align {
+ align_type position;
+ unsigned int width;
+};
+
struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
+ void (*at_end)(struct ref_formatting_stack *stack);
+ void *at_end_data;
};
struct ref_formatting_state {
@@ -69,6 +79,9 @@ struct ref_formatting_state {
struct atom_value {
const char *s;
+ union {
+ struct align align;
+ } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);
unsigned long ul; /* used for sorting when not FIELD_STR */
};
@@ -165,7 +178,16 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style)
static void append_atom(struct atom_value *v, struct ref_formatting_state *state)
{
- quote_formatting(&state->stack->output, v->s, state->quote_style);
+ /*
+ * Quote formatting is only done when the stack has a single
+ * element. Otherwise quote formatting is done on the
+ * element's entire output strbuf when the %(end) atom is
+ * encountered.
+ */
+ if (!state->stack->prev)
+ quote_formatting(&state->stack->output, v->s, state->quote_style);
+ else
+ strbuf_addstr(&state->stack->output, v->s);
}
static void push_stack_element(struct ref_formatting_stack **stack)
@@ -189,6 +211,48 @@ static void pop_stack_element(struct ref_formatting_stack **stack)
*stack = prev;
}
+static void end_align_handler(struct ref_formatting_stack *stack)
+{
+ struct align *align = (struct align *)stack->at_end_data;
+ struct strbuf s = STRBUF_INIT;
+
+ strbuf_utf8_align(&s, align->position, align->width, stack->output.buf);
+ strbuf_swap(&stack->output, &s);
+ strbuf_release(&s);
+}
+
+static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
+{
+ struct ref_formatting_stack *new;
+
+ push_stack_element(&state->stack);
+ new = state->stack;
+ new->at_end = end_align_handler;
+ new->at_end_data = &atomv->u.align;
+}
+
+static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
+{
+ struct ref_formatting_stack *current = state->stack;
+ struct strbuf s = STRBUF_INIT;
+
+ if (!current->at_end)
+ die(_("format: %%(end) atom used without corresponding atom"));
+ current->at_end(current);
+
+ /*
+ * Perform quote formatting when the stack element is that of
+ * a supporting atom. If nested then perform quote formatting
+ * only on the topmost supporting atom.
+ */
+ if (!state->stack->prev->prev) {
+ quote_formatting(&s, current->output.buf, state->quote_style);
+ strbuf_swap(¤t->output, &s);
+ }
+ strbuf_release(&s);
+ pop_stack_element(&state->stack);
+}
+
static int match_atom_name(const char *name, const char *atom_name, const char **val)
{
const char *body;
@@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)
else
v->s = " ";
continue;
+ } else if (match_atom_name(name, "align", &valp)) {
+ struct align *align = &v->u.align;
+ struct strbuf **s;
+
+ if (!valp)
+ die(_("expected format: %%(align:<width>, <position>)"));
+
+ /*
+ * TODO: Implement a function similar to strbuf_split_str()
+ * which would strip the terminator at the end.
+ */
+ s = strbuf_split_str(valp, ',', 0);
+
+ /* If the position is given trim the ',' from the first strbuf */
+ if (s[1])
+ strbuf_setlen(s[0], s[0]->len - 1);
+ if (s[2])
+ die(_("align:<width>,<position> followed by garbage: %s"), s[2]->buf);
+
+ if (strtoul_ui(s[0]->buf, 10, &align->width))
+ die(_("positive width expected align:%s"), s[0]->buf);
+
+ /*
+ * TODO: Implement a more general check, so that the values
+ * do not always have to be in a specific order.
+ */
+ if (!s[1])
+ align->position = ALIGN_LEFT;
+ else if (!strcmp(s[1]->buf, "left"))
+ align->position = ALIGN_LEFT;
+ else if (!strcmp(s[1]->buf, "right"))
+ align->position = ALIGN_RIGHT;
+ else if (!strcmp(s[1]->buf, "middle"))
+ align->position = ALIGN_MIDDLE;
+ else
+ die(_("improper format entered align:%s"), s[1]->buf);
+
+ strbuf_list_free(s);
+
+ v->handler = align_atom_handler;
+ continue;
+ } else if (!strcmp(name, "end")) {
+ v->handler = end_atom_handler;
+ continue;
} else
continue;
@@ -1347,6 +1455,8 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
resetv.s = color;
append_atom(&resetv, &state);
}
+ if (state.stack->prev)
+ die(_("format: %%(end) atom missing"));
final_buf = &state.stack->output;
fwrite(final_buf->buf, 1, final_buf->len, stdout);
pop_stack_element(&state.stack);
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index c4f0378..d0c0139 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -85,4 +85,86 @@ test_expect_success '%(color) must fail' '
test_must_fail git for-each-ref --format="%(color)%(refname)"
'
+test_expect_success 'left alignment' '
+ cat >expect <<-\EOF &&
+ refname is refs/heads/master |refs/heads/master
+ refname is refs/heads/side |refs/heads/side
+ refname is refs/odd/spot |refs/odd/spot
+ refname is refs/tags/double-tag|refs/tags/double-tag
+ refname is refs/tags/four |refs/tags/four
+ refname is refs/tags/one |refs/tags/one
+ refname is refs/tags/signed-tag|refs/tags/signed-tag
+ refname is refs/tags/three |refs/tags/three
+ refname is refs/tags/two |refs/tags/two
+ EOF
+ git for-each-ref --format="%(align:30,left)refname is %(refname)%(end)|%(refname)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'middle alignment' '
+ cat >expect <<-\EOF &&
+ | refname is refs/heads/master |refs/heads/master
+ | refname is refs/heads/side |refs/heads/side
+ | refname is refs/odd/spot |refs/odd/spot
+ |refname is refs/tags/double-tag|refs/tags/double-tag
+ | refname is refs/tags/four |refs/tags/four
+ | refname is refs/tags/one |refs/tags/one
+ |refname is refs/tags/signed-tag|refs/tags/signed-tag
+ | refname is refs/tags/three |refs/tags/three
+ | refname is refs/tags/two |refs/tags/two
+ EOF
+ git for-each-ref --format="|%(align:30,middle)refname is %(refname)%(end)|%(refname)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'right alignment' '
+ cat >expect <<-\EOF &&
+ | refname is refs/heads/master|refs/heads/master
+ | refname is refs/heads/side|refs/heads/side
+ | refname is refs/odd/spot|refs/odd/spot
+ |refname is refs/tags/double-tag|refs/tags/double-tag
+ | refname is refs/tags/four|refs/tags/four
+ | refname is refs/tags/one|refs/tags/one
+ |refname is refs/tags/signed-tag|refs/tags/signed-tag
+ | refname is refs/tags/three|refs/tags/three
+ | refname is refs/tags/two|refs/tags/two
+ EOF
+ git for-each-ref --format="|%(align:30,right)refname is %(refname)%(end)|%(refname)" >actual &&
+ test_cmp expect actual
+'
+
+# Individual atoms inside %(align:...) and %(end) must not be quoted.
+
+test_expect_success 'alignment with format quote' "
+ cat >expect <<-\EOF &&
+ |' master| A U Thor '|
+ |' side| A U Thor '|
+ |' odd/spot| A U Thor '|
+ |' double-tag| '|
+ |' four| A U Thor '|
+ |' one| A U Thor '|
+ |' signed-tag| '|
+ |' three| A U Thor '|
+ |' two| A U Thor '|
+ EOF
+ git for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&
+ test_cmp expect actual
+"
+
+test_expect_success 'nested alignment with quote formatting' "
+ cat >expect <<-\EOF &&
+ |' master '|
+ |' side '|
+ |' odd/spot '|
+ |' double-tag '|
+ |' four '|
+ |' one '|
+ |' signed-tag '|
+ |' three '|
+ |' two '|
+ EOF
+ git for-each-ref --shell --format='|%(align:30,left)%(align:15,right)%(refname:short)%(end)%(end)|' >actual &&
+ test_cmp expect actual
+"
+
test_done
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v16 06/14] ref-filter: implement an `align` atom
2015-09-05 18:52 ` [PATCH v16 06/14] ref-filter: implement an `align` atom Karthik Nayak
@ 2015-09-06 20:10 ` Eric Sunshine
2015-09-07 18:26 ` Karthik Nayak
0 siblings, 1 reply; 30+ messages in thread
From: Eric Sunshine @ 2015-09-06 20:10 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Git List, Christian Couder, Matthieu Moy, Junio C Hamano
On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> Implement an `align` atom which left-, middle-, or right-aligns the
> content between %(align:...) and %(end).
>
> It is followed by `:<width>,<position>`, where the `<position>` is
> either left, right or middle and `<width>` is the size of the area
> into which the content will be placed. If the content between
> %(align:...) and %(end) is more than the width then no alignment is
> performed. e.g. to align a refname atom to the middle with a total
> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>
> We introduce an `at_end` function for each element of the stack which
> is to be called when the `end` atom is encountered. Using this we
> implement end_align_handler() for the `align` atom, this aligns the
> final strbuf by calling `strbuf_utf8_align()` from utf8.c.
>
> Ensure that quote formatting is performed on the whole of
> %(align:...)...%(end) rather than individual atoms inside. We skip
> quote formatting for individual atoms when the current stack element
> is handling an %(align:...) atom and perform quote formatting at the
> end when we encounter the %(end) atom of the second element of then
> stack.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index e49d578..b23412d 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -127,6 +127,16 @@ color::
> +align::
> + Left-, middle-, or right-align the content between %(align:...)
> + and %(end). Followed by `:<width>,<position>`, where the
> + `<position>` is either left, right or middle and `<width>` is
> + the total length of the content with alignment. If the
This should mention that <position> is optional and default to "left"
if omitted.
> + contents length is more than the width then no alignment is
> + performed. If used with '--quote' everything in between
> + %(align:...) and %(end) is quoted, but if nested then only the
> + topmost level performs quoting.
> diff --git a/ref-filter.c b/ref-filter.c
> index e99c342..6c9ef08 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)
> else
> v->s = " ";
> continue;
> + } else if (match_atom_name(name, "align", &valp)) {
> + struct align *align = &v->u.align;
> + struct strbuf **s;
> +
> + if (!valp)
> + die(_("expected format: %%(align:<width>, <position>)"));
I'm pretty sure this parsing code won't deal well with a space after
the comma, so the space should be dropped from the diagnostic message
advising the user of the correct format: s/, /,/
> + /*
> + * TODO: Implement a function similar to strbuf_split_str()
> + * which would strip the terminator at the end.
"...which would omit the separator from the end of each value."
> + */
> + s = strbuf_split_str(valp, ',', 0);
> +
> + /* If the position is given trim the ',' from the first strbuf */
> + if (s[1])
> + strbuf_setlen(s[0], s[0]->len - 1);
> + if (s[2])
> + die(_("align:<width>,<position> followed by garbage: %s"), s[2]->buf);
If <position> is omitted, strbuf_split_str("42", ...) will return the
array ["42", NULL] which won't have an element [2], which means this
code will access beyond end-of-array. (This is another good argument
for looping over s[] as Junio suggested rather than assuming these
fixed yet optional positions. It can be hard to get it right.)
> + if (strtoul_ui(s[0]->buf, 10, &align->width))
> + die(_("positive width expected align:%s"), s[0]->buf);
> +
> + /*
> + * TODO: Implement a more general check, so that the values
> + * do not always have to be in a specific order.
> + */
> + if (!s[1])
> + align->position = ALIGN_LEFT;
> + else if (!strcmp(s[1]->buf, "left"))
> + align->position = ALIGN_LEFT;
> + else if (!strcmp(s[1]->buf, "right"))
> + align->position = ALIGN_RIGHT;
> + else if (!strcmp(s[1]->buf, "middle"))
> + align->position = ALIGN_MIDDLE;
> + else
> + die(_("improper format entered align:%s"), s[1]->buf);
> +
> + strbuf_list_free(s);
> +
> + v->handler = align_atom_handler;
> + continue;
> + } else if (!strcmp(name, "end")) {
> + v->handler = end_atom_handler;
> + continue;
> } else
> continue;
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 06/14] ref-filter: implement an `align` atom
2015-09-06 20:10 ` Eric Sunshine
@ 2015-09-07 18:26 ` Karthik Nayak
0 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-07 18:26 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List, Christian Couder, Matthieu Moy, Junio C Hamano
On Mon, Sep 7, 2015 at 1:40 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> Implement an `align` atom which left-, middle-, or right-aligns the
>> content between %(align:...) and %(end).
>>
>> It is followed by `:<width>,<position>`, where the `<position>` is
>> either left, right or middle and `<width>` is the size of the area
>> into which the content will be placed. If the content between
>> %(align:...) and %(end) is more than the width then no alignment is
>> performed. e.g. to align a refname atom to the middle with a total
>> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>>
>> We introduce an `at_end` function for each element of the stack which
>> is to be called when the `end` atom is encountered. Using this we
>> implement end_align_handler() for the `align` atom, this aligns the
>> final strbuf by calling `strbuf_utf8_align()` from utf8.c.
>>
>> Ensure that quote formatting is performed on the whole of
>> %(align:...)...%(end) rather than individual atoms inside. We skip
>> quote formatting for individual atoms when the current stack element
>> is handling an %(align:...) atom and perform quote formatting at the
>> end when we encounter the %(end) atom of the second element of then
>> stack.
>>
>> Add documentation and tests for the same.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> index e49d578..b23412d 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -127,6 +127,16 @@ color::
>> +align::
>> + Left-, middle-, or right-align the content between %(align:...)
>> + and %(end). Followed by `:<width>,<position>`, where the
>> + `<position>` is either left, right or middle and `<width>` is
>> + the total length of the content with alignment. If the
>
> This should mention that <position> is optional and default to "left"
> if omitted.
>
Will add that.
>> + contents length is more than the width then no alignment is
>> + performed. If used with '--quote' everything in between
>> + %(align:...) and %(end) is quoted, but if nested then only the
>> + topmost level performs quoting.
>> diff --git a/ref-filter.c b/ref-filter.c
>> index e99c342..6c9ef08 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)
>> else
>> v->s = " ";
>> continue;
>> + } else if (match_atom_name(name, "align", &valp)) {
>> + struct align *align = &v->u.align;
>> + struct strbuf **s;
>> +
>> + if (!valp)
>> + die(_("expected format: %%(align:<width>, <position>)"));
>
> I'm pretty sure this parsing code won't deal well with a space after
> the comma, so the space should be dropped from the diagnostic message
> advising the user of the correct format: s/, /,/
>
Will change.
>> + /*
>> + * TODO: Implement a function similar to strbuf_split_str()
>> + * which would strip the terminator at the end.
>
> "...which would omit the separator from the end of each value."
>
Will change.
>> + */
>> + s = strbuf_split_str(valp, ',', 0);
>> +
>> + /* If the position is given trim the ',' from the first strbuf */
>> + if (s[1])
>> + strbuf_setlen(s[0], s[0]->len - 1);
>> + if (s[2])
>> + die(_("align:<width>,<position> followed by garbage: %s"), s[2]->buf);
>
> If <position> is omitted, strbuf_split_str("42", ...) will return the
> array ["42", NULL] which won't have an element [2], which means this
> code will access beyond end-of-array. (This is another good argument
> for looping over s[] as Junio suggested rather than assuming these
> fixed yet optional positions. It can be hard to get it right.)
You're right, probably something on the lines of what Junio suggested here
http://article.gmane.org/gmane.comp.version-control.git/277041
--
Regards,
Karthik Nayak
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v16 07/14] ref-filter: add option to filter out tags, branches and remotes
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (5 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 06/14] ref-filter: implement an `align` atom Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 08/14] ref-filter: add support for %(contents:lines=X) Karthik Nayak
` (8 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak
From: Karthik Nayak <karthik.188@gmail.com>
Add a function called 'for_each_fullref_in()' to refs.{c,h} which
iterates through each ref for the given path without trimming the path
and also accounting for broken refs, if mentioned.
Add 'filter_ref_kind()' in ref-filter.c to check the kind of ref being
handled and return the kind to 'ref_filter_handler()', where we
discard refs which we do not need and assign the kind to needed refs.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
ref-filter.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
ref-filter.h | 13 ++++++++++--
refs.c | 9 +++++++++
refs.h | 1 +
4 files changed, 81 insertions(+), 7 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 6c9ef08..c7a8cf0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1192,6 +1192,34 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
return ref;
}
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+ unsigned int i;
+
+ static struct {
+ const char *prefix;
+ unsigned int kind;
+ } ref_kind[] = {
+ { "refs/heads/" , FILTER_REFS_BRANCHES },
+ { "refs/remotes/" , FILTER_REFS_REMOTES },
+ { "refs/tags/", FILTER_REFS_TAGS}
+ };
+
+ if (filter->kind == FILTER_REFS_BRANCHES ||
+ filter->kind == FILTER_REFS_REMOTES ||
+ filter->kind == FILTER_REFS_TAGS)
+ return filter->kind;
+ else if (!strcmp(refname, "HEAD"))
+ return FILTER_REFS_DETACHED_HEAD;
+
+ for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
+ if (starts_with(refname, ref_kind[i].prefix))
+ return ref_kind[i].kind;
+ }
+
+ return FILTER_REFS_OTHERS;
+}
+
/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
@@ -1202,6 +1230,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
struct ref_filter *filter = ref_cbdata->filter;
struct ref_array_item *ref;
struct commit *commit = NULL;
+ unsigned int kind;
if (flag & REF_BAD_NAME) {
warning("ignoring ref with broken name %s", refname);
@@ -1213,6 +1242,11 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
return 0;
}
+ /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */
+ kind = filter_ref_kind(filter, refname);
+ if (!(kind & filter->kind))
+ return 0;
+
if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
return 0;
@@ -1244,6 +1278,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
+ ref->kind = kind;
return 0;
}
@@ -1320,17 +1355,37 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
{
struct ref_filter_cbdata ref_cbdata;
int ret = 0;
+ unsigned int broken = 0;
ref_cbdata.array = array;
ref_cbdata.filter = filter;
+ if (type & FILTER_REFS_INCLUDE_BROKEN)
+ broken = 1;
+ filter->kind = type & FILTER_REFS_KIND_MASK;
+
/* Simple per-ref filtering */
- if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
- ret = for_each_rawref(ref_filter_handler, &ref_cbdata);
- else if (type & FILTER_REFS_ALL)
- ret = for_each_ref(ref_filter_handler, &ref_cbdata);
- else if (type)
+ if (!filter->kind)
die("filter_refs: invalid type");
+ else {
+ /*
+ * For common cases where we need only branches or remotes or tags,
+ * we only iterate through those refs. If a mix of refs is needed,
+ * we iterate over all refs and filter out required refs with the help
+ * of filter_ref_kind().
+ */
+ if (filter->kind == FILTER_REFS_BRANCHES)
+ ret = for_each_fullref_in("refs/heads/", ref_filter_handler, &ref_cbdata, broken);
+ else if (filter->kind == FILTER_REFS_REMOTES)
+ ret = for_each_fullref_in("refs/remotes/", ref_filter_handler, &ref_cbdata, broken);
+ else if (filter->kind == FILTER_REFS_TAGS)
+ ret = for_each_fullref_in("refs/tags/", ref_filter_handler, &ref_cbdata, broken);
+ else if (filter->kind & FILTER_REFS_ALL)
+ ret = for_each_fullref_in("", ref_filter_handler, &ref_cbdata, broken);
+ if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
+ head_ref(ref_filter_handler, &ref_cbdata);
+ }
+
/* Filters that need revision walking */
if (filter->merge_commit)
diff --git a/ref-filter.h b/ref-filter.h
index 45026d0..0913ba9 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -13,8 +13,15 @@
#define QUOTE_PYTHON 4
#define QUOTE_TCL 8
-#define FILTER_REFS_INCLUDE_BROKEN 0x1
-#define FILTER_REFS_ALL 0x2
+#define FILTER_REFS_INCLUDE_BROKEN 0x0001
+#define FILTER_REFS_TAGS 0x0002
+#define FILTER_REFS_BRANCHES 0x0004
+#define FILTER_REFS_REMOTES 0x0008
+#define FILTER_REFS_OTHERS 0x0010
+#define FILTER_REFS_ALL (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \
+ FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)
+#define FILTER_REFS_DETACHED_HEAD 0x0020
+#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD)
struct atom_value;
@@ -27,6 +34,7 @@ struct ref_sorting {
struct ref_array_item {
unsigned char objectname[20];
int flag;
+ unsigned int kind;
const char *symref;
struct commit *commit;
struct atom_value *value;
@@ -51,6 +59,7 @@ struct ref_filter {
struct commit *merge_commit;
unsigned int with_commit_tag_algo : 1;
+ unsigned int kind;
};
struct ref_filter_cbdata {
diff --git a/refs.c b/refs.c
index 4e15f60..a9469c2 100644
--- a/refs.c
+++ b/refs.c
@@ -2108,6 +2108,15 @@ int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
}
+int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
+{
+ unsigned int flag = 0;
+
+ if (broken)
+ flag = DO_FOR_EACH_INCLUDE_BROKEN;
+ return do_for_each_ref(&ref_cache, prefix, fn, 0, flag, cb_data);
+}
+
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn fn, void *cb_data)
{
diff --git a/refs.h b/refs.h
index e9a5f32..6d30c98 100644
--- a/refs.h
+++ b/refs.h
@@ -173,6 +173,7 @@ typedef int each_ref_fn(const char *refname,
extern int head_ref(each_ref_fn fn, void *cb_data);
extern int for_each_ref(each_ref_fn fn, void *cb_data);
extern int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);
+extern int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken);
extern int for_each_tag_ref(each_ref_fn fn, void *cb_data);
extern int for_each_branch_ref(each_ref_fn fn, void *cb_data);
extern int for_each_remote_ref(each_ref_fn fn, void *cb_data);
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 08/14] ref-filter: add support for %(contents:lines=X)
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (6 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 07/14] ref-filter: add option to filter out tags, branches and remotes Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 09/14] ref-filter: add support to sort by version Karthik Nayak
` (7 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
In 'tag.c' we can print N lines from the annotation of the tag using
the '-n<num>' option. Copy code from 'tag.c' to 'ref-filter' and
modify it to support appending of N lines from the annotation of tags
to the given strbuf.
Implement %(contents:lines=X) where X lines of the given object are
obtained.
Add documentation and test for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-for-each-ref.txt | 3 ++-
builtin/tag.c | 4 +++
ref-filter.c | 45 ++++++++++++++++++++++++++++++++-
ref-filter.h | 3 ++-
t/t6302-for-each-ref-filter.sh | 52 ++++++++++++++++++++++++++++++++++++++
5 files changed, 104 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index b23412d..99d108a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -149,7 +149,8 @@ The complete message in a commit and tag object is `contents`.
Its first line is `contents:subject`, where subject is the concatenation
of all lines of the commit message up to the first blank line. The next
line is 'contents:body', where body is all of the lines after the first
-blank line. Finally, the optional GPG signature is `contents:signature`.
+blank line. The optional GPG signature is `contents:signature`. The
+first `N` lines of the message is obtained using `contents:lines=N`.
For sorting purposes, fields with numeric values sort in numeric
order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/tag.c b/builtin/tag.c
index 471d6b1..b0bc1c5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -185,6 +185,10 @@ static enum contains_result contains(struct commit *candidate,
return contains_test(candidate, want);
}
+/*
+ * Currently modified and used in ref-filter as append_lines(), will
+ * eventually be removed as we port tag.c to use ref-filter APIs.
+ */
static void show_tag_lines(const struct object_id *oid, int lines)
{
int i;
diff --git a/ref-filter.c b/ref-filter.c
index c7a8cf0..4435ec8 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -56,6 +56,7 @@ static struct {
{ "color" },
{ "align" },
{ "end" },
+ { "contents:lines" },
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
@@ -65,6 +66,11 @@ struct align {
unsigned int width;
};
+struct contents {
+ unsigned int lines;
+ struct object_id oid;
+};
+
struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
@@ -81,6 +87,7 @@ struct atom_value {
const char *s;
union {
struct align align;
+ struct contents contents;
} u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);
unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -643,6 +650,30 @@ static void find_subpos(const char *buf, unsigned long sz,
*nonsiglen = *sig - buf;
}
+/*
+ * If 'lines' is greater than 0, append that many lines from the given
+ * 'buf' of length 'size' to the given strbuf.
+ */
+static void append_lines(struct strbuf *out, const char *buf, unsigned long size, int lines)
+{
+ int i;
+ const char *sp, *eol;
+ size_t len;
+
+ sp = buf;
+
+ for (i = 0; i < lines && sp < buf + size; i++) {
+ if (i)
+ strbuf_addstr(out, "\n ");
+ eol = memchr(sp, '\n', size - (sp - buf));
+ len = eol ? eol - sp : size - (sp - buf);
+ strbuf_add(out, sp, len);
+ if (!eol)
+ break;
+ sp = eol + 1;
+ }
+}
+
/* See grab_values */
static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
{
@@ -653,6 +684,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
for (i = 0; i < used_atom_cnt; i++) {
const char *name = used_atom[i];
struct atom_value *v = &val[i];
+ const char *valp = NULL;
if (!!deref != (*name == '*'))
continue;
if (deref)
@@ -662,7 +694,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
strcmp(name, "contents") &&
strcmp(name, "contents:subject") &&
strcmp(name, "contents:body") &&
- strcmp(name, "contents:signature"))
+ strcmp(name, "contents:signature") &&
+ !starts_with(name, "contents:lines="))
continue;
if (!subpos)
find_subpos(buf, sz,
@@ -682,6 +715,16 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
v->s = xmemdupz(sigpos, siglen);
else if (!strcmp(name, "contents"))
v->s = xstrdup(subpos);
+ else if (skip_prefix(name, "contents:lines=", &valp)) {
+ struct strbuf s = STRBUF_INIT;
+ const char *contents_end = bodylen + bodypos - siglen;
+
+ if (strtoul_ui(valp, 10, &v->u.contents.lines))
+ die(_("positive value expected contents:lines=%s"), valp);
+ /* Size is the length of the message after removing the signature */
+ append_lines(&s, subpos, contents_end - subpos, v->u.contents.lines);
+ v->s = strbuf_detach(&s, NULL);
+ }
}
}
diff --git a/ref-filter.h b/ref-filter.h
index 0913ba9..ab76b22 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -59,7 +59,8 @@ struct ref_filter {
struct commit *merge_commit;
unsigned int with_commit_tag_algo : 1;
- unsigned int kind;
+ unsigned int kind,
+ lines;
};
struct ref_filter_cbdata {
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index d0c0139..428d139 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -167,4 +167,56 @@ test_expect_success 'nested alignment with quote formatting' "
test_cmp expect actual
"
+test_expect_success 'check `%(contents:lines=1)`' '
+ cat >expect <<-\EOF &&
+ master |three
+ side |four
+ odd/spot |three
+ double-tag |Annonated doubly
+ four |four
+ one |one
+ signed-tag |A signed tag message
+ three |three
+ two |two
+ EOF
+ git for-each-ref --format="%(refname:short) |%(contents:lines=1)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check `%(contents:lines=0)`' '
+ cat >expect <<-\EOF &&
+ master |
+ side |
+ odd/spot |
+ double-tag |
+ four |
+ one |
+ signed-tag |
+ three |
+ two |
+ EOF
+ git for-each-ref --format="%(refname:short) |%(contents:lines=0)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check `%(contents:lines=99999)`' '
+ cat >expect <<-\EOF &&
+ master |three
+ side |four
+ odd/spot |three
+ double-tag |Annonated doubly
+ four |four
+ one |one
+ signed-tag |A signed tag message
+ three |three
+ two |two
+ EOF
+ git for-each-ref --format="%(refname:short) |%(contents:lines=99999)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '`%(contents:lines=-1)` should fail' '
+ test_must_fail git for-each-ref --format="%(refname:short) |%(contents:lines=-1)"
+'
+
test_done
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 09/14] ref-filter: add support to sort by version
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (7 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 08/14] ref-filter: add support for %(contents:lines=X) Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 10/14] ref-filter: add option to match literal pattern Karthik Nayak
` (6 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak
From: Karthik Nayak <karthik.188@gmail.com>
Add support to sort by version using the "v:refname" and
"version:refname" option. This is achieved by using the 'versioncmp()'
function as the comparing function for qsort.
This option is included to support sorting by versions in `git tag -l`
which will eventually be ported to use ref-filter APIs.
Add documentation and tests for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-for-each-ref.txt | 3 +++
ref-filter.c | 15 ++++++++++-----
ref-filter.h | 3 ++-
t/t6302-for-each-ref-filter.sh | 36 ++++++++++++++++++++++++++++++++++++
4 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 99d108a..c5154bb 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -156,6 +156,9 @@ For sorting purposes, fields with numeric values sort in numeric
order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
All other fields are used to sort in their byte-value order.
+There is also an option to sort by versions, this can be done by using
+the fieldname `version:refname` or its alias `v:refname`.
+
In any case, a field name that refers to a field inapplicable to
the object referred by the ref does not cause an error. It
returns an empty string instead.
diff --git a/ref-filter.c b/ref-filter.c
index 4435ec8..696c0c7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -11,6 +11,8 @@
#include "ref-filter.h"
#include "revision.h"
#include "utf8.h"
+#include "git-compat-util.h"
+#include "version.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -1445,19 +1447,19 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
get_ref_atom_value(a, s->atom, &va);
get_ref_atom_value(b, s->atom, &vb);
- switch (cmp_type) {
- case FIELD_STR:
+ if (s->version)
+ cmp = versioncmp(va->s, vb->s);
+ else if (cmp_type == FIELD_STR)
cmp = strcmp(va->s, vb->s);
- break;
- default:
+ else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
cmp = 0;
else
cmp = 1;
- break;
}
+
return (s->reverse) ? -cmp : cmp;
}
@@ -1590,6 +1592,9 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
s->reverse = 1;
arg++;
}
+ if (skip_prefix(arg, "version:", &arg) ||
+ skip_prefix(arg, "v:", &arg))
+ s->version = 1;
len = strlen(arg);
s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
diff --git a/ref-filter.h b/ref-filter.h
index ab76b22..ef25b6e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -28,7 +28,8 @@ struct atom_value;
struct ref_sorting {
struct ref_sorting *next;
int atom; /* index into used_atom array (internal) */
- unsigned reverse : 1;
+ unsigned reverse : 1,
+ version : 1;
};
struct ref_array_item {
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 428d139..4bc1055 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -219,4 +219,40 @@ test_expect_success '`%(contents:lines=-1)` should fail' '
test_must_fail git for-each-ref --format="%(refname:short) |%(contents:lines=-1)"
'
+test_expect_success 'setup for version sort' '
+ test_commit foo1.3 &&
+ test_commit foo1.6 &&
+ test_commit foo1.10
+'
+
+test_expect_success 'version sort' '
+ git for-each-ref --sort=version:refname --format="%(refname:short)" refs/tags/ | grep "foo" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'version sort (shortened)' '
+ git for-each-ref --sort=v:refname --format="%(refname:short)" refs/tags/ | grep "foo" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'reverse version sort' '
+ git for-each-ref --sort=-version:refname --format="%(refname:short)" refs/tags/ | grep "foo" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.6
+ foo1.3
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 10/14] ref-filter: add option to match literal pattern
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (8 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 09/14] ref-filter: add support to sort by version Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 11/14] tag.c: use 'ref-filter' data structures Karthik Nayak
` (5 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak
From: Karthik Nayak <karthik.188@gmail.com>
Since 'ref-filter' only has an option to match path names add an
option for plain fnmatch pattern-matching.
This is to support the pattern matching options which are used in `git
tag -l` and `git branch -l` where we can match patterns like `git tag
-l foo*` which would match all tags which has a "foo*" pattern.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/for-each-ref.c | 1 +
ref-filter.c | 40 +++++++++++++++++++++++++++++++++++++---
ref-filter.h | 3 ++-
3 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 40f343b..4e9f6c2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -68,6 +68,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
git_config(git_default_config, NULL);
filter.name_patterns = argv;
+ filter.match_as_path = 1;
filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);
ref_array_sort(sorting, &array);
diff --git a/ref-filter.c b/ref-filter.c
index 696c0c7..e3024d3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1167,9 +1167,33 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
/*
* Return 1 if the refname matches one of the patterns, otherwise 0.
+ * A pattern can be a literal prefix (e.g. a refname "refs/heads/master"
+ * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
+ * matches "refs/heads/mas*", too).
+ */
+static int match_pattern(const char **patterns, const char *refname)
+{
+ /*
+ * When no '--format' option is given we need to skip the prefix
+ * for matching refs of tags and branches.
+ */
+ (void)(skip_prefix(refname, "refs/tags/", &refname) ||
+ skip_prefix(refname, "refs/heads/", &refname) ||
+ skip_prefix(refname, "refs/remotes/", &refname) ||
+ skip_prefix(refname, "refs/", &refname));
+
+ for (; *patterns; patterns++) {
+ if (!wildmatch(*patterns, refname, 0, NULL))
+ return 1;
+ }
+ return 0;
+}
+
+/*
+ * Return 1 if the refname matches one of the patterns, otherwise 0.
* A pattern can be path prefix (e.g. a refname "refs/heads/master"
- * matches a pattern "refs/heads/") or a wildcard (e.g. the same ref
- * matches "refs/heads/m*",too).
+ * matches a pattern "refs/heads/" but not "refs/heads/m") or a
+ * wildcard (e.g. the same ref matches "refs/heads/m*", too).
*/
static int match_name_as_path(const char **pattern, const char *refname)
{
@@ -1190,6 +1214,16 @@ static int match_name_as_path(const char **pattern, const char *refname)
return 0;
}
+/* Return 1 if the refname matches one of the patterns, otherwise 0. */
+static int filter_pattern_match(struct ref_filter *filter, const char *refname)
+{
+ if (!*filter->name_patterns)
+ return 1; /* No pattern always matches */
+ if (filter->match_as_path)
+ return match_name_as_path(filter->name_patterns, refname);
+ return match_pattern(filter->name_patterns, refname);
+}
+
/*
* Given a ref (sha1, refname), check if the ref belongs to the array
* of sha1s. If the given ref is a tag, check if the given tag points
@@ -1292,7 +1326,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
if (!(kind & filter->kind))
return 0;
- if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
+ if (!filter_pattern_match(filter, refname))
return 0;
if (filter->points_at.nr && !match_points_at(&filter->points_at, oid->hash, refname))
diff --git a/ref-filter.h b/ref-filter.h
index ef25b6e..a5cfa5e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -59,7 +59,8 @@ struct ref_filter {
} merge;
struct commit *merge_commit;
- unsigned int with_commit_tag_algo : 1;
+ unsigned int with_commit_tag_algo : 1,
+ match_as_path : 1;
unsigned int kind,
lines;
};
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 11/14] tag.c: use 'ref-filter' data structures
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (9 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 10/14] ref-filter: add option to match literal pattern Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 12/14] tag.c: use 'ref-filter' APIs Karthik Nayak
` (4 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak
From: Karthik Nayak <karthik.188@gmail.com>
Make 'tag.c' use 'ref-filter' data structures and make changes to
support the new data structures. This is a part of the process
of porting 'tag.c' to use 'ref-filter' APIs.
This is a temporary step before porting 'tag.c' to use 'ref-filter'
completely. As this is a temporary step, most of the code
introduced here will be removed when 'tag.c' is ported over to use
'ref-filter' APIs.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/tag.c | 106 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 57 insertions(+), 49 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index b0bc1c5..fe66f7b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -17,6 +17,7 @@
#include "gpg-interface.h"
#include "sha1-array.h"
#include "column.h"
+#include "ref-filter.h"
static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
@@ -34,15 +35,6 @@ static const char * const git_tag_usage[] = {
static int tag_sort;
-struct tag_filter {
- const char **patterns;
- int lines;
- int sort;
- struct string_list tags;
- struct commit_list *with_commit;
-};
-
-static struct sha1_array points_at;
static unsigned int colopts;
static int match_pattern(const char **patterns, const char *ref)
@@ -61,19 +53,20 @@ static int match_pattern(const char **patterns, const char *ref)
* removed as we port tag.c to use the ref-filter APIs.
*/
static const unsigned char *match_points_at(const char *refname,
- const unsigned char *sha1)
+ const unsigned char *sha1,
+ struct sha1_array *points_at)
{
const unsigned char *tagged_sha1 = NULL;
struct object *obj;
- if (sha1_array_lookup(&points_at, sha1) >= 0)
+ if (sha1_array_lookup(points_at, sha1) >= 0)
return sha1;
obj = parse_object(sha1);
if (!obj)
die(_("malformed object at '%s'"), refname);
if (obj->type == OBJ_TAG)
tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
- if (tagged_sha1 && sha1_array_lookup(&points_at, tagged_sha1) >= 0)
+ if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
return tagged_sha1;
return NULL;
}
@@ -228,12 +221,24 @@ free_return:
free(buf);
}
+static void ref_array_append(struct ref_array *array, const char *refname)
+{
+ size_t len = strlen(refname);
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);
+ memcpy(ref->refname, refname, len);
+ ref->refname[len] = '\0';
+ REALLOC_ARRAY(array->items, array->nr + 1);
+ array->items[array->nr++] = ref;
+}
+
static int show_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
- struct tag_filter *filter = cb_data;
+ struct ref_filter_cbdata *data = cb_data;
+ struct ref_array *array = data->array;
+ struct ref_filter *filter = data->filter;
- if (match_pattern(filter->patterns, refname)) {
+ if (match_pattern(filter->name_patterns, refname)) {
if (filter->with_commit) {
struct commit *commit;
@@ -244,12 +249,12 @@ static int show_reference(const char *refname, const struct object_id *oid,
return 0;
}
- if (points_at.nr && !match_points_at(refname, oid->hash))
+ if (filter->points_at.nr && !match_points_at(refname, oid->hash, &filter->points_at))
return 0;
if (!filter->lines) {
- if (filter->sort)
- string_list_append(&filter->tags, refname);
+ if (tag_sort)
+ ref_array_append(array, refname);
else
printf("%s\n", refname);
return 0;
@@ -264,36 +269,36 @@ static int show_reference(const char *refname, const struct object_id *oid,
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 versioncmp(a->string, b->string);
+ const struct ref_array_item *a = *((struct ref_array_item **)a_);
+ const struct ref_array_item *b = *((struct ref_array_item **)b_);
+ return versioncmp(a->refname, b->refname);
}
-static int list_tags(const char **patterns, int lines,
- struct commit_list *with_commit, int sort)
+static int list_tags(struct ref_filter *filter, int sort)
{
- struct tag_filter filter;
+ struct ref_array array;
+ struct ref_filter_cbdata data;
+
+ memset(&array, 0, sizeof(array));
+ data.array = &array;
+ data.filter = filter;
- filter.patterns = patterns;
- filter.lines = lines;
- filter.sort = sort;
- filter.with_commit = with_commit;
- memset(&filter.tags, 0, sizeof(filter.tags));
- filter.tags.strdup_strings = 1;
+ if (filter->lines == -1)
+ filter->lines = 0;
- for_each_tag_ref(show_reference, (void *)&filter);
+ for_each_tag_ref(show_reference, &data);
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);
+ qsort(array.items, array.nr,
+ sizeof(struct ref_array_item *), sort_by_version);
if (sort & REVERSE_SORT)
- for (i = filter.tags.nr - 1; i >= 0; i--)
- printf("%s\n", filter.tags.items[i].string);
+ for (i = array.nr - 1; i >= 0; i--)
+ printf("%s\n", array.items[i]->refname);
else
- for (i = 0; i < filter.tags.nr; i++)
- printf("%s\n", filter.tags.items[i].string);
- string_list_clear(&filter.tags, 0);
+ for (i = 0; i < array.nr; i++)
+ printf("%s\n", array.items[i]->refname);
+ ref_array_clear(&array);
}
return 0;
}
@@ -574,17 +579,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *object_ref, *tag;
struct create_tag_options opt;
char *cleanup_arg = NULL;
- int annotate = 0, force = 0, lines = -1;
int create_reflog = 0;
+ int annotate = 0, force = 0;
int cmdmode = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
- struct commit_list *with_commit = NULL;
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
+ struct ref_filter filter;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
- { OPTION_INTEGER, 'n', NULL, &lines, N_("n"),
+ { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
N_("print <n> lines of each tag message"),
PARSE_OPT_OPTARG, NULL, 1 },
OPT_CMDMODE('d', "delete", &cmdmode, N_("delete tags"), 'd'),
@@ -606,14 +611,14 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_GROUP(N_("Tag listing options")),
OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
- OPT_CONTAINS(&with_commit, N_("print only tags that contain the commit")),
- OPT_WITH(&with_commit, N_("print only tags that contain the commit")),
+ OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+ OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
{
OPTION_CALLBACK, 0, "sort", &tag_sort, N_("type"), N_("sort tags"),
PARSE_OPT_NONEG, parse_opt_sort
},
{
- OPTION_CALLBACK, 0, "points-at", &points_at, N_("object"),
+ OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
N_("print only tags of the object"), 0, parse_opt_object_name
},
OPT_END()
@@ -622,6 +627,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
git_config(git_tag_config, NULL);
memset(&opt, 0, sizeof(opt));
+ memset(&filter, 0, sizeof(filter));
+ filter.lines = -1;
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
@@ -638,7 +645,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
usage_with_options(git_tag_usage, options);
finalize_colopts(&colopts, -1);
- if (cmdmode == 'l' && lines != -1) {
+ if (cmdmode == 'l' && filter.lines != -1) {
if (explicitly_enable_column(colopts))
die(_("--column and -n are incompatible"));
colopts = 0;
@@ -651,18 +658,19 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
copts.padding = 2;
run_column_filter(colopts, &copts);
}
- if (lines != -1 && tag_sort)
+ if (filter.lines != -1 && tag_sort)
die(_("--sort and -n are incompatible"));
- ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit, tag_sort);
+ filter.name_patterns = argv;
+ ret = list_tags(&filter, tag_sort);
if (column_active(colopts))
stop_column_filter();
return ret;
}
- if (lines != -1)
+ if (filter.lines != -1)
die(_("-n option is only allowed with -l."));
- if (with_commit)
+ if (filter.with_commit)
die(_("--contains option is only allowed with -l."));
- if (points_at.nr)
+ if (filter.points_at.nr)
die(_("--points-at option is only allowed with -l."));
if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag);
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 12/14] tag.c: use 'ref-filter' APIs
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (10 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 11/14] tag.c: use 'ref-filter' data structures Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 13/14] tag.c: implement '--format' option Karthik Nayak
` (3 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak
From: Karthik Nayak <karthik.188@gmail.com>
Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting
and printing of refs. This removes most of the code used in 'tag.c'
replacing it with calls to the 'ref-filter' library.
Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'
to filter out tags based on the options set.
For printing tags we use 'show_ref_array_item()' function provided by
'ref-filter'.
We improve the sorting option provided by 'tag.c' by using the sorting
options provided by 'ref-filter'. This causes the test 'invalid sort
parameter on command line' in t7004 to fail, as 'ref-filter' throws an
error for all sorting fields which are incorrect. The test is changed
to reflect the same.
Modify documentation for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-tag.txt | 16 ++-
builtin/tag.c | 344 ++++++----------------------------------------
t/t7004-tag.sh | 8 +-
3 files changed, 52 insertions(+), 316 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 84f6496..3ac4a96 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
<tagname> [<commit> | <object>]
'git tag' -d <tagname>...
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
- [--column[=<options>] | --no-column] [--create-reflog] [<pattern>...]
+ [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>] [<pattern>...]
'git tag' -v <tagname>...
DESCRIPTION
@@ -94,14 +94,16 @@ 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 "refname"
- (lexicographic order), "version:refname" or "v:refname" (tag
+--sort=<key>::
+ Sort based on the key given. Prefix `-` to sort in
+ descending order of the value. You may use the --sort=<key> option
+ multiple times, in which case the last key becomes the primary
+ key. Also supports "version:refname" or "v:refname" (tag
names are treated as versions). The "version:refname" sort
order can also be affected by the
- "versionsort.prereleaseSuffix" configuration variable. Prepend
- "-" to reverse sort order. When this option is not given, the
- sort order defaults to the value configured for the 'tag.sort'
+ "versionsort.prereleaseSuffix" configuration variable.
+ The keys supported are the same as those in `git for-each-ref`.
+ Sort order defaults to the value configured for the 'tag.sort'
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
diff --git a/builtin/tag.c b/builtin/tag.c
index fe66f7b..27e3566 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -28,278 +28,34 @@ static const char * const git_tag_usage[] = {
NULL
};
-#define STRCMP_SORT 0 /* must be zero */
-#define VERCMP_SORT 1
-#define SORT_MASK 0x7fff
-#define REVERSE_SORT 0x8000
-
-static int tag_sort;
-
static unsigned int colopts;
-static int match_pattern(const char **patterns, const char *ref)
-{
- /* no pattern means match everything */
- if (!*patterns)
- return 1;
- for (; *patterns; patterns++)
- if (!wildmatch(*patterns, ref, 0, NULL))
- return 1;
- return 0;
-}
-
-/*
- * This is currently duplicated in ref-filter.c, and will eventually be
- * removed as we port tag.c to use the ref-filter APIs.
- */
-static const unsigned char *match_points_at(const char *refname,
- const unsigned char *sha1,
- struct sha1_array *points_at)
-{
- const unsigned char *tagged_sha1 = NULL;
- struct object *obj;
-
- if (sha1_array_lookup(points_at, sha1) >= 0)
- return sha1;
- obj = parse_object(sha1);
- if (!obj)
- die(_("malformed object at '%s'"), refname);
- if (obj->type == OBJ_TAG)
- tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
- if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
- return tagged_sha1;
- return NULL;
-}
-
-static int in_commit_list(const struct commit_list *want, struct commit *c)
-{
- for (; want; want = want->next)
- if (!hashcmp(want->item->object.sha1, c->object.sha1))
- return 1;
- return 0;
-}
-
-/*
- * The entire code segment for supporting the --contains option has been
- * copied over to ref-filter.{c,h}. This will be deleted evetually when
- * we port tag.c to use ref-filter APIs.
- */
-enum contains_result {
- CONTAINS_UNKNOWN = -1,
- CONTAINS_NO = 0,
- CONTAINS_YES = 1
-};
-
-/*
- * Test whether the candidate or one of its parents is contained in the list.
- * Do not recurse to find out, though, but return -1 if inconclusive.
- */
-static enum contains_result contains_test(struct commit *candidate,
- const struct commit_list *want)
-{
- /* was it previously marked as containing a want commit? */
- if (candidate->object.flags & TMP_MARK)
- return 1;
- /* or marked as not possibly containing a want commit? */
- if (candidate->object.flags & UNINTERESTING)
- return 0;
- /* or are we it? */
- if (in_commit_list(want, candidate)) {
- candidate->object.flags |= TMP_MARK;
- return 1;
- }
-
- if (parse_commit(candidate) < 0)
- return 0;
-
- return -1;
-}
-
-/*
- * Mimicking the real stack, this stack lives on the heap, avoiding stack
- * overflows.
- *
- * At each recursion step, the stack items points to the commits whose
- * ancestors are to be inspected.
- */
-struct stack {
- int nr, alloc;
- struct stack_entry {
- struct commit *commit;
- struct commit_list *parents;
- } *stack;
-};
-
-static void push_to_stack(struct commit *candidate, struct stack *stack)
-{
- int index = stack->nr++;
- ALLOC_GROW(stack->stack, stack->nr, stack->alloc);
- stack->stack[index].commit = candidate;
- stack->stack[index].parents = candidate->parents;
-}
-
-static enum contains_result contains(struct commit *candidate,
- const struct commit_list *want)
-{
- struct stack stack = { 0, 0, NULL };
- int result = contains_test(candidate, want);
-
- if (result != CONTAINS_UNKNOWN)
- return result;
-
- push_to_stack(candidate, &stack);
- while (stack.nr) {
- struct stack_entry *entry = &stack.stack[stack.nr - 1];
- struct commit *commit = entry->commit;
- struct commit_list *parents = entry->parents;
-
- if (!parents) {
- commit->object.flags |= UNINTERESTING;
- stack.nr--;
- }
- /*
- * If we just popped the stack, parents->item has been marked,
- * therefore contains_test will return a meaningful 0 or 1.
- */
- else switch (contains_test(parents->item, want)) {
- case CONTAINS_YES:
- commit->object.flags |= TMP_MARK;
- stack.nr--;
- break;
- case CONTAINS_NO:
- entry->parents = parents->next;
- break;
- case CONTAINS_UNKNOWN:
- push_to_stack(parents->item, &stack);
- break;
- }
- }
- free(stack.stack);
- return contains_test(candidate, want);
-}
-
-/*
- * Currently modified and used in ref-filter as append_lines(), will
- * eventually be removed as we port tag.c to use ref-filter APIs.
- */
-static void show_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;
-
- buf = read_sha1_file(oid->hash, &type, &size);
- if (!buf)
- die_errno("unable to read object %s", oid_to_hex(oid));
- if (type != OBJ_COMMIT && type != OBJ_TAG)
- goto free_return;
- if (!size)
- die("an empty %s object %s?",
- typename(type), oid_to_hex(oid));
-
- /* skip header */
- sp = strstr(buf, "\n\n");
- if (!sp)
- goto free_return;
-
- /* only take up to "lines" lines, and strip the signature from a tag */
- if (type == OBJ_TAG)
- size = parse_signature(buf, size);
- for (i = 0, sp += 2; i < lines && sp < buf + size; i++) {
- if (i)
- printf("\n ");
- eol = memchr(sp, '\n', size - (sp - buf));
- len = eol ? eol - sp : size - (sp - buf);
- fwrite(sp, len, 1, stdout);
- if (!eol)
- break;
- sp = eol + 1;
- }
-free_return:
- free(buf);
-}
-
-static void ref_array_append(struct ref_array *array, const char *refname)
-{
- size_t len = strlen(refname);
- struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);
- memcpy(ref->refname, refname, len);
- ref->refname[len] = '\0';
- REALLOC_ARRAY(array->items, array->nr + 1);
- array->items[array->nr++] = ref;
-}
-
-static int show_reference(const char *refname, const struct object_id *oid,
- int flag, void *cb_data)
-{
- struct ref_filter_cbdata *data = cb_data;
- struct ref_array *array = data->array;
- struct ref_filter *filter = data->filter;
-
- if (match_pattern(filter->name_patterns, refname)) {
- if (filter->with_commit) {
- struct commit *commit;
-
- commit = lookup_commit_reference_gently(oid->hash, 1);
- if (!commit)
- return 0;
- if (!contains(commit, filter->with_commit))
- return 0;
- }
-
- if (filter->points_at.nr && !match_points_at(refname, oid->hash, &filter->points_at))
- return 0;
-
- if (!filter->lines) {
- if (tag_sort)
- ref_array_append(array, refname);
- else
- printf("%s\n", refname);
- return 0;
- }
- printf("%-15s ", refname);
- show_tag_lines(oid, filter->lines);
- putchar('\n');
- }
-
- return 0;
-}
-
-static int sort_by_version(const void *a_, const void *b_)
-{
- const struct ref_array_item *a = *((struct ref_array_item **)a_);
- const struct ref_array_item *b = *((struct ref_array_item **)b_);
- return versioncmp(a->refname, b->refname);
-}
-
-static int list_tags(struct ref_filter *filter, int sort)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
{
struct ref_array array;
- struct ref_filter_cbdata data;
+ char *format, *to_free = NULL;
+ int i;
memset(&array, 0, sizeof(array));
- data.array = &array;
- data.filter = filter;
if (filter->lines == -1)
filter->lines = 0;
- for_each_tag_ref(show_reference, &data);
- if (sort) {
- int i;
- if ((sort & SORT_MASK) == VERCMP_SORT)
- qsort(array.items, array.nr,
- sizeof(struct ref_array_item *), sort_by_version);
- if (sort & REVERSE_SORT)
- for (i = array.nr - 1; i >= 0; i--)
- printf("%s\n", array.items[i]->refname);
- else
- for (i = 0; i < array.nr; i++)
- printf("%s\n", array.items[i]->refname);
- ref_array_clear(&array);
- }
+ if (filter->lines)
+ format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
+ "%%(contents:lines=%d)", filter->lines);
+ else
+ format = "%(refname:short)";
+
+ verify_ref_format(format);
+ filter_refs(&array, filter, FILTER_REFS_TAGS);
+ ref_array_sort(sorting, &array);
+
+ for (i = 0; i < array.nr; i++)
+ show_ref_array_item(array.items[i], format, 0);
+ ref_array_clear(&array);
+ free(to_free);
+
return 0;
}
@@ -366,35 +122,26 @@ static const char tag_template_nocleanup[] =
"Lines starting with '%c' will be kept; you may remove them"
" yourself if you want to.\n");
-/*
- * Parse a sort string, and return 0 if parsed successfully. Will return
- * non-zero when the sort string does not parse into a known type. If var is
- * given, the error message becomes a warning and includes information about
- * the configuration value.
- */
-static int parse_sort_string(const char *var, const char *arg, int *sort)
+/* Parse arg given and add it the ref_sorting array */
+static int parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)
{
- int type = 0, flags = 0;
-
- if (skip_prefix(arg, "-", &arg))
- flags |= REVERSE_SORT;
+ struct ref_sorting *s;
+ int len;
- if (skip_prefix(arg, "version:", &arg) || skip_prefix(arg, "v:", &arg))
- type = VERCMP_SORT;
- else
- type = STRCMP_SORT;
+ s = xcalloc(1, sizeof(*s));
+ s->next = *sorting_tail;
+ *sorting_tail = s;
- if (strcmp(arg, "refname")) {
- if (!var)
- return error(_("unsupported sort specification '%s'"), arg);
- else {
- warning(_("unsupported sort specification '%s' in variable '%s'"),
- var, arg);
- return -1;
- }
+ if (*arg == '-') {
+ s->reverse = 1;
+ arg++;
}
+ if (skip_prefix(arg, "version:", &arg) ||
+ skip_prefix(arg, "v:", &arg))
+ s->version = 1;
- *sort = (type | flags);
+ len = strlen(arg);
+ s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
}
@@ -402,11 +149,12 @@ static int parse_sort_string(const char *var, const char *arg, int *sort)
static int git_tag_config(const char *var, const char *value, void *cb)
{
int status;
+ struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
if (!strcmp(var, "tag.sort")) {
if (!value)
return config_error_nonbool(var);
- parse_sort_string(var, value, &tag_sort);
+ parse_sorting_string(value, sorting_tail);
return 0;
}
@@ -564,13 +312,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
return check_refname_format(sb->buf, 0);
}
-static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
-{
- int *sort = opt->value;
-
- return parse_sort_string(NULL, arg, sort);
-}
-
int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
@@ -587,6 +328,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
+ static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -613,10 +355,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
- {
- OPTION_CALLBACK, 0, "sort", &tag_sort, N_("type"), N_("sort tags"),
- PARSE_OPT_NONEG, parse_opt_sort
- },
+ OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+ N_("field name to sort on"), &parse_opt_ref_sorting),
{
OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
N_("print only tags of the object"), 0, parse_opt_object_name
@@ -624,7 +364,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_END()
};
- git_config(git_tag_config, NULL);
+ git_config(git_tag_config, sorting_tail);
memset(&opt, 0, sizeof(opt));
memset(&filter, 0, sizeof(filter));
@@ -650,6 +390,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("--column and -n are incompatible"));
colopts = 0;
}
+ if (!sorting)
+ sorting = ref_default_sorting();
if (cmdmode == 'l') {
int ret;
if (column_active(colopts)) {
@@ -658,10 +400,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
copts.padding = 2;
run_column_filter(colopts, &copts);
}
- if (filter.lines != -1 && tag_sort)
- die(_("--sort and -n are incompatible"));
filter.name_patterns = argv;
- ret = list_tags(&filter, tag_sort);
+ ret = list_tags(&filter, sorting);
if (column_active(colopts))
stop_column_filter();
return ret;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d31788c..84153ef 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1462,13 +1462,7 @@ test_expect_success 'invalid sort parameter on command line' '
test_expect_success 'invalid sort parameter in configuratoin' '
git config tag.sort "v:notvalid" &&
- git tag -l "foo*" >actual &&
- cat >expect <<-\EOF &&
- foo1.10
- foo1.3
- foo1.6
- EOF
- test_cmp expect actual
+ test_must_fail git tag -l "foo*"
'
test_expect_success 'version sort with prerelease reordering' '
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 13/14] tag.c: implement '--format' option
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (11 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 12/14] tag.c: use 'ref-filter' APIs Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-05 18:52 ` [PATCH v16 14/14] tag.c: implement '--merged' and '--no-merged' options Karthik Nayak
` (2 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Implement the '--format' option provided by 'ref-filter'.
This lets the user list tags as per desired format similar
to the implementation in 'git for-each-ref'.
Add tests and documentation for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-tag.txt | 8 +++++++-
builtin/tag.c | 22 +++++++++++++---------
t/t7004-tag.sh | 12 ++++++++++++
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 3ac4a96..0c7f4e6 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,8 @@ SYNOPSIS
<tagname> [<commit> | <object>]
'git tag' -d <tagname>...
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
- [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>] [<pattern>...]
+ [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
+ [--format=<format>] [<pattern>...]
'git tag' -v <tagname>...
DESCRIPTION
@@ -158,6 +159,11 @@ This option is only applicable when listing tags without annotation lines.
The object that the new tag will refer to, usually a commit.
Defaults to HEAD.
+<format>::
+ A string that interpolates `%(fieldname)` from the object
+ pointed at by a ref being shown. The format is the same as
+ that of linkgit:git-for-each-ref[1]. When unspecified,
+ defaults to `%(refname:short)`.
CONFIGURATION
-------------
diff --git a/builtin/tag.c b/builtin/tag.c
index 27e3566..f9c56ac 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -23,17 +23,17 @@ static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
N_("git tag -d <tagname>..."),
N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
- "\n\t\t[<pattern>...]"),
+ "\n\t\t[--format=<format>] [<pattern>...]"),
N_("git tag -v <tagname>..."),
NULL
};
static unsigned int colopts;
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
{
struct ref_array array;
- char *format, *to_free = NULL;
+ char *to_free = NULL;
int i;
memset(&array, 0, sizeof(array));
@@ -41,11 +41,13 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
if (filter->lines == -1)
filter->lines = 0;
- if (filter->lines)
- format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
- "%%(contents:lines=%d)", filter->lines);
- else
- format = "%(refname:short)";
+ if (!format) {
+ if (filter->lines)
+ format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
+ "%%(contents:lines=%d)", filter->lines);
+ else
+ format = "%(refname:short)";
+ }
verify_ref_format(format);
filter_refs(&array, filter, FILTER_REFS_TAGS);
@@ -329,6 +331,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
+ const char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -361,6 +364,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
N_("print only tags of the object"), 0, parse_opt_object_name
},
+ OPT_STRING( 0 , "format", &format, N_("format"), N_("format to use for the output")),
OPT_END()
};
@@ -401,7 +405,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
run_column_filter(colopts, &copts);
}
filter.name_patterns = argv;
- ret = list_tags(&filter, sorting);
+ ret = list_tags(&filter, sorting, format);
if (column_active(colopts))
stop_column_filter();
return ret;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 84153ef..8987fb1 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1519,4 +1519,16 @@ EOF"
test_cmp expect actual
'
+test_expect_success '--format should list tags as per format given' '
+ cat >expect <<-\EOF &&
+ refname : refs/tags/foo1.10
+ refname : refs/tags/foo1.3
+ refname : refs/tags/foo1.6
+ refname : refs/tags/foo1.6-rc1
+ refname : refs/tags/foo1.6-rc2
+ EOF
+ git tag -l --format="refname : %(refname)" "foo*" >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v16 14/14] tag.c: implement '--merged' and '--no-merged' options
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (12 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 13/14] tag.c: implement '--format' option Karthik Nayak
@ 2015-09-05 18:52 ` Karthik Nayak
2015-09-06 18:49 ` [PATCH v16 00/14] port tag.c to use ref-filter APIs Matthieu Moy
2015-09-07 6:33 ` Junio C Hamano
15 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-05 18:52 UTC (permalink / raw)
To: git; +Cc: christian.couder, Matthieu.Moy, gitster, Karthik Nayak,
Karthik Nayak
Use 'ref-filter' APIs to implement the '--merged' and '--no-merged'
options into 'tag.c'. The '--merged' option lets the user to only list
tags merged into the named commit. The '--no-merged' option lets the
user to only list tags not merged into the named commit. If no object
is provided it assumes HEAD as the object.
Add documentation and tests for the same.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-tag.txt | 7 ++++++-
builtin/tag.c | 6 +++++-
t/t7004-tag.sh | 27 +++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 0c7f4e6..3803bf7 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -14,7 +14,7 @@ SYNOPSIS
'git tag' -d <tagname>...
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
- [--format=<format>] [<pattern>...]
+ [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
'git tag' -v <tagname>...
DESCRIPTION
@@ -165,6 +165,11 @@ This option is only applicable when listing tags without annotation lines.
that of linkgit:git-for-each-ref[1]. When unspecified,
defaults to `%(refname:short)`.
+--[no-]merged [<commit>]::
+ Only list tags whose tips are reachable, or not reachable
+ if '--no-merged' is used, from the specified commit ('HEAD'
+ if not specified).
+
CONFIGURATION
-------------
By default, 'git tag' in sign-with-default mode (-s) will use your
diff --git a/builtin/tag.c b/builtin/tag.c
index f9c56ac..f55dfda 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
N_("git tag -d <tagname>..."),
N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
- "\n\t\t[--format=<format>] [<pattern>...]"),
+ "\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
N_("git tag -v <tagname>..."),
NULL
};
@@ -358,6 +358,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+ OPT_MERGED(&filter, N_("print only tags that are merged")),
+ OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
N_("field name to sort on"), &parse_opt_ref_sorting),
{
@@ -416,6 +418,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("--contains option is only allowed with -l."));
if (filter.points_at.nr)
die(_("--points-at option is only allowed with -l."));
+ if (filter.merge_commit)
+ die(_("--merged and --no-merged option are only allowed with -l"));
if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag);
if (cmdmode == 'v')
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8987fb1..3dd2f51 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1531,4 +1531,31 @@ test_expect_success '--format should list tags as per format given' '
test_cmp expect actual
'
+test_expect_success 'setup --merged test tags' '
+ git tag mergetest-1 HEAD~2 &&
+ git tag mergetest-2 HEAD~1 &&
+ git tag mergetest-3 HEAD
+'
+
+test_expect_success '--merged cannot be used in non-list mode' '
+ test_must_fail git tag --merged=mergetest-2 foo
+'
+
+test_expect_success '--merged shows merged tags' '
+ cat >expect <<-\EOF &&
+ mergetest-1
+ mergetest-2
+ EOF
+ git tag -l --merged=mergetest-2 mergetest-* >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--no-merged show unmerged tags' '
+ cat >expect <<-\EOF &&
+ mergetest-3
+ EOF
+ git tag -l --no-merged=mergetest-2 mergetest-* >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.5.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (13 preceding siblings ...)
2015-09-05 18:52 ` [PATCH v16 14/14] tag.c: implement '--merged' and '--no-merged' options Karthik Nayak
@ 2015-09-06 18:49 ` Matthieu Moy
2015-09-06 19:16 ` Karthik Nayak
2015-09-07 6:33 ` Junio C Hamano
15 siblings, 1 reply; 30+ messages in thread
From: Matthieu Moy @ 2015-09-06 18:49 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, christian.couder, gitster
Karthik Nayak <karthik.188@gmail.com> writes:
> @@ -705,9 +719,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
> v->s = xstrdup(subpos);
> else if (skip_prefix(name, "contents:lines=", &valp)) {
> struct strbuf s = STRBUF_INIT;
> + const char *contents_end = bodylen + bodypos - siglen;
> +
> if (strtoul_ui(valp, 10, &v->u.contents.lines))
> - die(_("positive width expected contents:lines=%s"), valp);
> - append_lines(&s, subpos, sublen + bodylen - siglen, v->u.contents.lines);
> + die(_("positive value expected contents:lines=%s"), valp);
> + /* Size is the length of the message after removing the signature */
Nit: double-space after /*.
> -test_expect_success 'alignment with format quote' '
> - cat >expect <<-EOF &&
> - refname is ${sq} ${sq}\\${sq}${sq}master${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}side${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}odd/spot${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}double-tag${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}four${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}one${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}signed-tag${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}three${sq}\\${sq}${sq} ${sq}|
> - refname is ${sq} ${sq}\\${sq}${sq}two${sq}\\${sq}${sq} ${sq}|
> +test_expect_success 'alignment with format quote' "
> + cat >expect <<-\EOF &&
> + |' master| A U Thor '|
> + |' side| A U Thor '|
> + |' odd/spot| A U Thor '|
> + |' double-tag| '|
> + |' four| A U Thor '|
> + |' one| A U Thor '|
> + |' signed-tag| '|
> + |' three| A U Thor '|
> + |' two| A U Thor '|
> EOF
> - git for-each-ref --shell --format="refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|" >actual &&
> + git for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&
> test_cmp expect actual
> -'
> +"
The new test is indeed easier to read, but you're not testing anymore
the consequence of a single-quote within the aligned string.
An implementation that just adds single-quotes around the string without
turning ' into '\'' would be broken but still pass the test.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-06 18:49 ` [PATCH v16 00/14] port tag.c to use ref-filter APIs Matthieu Moy
@ 2015-09-06 19:16 ` Karthik Nayak
0 siblings, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-06 19:16 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Git, Christian Couder, Junio C Hamano
On Mon, Sep 7, 2015 at 12:19 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> @@ -705,9 +719,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>> v->s = xstrdup(subpos);
>> else if (skip_prefix(name, "contents:lines=", &valp)) {
>> struct strbuf s = STRBUF_INIT;
>> + const char *contents_end = bodylen + bodypos - siglen;
>> +
>> if (strtoul_ui(valp, 10, &v->u.contents.lines))
>> - die(_("positive width expected contents:lines=%s"), valp);
>> - append_lines(&s, subpos, sublen + bodylen - siglen, v->u.contents.lines);
>> + die(_("positive value expected contents:lines=%s"), valp);
>> + /* Size is the length of the message after removing the signature */
>
> Nit: double-space after /*.
>
>> -test_expect_success 'alignment with format quote' '
>> - cat >expect <<-EOF &&
>> - refname is ${sq} ${sq}\\${sq}${sq}master${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}side${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}odd/spot${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}double-tag${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}four${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}one${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}signed-tag${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}three${sq}\\${sq}${sq} ${sq}|
>> - refname is ${sq} ${sq}\\${sq}${sq}two${sq}\\${sq}${sq} ${sq}|
>> +test_expect_success 'alignment with format quote' "
>> + cat >expect <<-\EOF &&
>> + |' master| A U Thor '|
>> + |' side| A U Thor '|
>> + |' odd/spot| A U Thor '|
>> + |' double-tag| '|
>> + |' four| A U Thor '|
>> + |' one| A U Thor '|
>> + |' signed-tag| '|
>> + |' three| A U Thor '|
>> + |' two| A U Thor '|
>> EOF
>> - git for-each-ref --shell --format="refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|" >actual &&
>> + git for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&
>> test_cmp expect actual
>> -'
>> +"
>
> The new test is indeed easier to read, but you're not testing anymore
> the consequence of a single-quote within the aligned string.
>
> An implementation that just adds single-quotes around the string without
> turning ' into '\'' would be broken but still pass the test.
>
You're right, should squash this in
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index d0c0139..778c33b 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -137,17 +137,17 @@ test_expect_success 'right alignment' '
test_expect_success 'alignment with format quote' "
cat >expect <<-\EOF &&
- |' master| A U Thor '|
- |' side| A U Thor '|
- |' odd/spot| A U Thor '|
- |' double-tag| '|
- |' four| A U Thor '|
- |' one| A U Thor '|
- |' signed-tag| '|
- |' three| A U Thor '|
- |' two| A U Thor '|
+ |' '\''master| A U Thor'\'' '|
+ |' '\''side| A U Thor'\'' '|
+ |' '\''odd/spot| A U Thor'\'' '|
+ |' '\''double-tag| '\'' '|
+ |' '\''four| A U Thor'\'' '|
+ |' '\''one| A U Thor'\'' '|
+ |' '\''signed-tag| '\'' '|
+ |' '\''three| A U Thor'\'' '|
+ |' '\''two| A U Thor'\'' '|
EOF
- git for-each-ref --shell
--format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|'
>actual &&
+ git for-each-ref --shell
--format=\"|%(align:30,middle)'%(refname:short)|
%(authorname)'%(end)|\" >actual &&
test_cmp expect actual
"
--
Regards,
Karthik Nayak
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-05 18:52 [PATCH v16 00/14] port tag.c to use ref-filter APIs Karthik Nayak
` (14 preceding siblings ...)
2015-09-06 18:49 ` [PATCH v16 00/14] port tag.c to use ref-filter APIs Matthieu Moy
@ 2015-09-07 6:33 ` Junio C Hamano
2015-09-07 6:42 ` Matthieu Moy
2015-09-07 13:56 ` Karthik Nayak
15 siblings, 2 replies; 30+ messages in thread
From: Junio C Hamano @ 2015-09-07 6:33 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, christian.couder, Matthieu.Moy
Karthik Nayak <karthik.188@gmail.com> writes:
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index d039f40..c5154bb 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -128,13 +128,14 @@ color::
> are described in `color.branch.*`.
>
> align::
> - Left-, middle-, or right-align the content between %(align:..)
> + Left-, middle-, or right-align the content between %(align:...)
> and %(end). Followed by `:<width>,<position>`, where the
> `<position>` is either left, right or middle and `<width>` is
> the total length of the content with alignment. If the
> contents length is more than the width then no alignment is
> performed. If used with '--quote' everything in between
> - %(align:..) and %(end) is quoted.
> + %(align:...) and %(end) is quoted, but if nested then only the
> + topmost level performs quoting.
I am not sure if the description of quote belongs to "align". Isn't
the general rule at the endgame when there are more opening things
that would buffer its effect up to the corresponding %(end):
- Some atoms like %(align) and %(if) always require a matching
%(end). We call them "opening atoms" and sometimes denote them
as %($open).
- When a scripting language specific quoting is in effect, except
for opening atoms, replacement from every %(atom) is quoted when
and only when it appears at the top-level (that is, when it
appears outside %($open)...%(end)).
- When a scripting language specific quoting is in effect,
everything between a top-level opening atom and its matching
%(end) is evaluated according to the semantics of the opening
atom and its result is quoted.
To put the third item above in a different way, a matching
%($open)... %(end) pair behaves as if it is a single normal atom,
from the point of view of the quoting rule. All top-level atoms are
invidivually quoted, whether they are normal atoms or open/end pairs.
And that rule is not limited to %(align).
I am flexible with the terminology, but the point is that I think
the quoting rules are better be specified _outside_ the description
of a particular atom, but as a general rule.
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9fa1400..f55dfda 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
>
> if (!format) {
> if (filter->lines)
> - format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
> - filter->lines);
> + format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
> + "%%(contents:lines=%d)", filter->lines);
This line still looks overlong. Would it help to stop spelling this
as a double "a = b = overlong expression" assignment?
> /*
> * Perform quote formatting when the stack element is that of
> - * a modifier atom and right above the first stack element.
> + * a supporting atom. If nested then perform quote formatting
> + * only on the topmost supporting atom.
> */
> if (!state->stack->prev->prev) {
> quote_formatting(&s, current->output.buf, state->quote_style);
> @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta
> pop_stack_element(&state->stack);
> }
>
> +static int match_atom_name(const char *name, const char *atom_name, const char **val)
> +{
> + const char *body;
> +
> + if (!skip_prefix(name, atom_name, &body))
> + return 0; /* doesn't even begin with "atom_name" */
> + if (!body[0] || !body[1]) {
> + *val = NULL; /* %(atom_name) and no customization */
> + return 1;
> + }
This if condition is puzzling. !body[0] would mean the name was
exactly atom_name, which is what you want to catch.
But the other part does not make any sense to me. what does
(body[0] != '\0' && !body[1]) mean? atom_name asked for was "align"
and name was "aligna"? That is not "%(align) and no customization".
> + if (body[0] != ':')
> + return 0; /* "atom_namefoo" is not "atom_name" or "atom_name:..." */
This one makes sense to me.
> + *val = body + 1; /* "atomname:val" */
Spell that "atom_name:val" perhaps?
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-07 6:33 ` Junio C Hamano
@ 2015-09-07 6:42 ` Matthieu Moy
2015-09-07 13:56 ` Karthik Nayak
1 sibling, 0 replies; 30+ messages in thread
From: Matthieu Moy @ 2015-09-07 6:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Karthik Nayak, git, christian.couder
Junio C Hamano <gitster@pobox.com> writes:
> I am flexible with the terminology, but the point is that I think
> the quoting rules are better be specified _outside_ the description
> of a particular atom, but as a general rule.
I agree, but for now we have only one %($open) ... %(end) pair, so it
makes sense to me to leave the series as-is and to move some of the
explanations in a general section in the next series (that will
introduce %(if) or another opening atom).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-07 6:33 ` Junio C Hamano
2015-09-07 6:42 ` Matthieu Moy
@ 2015-09-07 13:56 ` Karthik Nayak
2015-09-07 14:05 ` Matthieu Moy
2015-09-08 18:20 ` Junio C Hamano
1 sibling, 2 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-07 13:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Christian Couder, Matthieu Moy
On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> index d039f40..c5154bb 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -128,13 +128,14 @@ color::
>> are described in `color.branch.*`.
>>
>> align::
>> - Left-, middle-, or right-align the content between %(align:..)
>> + Left-, middle-, or right-align the content between %(align:...)
>> and %(end). Followed by `:<width>,<position>`, where the
>> `<position>` is either left, right or middle and `<width>` is
>> the total length of the content with alignment. If the
>> contents length is more than the width then no alignment is
>> performed. If used with '--quote' everything in between
>> - %(align:..) and %(end) is quoted.
>> + %(align:...) and %(end) is quoted, but if nested then only the
>> + topmost level performs quoting.
>
> I am not sure if the description of quote belongs to "align". Isn't
> the general rule at the endgame when there are more opening things
> that would buffer its effect up to the corresponding %(end):
>
> - Some atoms like %(align) and %(if) always require a matching
> %(end). We call them "opening atoms" and sometimes denote them
> as %($open).
>
> - When a scripting language specific quoting is in effect, except
> for opening atoms, replacement from every %(atom) is quoted when
> and only when it appears at the top-level (that is, when it
> appears outside %($open)...%(end)).
>
> - When a scripting language specific quoting is in effect,
> everything between a top-level opening atom and its matching
> %(end) is evaluated according to the semantics of the opening
> atom and its result is quoted.
>
> To put the third item above in a different way, a matching
> %($open)... %(end) pair behaves as if it is a single normal atom,
> from the point of view of the quoting rule. All top-level atoms are
> invidivually quoted, whether they are normal atoms or open/end pairs.
> And that rule is not limited to %(align).
>
> I am flexible with the terminology, but the point is that I think
> the quoting rules are better be specified _outside_ the description
> of a particular atom, but as a general rule.
>
I definitely agree, but like Matthieu said, corrently we have only
one such atom and it makes sense to note this behaviour under that.
When we get %(if) to work we could move this over to a more general
section?
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index 9fa1400..f55dfda 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
>>
>> if (!format) {
>> if (filter->lines)
>> - format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>> - filter->lines);
>> + format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
>> + "%%(contents:lines=%d)", filter->lines);
>
> This line still looks overlong. Would it help to stop spelling this
> as a double "a = b = overlong expression" assignment?
>
I'm not sure, I get what you mean.
>> /*
>> * Perform quote formatting when the stack element is that of
>> - * a modifier atom and right above the first stack element.
>> + * a supporting atom. If nested then perform quote formatting
>> + * only on the topmost supporting atom.
>> */
>> if (!state->stack->prev->prev) {
>> quote_formatting(&s, current->output.buf, state->quote_style);
>> @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta
>> pop_stack_element(&state->stack);
>> }
>>
>> +static int match_atom_name(const char *name, const char *atom_name, const char **val)
>> +{
>> + const char *body;
>> +
>> + if (!skip_prefix(name, atom_name, &body))
>> + return 0; /* doesn't even begin with "atom_name" */
>> + if (!body[0] || !body[1]) {
>> + *val = NULL; /* %(atom_name) and no customization */
>> + return 1;
>> + }
>
> This if condition is puzzling. !body[0] would mean the name was
> exactly atom_name, which is what you want to catch.
>
> But the other part does not make any sense to me. what does
> (body[0] != '\0' && !body[1]) mean? atom_name asked for was "align"
> and name was "aligna"? That is not "%(align) and no customization".
>
The idea was to ensure that the atom doesn't end at the ':'.
>> + if (body[0] != ':')
>> + return 0; /* "atom_namefoo" is not "atom_name" or "atom_name:..." */
>
> This one makes sense to me.
>
>> + *val = body + 1; /* "atomname:val" */
>
> Spell that "atom_name:val" perhaps?
Yeah will do.
--
Regards,
Karthik Nayak
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-07 13:56 ` Karthik Nayak
@ 2015-09-07 14:05 ` Matthieu Moy
2015-09-08 5:43 ` Karthik Nayak
2015-09-08 18:20 ` Junio C Hamano
2015-09-08 18:20 ` Junio C Hamano
1 sibling, 2 replies; 30+ messages in thread
From: Matthieu Moy @ 2015-09-07 14:05 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Junio C Hamano, Git, Christian Couder
Karthik Nayak <karthik.188@gmail.com> writes:
> On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> diff --git a/builtin/tag.c b/builtin/tag.c
>>> index 9fa1400..f55dfda 100644
>>> --- a/builtin/tag.c
>>> +++ b/builtin/tag.c
>>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
>>>
>>> if (!format) {
>>> if (filter->lines)
>>> - format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>>> - filter->lines);
>>> + format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
>>> + "%%(contents:lines=%d)", filter->lines);
>>
>> This line still looks overlong. Would it help to stop spelling this
>> as a double "a = b = overlong expression" assignment?
>>
>
> I'm not sure, I get what you mean.
I guess
format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
filter->lines);
to_free = format;
(still 83 columns + indentation, but that's a bit shorter than your
version).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-07 14:05 ` Matthieu Moy
@ 2015-09-08 5:43 ` Karthik Nayak
2015-09-08 18:20 ` Junio C Hamano
1 sibling, 0 replies; 30+ messages in thread
From: Karthik Nayak @ 2015-09-08 5:43 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, Git, Christian Couder
On Mon, Sep 7, 2015 at 7:35 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> diff --git a/builtin/tag.c b/builtin/tag.c
>>>> index 9fa1400..f55dfda 100644
>>>> --- a/builtin/tag.c
>>>> +++ b/builtin/tag.c
>>>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
>>>>
>>>> if (!format) {
>>>> if (filter->lines)
>>>> - format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>>>> - filter->lines);
>>>> + format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
>>>> + "%%(contents:lines=%d)", filter->lines);
>>>
>>> This line still looks overlong. Would it help to stop spelling this
>>> as a double "a = b = overlong expression" assignment?
>>>
>>
>> I'm not sure, I get what you mean.
>
> I guess
>
> format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
> filter->lines);
> to_free = format;
>
> (still 83 columns + indentation, but that's a bit shorter than your
> version).
Also we could drop left, its default anyways.
--
Regards,
Karthik Nayak
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-07 14:05 ` Matthieu Moy
2015-09-08 5:43 ` Karthik Nayak
@ 2015-09-08 18:20 ` Junio C Hamano
2015-09-09 17:37 ` Karthik Nayak
1 sibling, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2015-09-08 18:20 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Karthik Nayak, Git, Christian Couder
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>> This line still looks overlong. Would it help to stop spelling this
>>> as a double "a = b = overlong expression" assignment?
>>>
>>
>> I'm not sure, I get what you mean.
>
> I guess
>
> format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
> filter->lines);
> to_free = format;
>
> (still 83 columns + indentation, but that's a bit shorter than your
> version).
Sure. This may also be possible
xstrfmt("%s %%(contents:lines=%d)",
"%(align:15,left)%(refname:short)%(end)", filter->lines);
and highlights that the only thing that is variable is the number
of lines, which might be better.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-08 18:20 ` Junio C Hamano
@ 2015-09-09 17:37 ` Karthik Nayak
2015-09-09 19:18 ` Junio C Hamano
0 siblings, 1 reply; 30+ messages in thread
From: Karthik Nayak @ 2015-09-09 17:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, Git, Christian Couder
On Tue, Sep 8, 2015 at 11:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>>>> This line still looks overlong. Would it help to stop spelling this
>>>> as a double "a = b = overlong expression" assignment?
>>>>
>>>
>>> I'm not sure, I get what you mean.
>>
>> I guess
>>
>> format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>> filter->lines);
>> to_free = format;
>>
>> (still 83 columns + indentation, but that's a bit shorter than your
>> version).
>
> Sure. This may also be possible
>
> xstrfmt("%s %%(contents:lines=%d)",
> "%(align:15,left)%(refname:short)%(end)", filter->lines);
>
> and highlights that the only thing that is variable is the number
> of lines, which might be better.
Yeah looks better, thanks.
--
Regards,
Karthik Nayak
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-09 17:37 ` Karthik Nayak
@ 2015-09-09 19:18 ` Junio C Hamano
0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2015-09-09 19:18 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Matthieu Moy, Git, Christian Couder
Ehh, I was only kidding, though...
On Wed, Sep 9, 2015 at 10:37 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
> On Tue, Sep 8, 2015 at 11:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>>>> This line still looks overlong. Would it help to stop spelling this
>>>>> as a double "a = b = overlong expression" assignment?
>>>>>
>>>>
>>>> I'm not sure, I get what you mean.
>>>
>>> I guess
>>>
>>> format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>>> filter->lines);
>>> to_free = format;
>>>
>>> (still 83 columns + indentation, but that's a bit shorter than your
>>> version).
>>
>> Sure. This may also be possible
>>
>> xstrfmt("%s %%(contents:lines=%d)",
>> "%(align:15,left)%(refname:short)%(end)", filter->lines);
>>
>> and highlights that the only thing that is variable is the number
>> of lines, which might be better.
>
> Yeah looks better, thanks.
>
> --
> Regards,
> Karthik Nayak
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs
2015-09-07 13:56 ` Karthik Nayak
2015-09-07 14:05 ` Matthieu Moy
@ 2015-09-08 18:20 ` Junio C Hamano
1 sibling, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2015-09-08 18:20 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Git, Christian Couder, Matthieu Moy
Karthik Nayak <karthik.188@gmail.com> writes:
>> I am flexible with the terminology, but the point is that I think
>> the quoting rules are better be specified _outside_ the description
>> of a particular atom, but as a general rule.
>
> I definitely agree, but like Matthieu said, corrently we have only
> one such atom and it makes sense to note this behaviour under that.
> When we get %(if) to work we could move this over to a more general
> section?
Sure.
^ permalink raw reply [flat|nested] 30+ messages in thread