[PATCH v2 00/10] port branch.c to use ref-filter's printing options

classic Classic list List threaded Threaded
66 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 00/10] port branch.c to use ref-filter's printing options

Karthik Nayak
This is part of unification of the commands 'git tag -l, git branch -l
and git for-each-ref'. This ports over branch.c to use ref-filter's
printing options. The previous version of the patch can be found at:
http://article.gmane.org/gmane.comp.version-control.git/278926

Karthik Nayak (10):
  ref-filter: implement %(if), %(then), and %(else) atoms
  ref-filter: implement %(if:equals=<string>) and
    %(if:notequals=<string>)
  ref-filter: add support for %(refname:dir) and %(refname:base)
  ref-filter: modify "%(objectname:short)" to take length
  ref-filter: adopt get_head_description() from branch.c
  ref-filter: introduce format_ref_array_item()
  ref-filter: make %(upstream:track) prints "[gone]" for invalid
    upstreams
  ref-filter: add support for %(upstream:track,nobracket)
  branch: use ref-filter printing APIs
  branch: implement '--format' option

 Documentation/git-branch.txt       |   7 +-
 Documentation/git-for-each-ref.txt |  63 +++++++-
 builtin/branch.c                   | 265 +++++++++-------------------------
 ref-filter.c                       | 285 +++++++++++++++++++++++++++++++++----
 ref-filter.h                       |   5 +
 t/t3203-branch-output.sh           |  11 ++
 t/t6040-tracking-info.sh           |   2 +-
 t/t6300-for-each-ref.sh            |  33 ++++-
 t/t6302-for-each-ref-filter.sh     | 132 +++++++++++++++++
 9 files changed, 573 insertions(+), 230 deletions(-)

Changes in this version:

1. Use %(refname:dir) and %(refname:base) instead of %(path) and
%(path:short).
2. Make it %(objectname:short=<len>) rather than %(objectname:short,<len>).
3. Add %(upstream:track,nobracket) which removes the brackets from tracking
information.
4. calc_maxwidth() now also considers head ref descriptions, this wasn't done
in the last patch and would cause detached head description to be not aligned with the other ref-names.
5. remove the if_atom field in if_then_else struct.
6. introduce better error checking for the %(if), %(then), %(else) atoms
as suggested by Matthieu (when used along with %(align) atom and so on).
7. refactor code and improve documentation.

Interdiff:

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 5097915..a38cbf6 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -92,7 +92,9 @@ refname::
  The name of the ref (the part after $GIT_DIR/).
  For a non-ambiguous short name of the ref append `:short`.
  The option core.warnAmbiguousRefs is used to select the strict
- abbreviation mode.
+ abbreviation mode. For the base directory of the ref (i.e. foo
+ in refs/foo/bar/boz) append `:base`. For the entire directory
+ path append `:dir`.
 
 objecttype::
  The type of the object (`blob`, `tree`, `commit`, `tag`).
@@ -102,9 +104,9 @@ objectsize::
 
 objectname::
  The object name (aka SHA-1).
- For a non-ambiguous abbreviation of the object name append `:short`.
- The length can be explicitly specified by appending either
- `:short,<length>` or `:<length>,short`.  Minimum length is 4.
+ For a non-ambiguous abbreviation of the object name append
+ `:short`.  The length can be explicitly specified by appending
+ `:short=<length>`.  Minimum length being 4.
 
 upstream::
  The name of a local ref which can be considered ``upstream''
@@ -112,8 +114,10 @@ upstream::
  `refname` above.  Additionally respects `:track` to show
  "[ahead N, behind M]" and `:trackshort` to show the terse
  version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
- or "=" (in sync).  Has no effect if the ref does not have
- tracking information associated with it.
+ or "=" (in sync).  Append `:track,nobracket` to show tracking
+ information without brackets (i.e "ahead N, behind M").  Has
+ no effect if the ref does not have tracking information
+ associated with it.
 
 push::
  The name of a local ref which represents the `@{push}` location
@@ -143,14 +147,12 @@ if::
  If there is an atom with value or string literal after the
  %(if) then everything after the %(then) is printed, else if
  the %(else) atom is used, then everything after %(else) is
- printed. Append ":equals=<string>" or ":notequals=<string>" to
- compare the value between the %(if:...) and %(then) atoms with the
- given string.
-
-path::
- The path of the given ref. For a shortened version listing
- only the name of the directory the ref is under append
- `:short`.
+ printed. We ignore space when evaluating the string before
+ %(then), this is useful when we use the %(HEAD) atom which
+ prints either "*" or " " and we want to apply the 'if'
+ condition only on the 'HEAD' ref. Append ":equals=<string>" or
+ ":notequals=<string>" to compare the value between the
+ %(if:...) and %(then) atoms with the given string.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
@@ -191,8 +193,9 @@ 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
+When a scripting language specific quoting is in effect (i.e. one of
+`--shell`, `--perl`, `--python`, `--tcl` is used), 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.
 
@@ -282,6 +285,26 @@ eval=`git for-each-ref --shell --format="$fmt" \
 eval "$eval"
 ------------
 
+
+An example to show the usage of %(if)...%(then)...%(else)...%(end).
+This prefixes the current branch with a star.
+
+------------
+#!/bin/sh
+
+git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  %(end)%(refname:short)" refs/heads/
+------------
+
+
+An example to show the usage of %(if)...%(then)...%(end).
+This adds a red color to authorname, if present
+
+------------
+#!/bin/sh
+
+git for-each-ref --format="%(refname)%(if)%(authorname)%(then)%(color:red)%(end) %(authorname)"
+------------
+
 SEE ALSO
 --------
 linkgit:git-show-ref[1]
diff --git a/builtin/branch.c b/builtin/branch.c
index ae3ecfb..2356e72 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -282,7 +282,10 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
 
  skip_prefix(it->refname, "refs/heads/", &desc);
  skip_prefix(it->refname, "refs/remotes/", &desc);
- w = utf8_strwidth(desc);
+ if (it->kind == FILTER_REFS_DETACHED_HEAD)
+ w = strlen(get_head_description());
+ else
+ w = utf8_strwidth(desc);
 
  if (it->kind == FILTER_REFS_REMOTES)
  w += remote_bonus;
@@ -292,44 +295,41 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
  return max;
 }
 
-static char *get_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)
+static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)
 {
- char *remote = NULL;
- char *local = NULL;
- char *final = NULL;
+ struct strbuf fmt = STRBUF_INIT;
+ struct strbuf local = STRBUF_INIT;
+ struct strbuf remote = STRBUF_INIT;
+
+ strbuf_addf(&fmt, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)", branch_get_color(BRANCH_COLOR_CURRENT));
 
  if (filter->verbose) {
- if (filter->verbose > 1)
- local = xstrfmt("%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)%%(align:%d,left)%%(refname:short)%%(end)%s"
- " %%(objectname:short,7) %%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s] %%(end)"
- "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)",
- branch_get_color(BRANCH_COLOR_CURRENT), maxwidth, branch_get_color(BRANCH_COLOR_RESET),
- branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));
+ strbuf_addf(&local, "%%(align:%d,left)%%(refname:short)%%(end)", maxwidth);
+ strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
+ strbuf_addf(&local, " %%(objectname:short=7) ");
 
+ if (filter->verbose > 1)
+ strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
+    "%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
+    branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));
  else
- local = xstrfmt("%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)%%(align:%d,left)%"
- "%(refname:short)%%(end)%s %%(objectname:short,7) %%(if)%%(upstream:track)%"
- "%(then)%%(upstream:track) %%(end)%%(contents:subject)",
- branch_get_color(BRANCH_COLOR_CURRENT), maxwidth, branch_get_color(BRANCH_COLOR_RESET));
-
- remote = xstrfmt("  %s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)"
- "%%(else) %%(objectname:short,7) %%(contents:subject)%%(end)",
- branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,
- remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
- final = xstrfmt("%%(if:notequals=remotes)%%(path:short)%%(then)%s%%(else)%s%%(end)", local, remote);
+ strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
 
+ strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)"
+    "%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
+    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,
+    remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
  } else {
- local = xstrfmt("%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)%%(refname:short)%s",
- branch_get_color(BRANCH_COLOR_CURRENT), branch_get_color(BRANCH_COLOR_RESET));
- remote = xstrfmt("  %s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
- branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
- final = xstrfmt("%%(if:notequals=remotes)%%(path:short)%%(then)%s%%(else)%s%%(end)", local, remote);
+ strbuf_addf(&local, "%%(refname:short)%s", branch_get_color(BRANCH_COLOR_RESET));
+ strbuf_addf(&remote, "%s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
+    branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
  }
 
- free(local);
- free(remote);
+ strbuf_addf(&fmt, "%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)", local.buf, remote.buf);
 
- return final;
+ strbuf_release(&local);
+ strbuf_release(&remote);
+ return strbuf_detach(&fmt, NULL);
 }
 
 static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
@@ -357,7 +357,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
  maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
  if (!format)
- format = to_free = get_format(filter, maxwidth, remote_prefix);
+ format = to_free = build_format(filter, maxwidth, remote_prefix);
  verify_ref_format(format);
 
  /*
@@ -375,6 +375,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
  format_ref_array_item(array.items[i], format, 0, &out);
  if (column_active(colopts)) {
  assert(!filter->verbose && "--column and --verbose are incompatible");
+ /* format to a string_list to let print_columns() do its job */
  string_list_append(&output, out.buf);
  } else {
  fwrite(out.buf, 1, out.len, stdout);
diff --git a/ref-filter.c b/ref-filter.c
index 48b06e3..6044eb0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -59,7 +59,6 @@ static struct {
  { "if" },
  { "then" },
  { "else" },
- { "path" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -77,8 +76,7 @@ struct contents {
 struct if_then_else {
  const char *if_equals,
  *not_equals;
- unsigned int if_atom : 1,
- then_atom : 1,
+ unsigned int then_atom : 1,
  else_atom : 1,
  condition_satisfied : 1;
 };
@@ -257,22 +255,27 @@ static void if_then_else_handler(struct ref_formatting_stack **stack)
  struct ref_formatting_stack *prev = cur->prev;
  struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
 
- /*
- * If the 'if' condition is satisfied, then if there exists an
- * 'else' atom drop the 'else' atom's state. Else we swap the
- * buffer of the 'else' atom with the previous state and drop
- * that state. If the condition is satisfied and no 'else' atom
- * exists, then just clear the buffer.
- */
- if (if_then_else->condition_satisfied && if_then_else->else_atom) {
- strbuf_reset(&cur->output);
- pop_stack_element(&cur);
- } else if (if_then_else->else_atom) {
- strbuf_swap(&cur->output, &prev->output);
- strbuf_reset(&cur->output);
- pop_stack_element(&cur);
+ if (if_then_else->else_atom) {
+ /*
+ * There is an %(else) atom: we need to drop one state from the
+ * stack, either the %(else) branch if the condition is satisfied, or
+ * the %(then) branch if it isn't.
+ */
+ if (if_then_else->condition_satisfied) {
+ strbuf_reset(&cur->output);
+ pop_stack_element(&cur);
+ } else {
+ strbuf_swap(&cur->output, &prev->output);
+ strbuf_reset(&cur->output);
+ pop_stack_element(&cur);
+ }
  } else if (!if_then_else->condition_satisfied)
+ /*
+ * No %(else) atom: just drop the %(then) branch if the
+ * condition is not satisfied.
+ */
  strbuf_reset(&cur->output);
+
  *stack = cur;
  free(if_then_else);
 }
@@ -283,7 +286,6 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat
  struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);
  const char *valp;
 
- if_then_else->if_atom = 1;
  if (skip_prefix(atomv->s, "equals=", &valp))
  if_then_else->if_equals = valp;
  else if (skip_prefix(atomv->s, "notequals=", &valp))
@@ -297,7 +299,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat
  new->at_end_data = if_then_else;
 }
 
-static int is_empty(const char * s){
+static int is_empty(const char *s){
  while (*s != '\0') {
  if (!isspace(*s))
  return 0;
@@ -309,10 +311,14 @@ static int is_empty(const char * s){
 static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
 {
  struct ref_formatting_stack *cur = state->stack;
- struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
+ struct if_then_else *if_then_else = NULL;
 
+ if (cur->at_end == if_then_else_handler)
+ if_then_else = (struct if_then_else *)cur->at_end_data;
  if (!if_then_else)
  die(_("format: %%(then) atom used without an %%(if) atom"));
+ if (if_then_else->then_atom)
+ die(_("format: %%(then) atom used more than once"));
  if_then_else->then_atom = 1;
 
  /*
@@ -334,12 +340,16 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st
 static void else_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
 {
  struct ref_formatting_stack *prev = state->stack;
- struct if_then_else *if_then_else = (struct if_then_else *)state->stack->at_end_data;
+ struct if_then_else *if_then_else = NULL;
 
+ if (prev->at_end == if_then_else_handler)
+ if_then_else = (struct if_then_else *)prev->at_end_data;
  if (!if_then_else)
  die(_("format: %%(else) atom used without an %%(if) atom"));
  if (!if_then_else->then_atom)
  die(_("format: %%(else) atom used without a %%(then) atom"));
+ if (if_then_else->else_atom)
+ die(_("format: %%(else) atom used more than once"));
  if_then_else->else_atom = 1;
  push_stack_element(&state->stack);
  state->stack->at_end_data = prev->at_end_data;
@@ -355,7 +365,7 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta
  die(_("format: %%(end) atom used without corresponding atom"));
  current->at_end(&state->stack);
 
- /*  Stack may have been popped, hence reset the current pointer */
+ /*  Stack may have been popped within at_end(), hence reset the current pointer */
  current = state->stack;
 
  /*
@@ -458,33 +468,24 @@ static int grab_objectname(const char *name, const unsigned char *sha1,
  const char *p;
 
  if (match_atom_name(name, "objectname", &p)) {
- struct strbuf **s, **to_free;
- int length = DEFAULT_ABBREV;
-
  /*  No arguments given, copy the entire objectname */
  if (!p) {
  char *s = xmalloc(41);
  strcpy(s, sha1_to_hex(sha1));
  v->s = s;
  } else {
- s = to_free = strbuf_split_str(p, ',', 0);
- while (*s) {
- /*  Strip trailing comma */
- if (s[1])
- strbuf_setlen(s[0], s[0]->len - 1);
- if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)&length))
- ;
- /*  The `short` argument uses the default length */
- else if (!strcmp("short", s[0]->buf))
- ;
- else
- die(_("format: unknown option entered with objectname:%s"), s[0]->buf);
- s++;
- }
+ unsigned int length = DEFAULT_ABBREV;
+
+ if (skip_prefix(p, "short", &p)) {
+ if (p[0] == '=')
+ if (strtoul_ui(p + 1, 10, &length))
+ die(_("positive length expected with objectname:%s"), p + 1);
+ } else
+ die(_("format: unknown option entered with objectname:%s"), p);
+
  if (length < MINIMUM_ABBREV)
  length = MINIMUM_ABBREV;
  v->s = xstrdup(find_unique_abbrev(sha1, length));
- free(to_free);
  }
  return 1;
  }
@@ -916,7 +917,7 @@ static inline char *copy_advance(char *dst, const char *src)
  return dst;
 }
 
-static char *get_head_description(void)
+char *get_head_description(void)
 {
  struct strbuf desc = STRBUF_INIT;
  struct wt_status_state state;
@@ -1100,22 +1101,6 @@ static void populate_value(struct ref_array_item *ref)
  } else if (!strcmp(name, "else")) {
  v->handler = else_atom_handler;
  continue;
- } else if (match_atom_name(name, "path", &valp)) {
- const char *sp, *ep;
-
- if (ref->kind & FILTER_REFS_DETACHED_HEAD)
- continue;
-
- sp = strchr(ref->refname, '/');
- ep = strchr(++sp, '/');
-
- if (!valp)
- sp = ref->refname;
- else if (strcmp(valp, "short"))
- die(_("format: invalid value given path:%s"), valp);
-
- v->s = xstrndup(sp, ep - sp);
- continue;
  } else
  continue;
 
@@ -1127,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)
  if (!strcmp(formatp, "short"))
  refname = shorten_unambiguous_ref(refname,
       warn_ambiguous_refs);
- else if (!strcmp(formatp, "track") &&
+ else if (skip_prefix(formatp, "track", &valp) &&
+ strcmp(formatp, "trackshort") &&
  (starts_with(name, "upstream") ||
   starts_with(name, "push"))) {
  char buf[40];
+ unsigned int nobracket = 0;
+
+ if (!strcmp(valp, ",nobracket"))
+ nobracket = 1;
 
  if (stat_tracking_info(branch, &num_ours,
        &num_theirs, NULL)) {
- v->s = xstrdup("[gone]");
+ if (nobracket)
+ v->s = "gone";
+ else
+ v->s = "[gone]";
  continue;
  }
 
  if (!num_ours && !num_theirs)
  v->s = "";
  else if (!num_ours) {
- sprintf(buf, "[behind %d]", num_theirs);
+ if (nobracket)
+ sprintf(buf, "behind %d", num_theirs);
+ else
+ sprintf(buf, "[behind %d]", num_theirs);
  v->s = xstrdup(buf);
  } else if (!num_theirs) {
- sprintf(buf, "[ahead %d]", num_ours);
+ if (nobracket)
+ sprintf(buf, "ahead %d", num_ours);
+ else
+ sprintf(buf, "[ahead %d]", num_ours);
  v->s = xstrdup(buf);
  } else {
- sprintf(buf, "[ahead %d, behind %d]",
+ if (nobracket)
+ sprintf(buf, "ahead %d, behind %d",
+ num_ours, num_theirs);
+ else
+ sprintf(buf, "[ahead %d, behind %d]",
  num_ours, num_theirs);
  v->s = xstrdup(buf);
  }
@@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)
  else
  v->s = "<>";
  continue;
+ } else if (!strcmp(formatp, "dir") &&
+   (starts_with(name, "refname"))) {
+ const char *sp, *ep, *tmp;
+
+ sp = tmp = ref->refname;
+ /*  Obtain refs/foo/bar/ from refname refs/foo/bar/abc */
+ do {
+ ep = tmp;
+ tmp = strchr(ep + 1, '/');
+ } while (tmp);
+ v->s = xstrndup(sp, ep - sp);
+ continue;
+ } else if (!strcmp(formatp, "base") &&
+   (starts_with(name, "refname"))) {
+ const char *sp, *ep;
+
+ if (skip_prefix(ref->refname, "refs/", &sp)) {
+ ep = strchr(sp, '/');
+ if (!ep)
+ continue;
+ v->s = xstrndup(sp, ep - sp);
+ }
+ continue;
  } else
  die("unknown %.*s format %s",
     (int)(formatp - name), name, formatp);
diff --git a/ref-filter.h b/ref-filter.h
index e0b26e8..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,5 +109,7 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);
 struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
+/*  Get the current HEAD's description */
+char *get_head_description(void);
 
 #endif /*  REF_FILTER_H  */
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index d110f26..97a0765 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -58,11 +58,11 @@ test_expect_success 'branch -v' '
 '
 
 cat >expect <<\EOF
-b1 [origin/master] [ahead 1, behind 1] d
-b2 [origin/master] [ahead 1, behind 1] d
-b3 [origin/master] [behind 1] b
-b4 [origin/master] [ahead 2] f
-b5 [brokenbase] [gone] g
+b1 [origin/master: ahead 1, behind 1] d
+b2 [origin/master: ahead 1, behind 1] d
+b3 [origin/master: behind 1] b
+b4 [origin/master: ahead 2] f
+b5 [brokenbase: gone] g
 b6 [origin/master] c
 EOF
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2a01645..7ab8bf8 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '
 '
 
 cat >expected <<EOF
+ahead 1
+EOF
+
+test_expect_success 'Check upstream:track,nobracket format' '
+ git for-each-ref --format="%(upstream:track,nobracket)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<EOF
 >
 EOF
 
@@ -390,7 +399,7 @@ $(git rev-parse --short=1 HEAD)
 EOF
 
 test_expect_success 'Check objectname:short format when size is less than MINIMUM_ABBREV' '
- git for-each-ref --format="%(objectname:short,1)" refs/heads >actual &&
+ git for-each-ref --format="%(objectname:short=1)" refs/heads >actual &&
  test_cmp expected actual
 '
 
@@ -399,12 +408,12 @@ $(git rev-parse --short=10 HEAD)
 EOF
 
 test_expect_success 'Check objectname:short format' '
- git for-each-ref --format="%(objectname:short,10)" refs/heads >actual &&
+ git for-each-ref --format="%(objectname:short=10)" refs/heads >actual &&
  test_cmp expected actual
 '
 
 test_expect_success 'Check objectname:short format for invalid input' '
- test_must_fail git for-each-ref --format="%(objectname:short,-1)" refs/heads
+ test_must_fail git for-each-ref --format="%(objectname:short=-1)" refs/heads
 '
 
 test_expect_success 'Check for invalid refname format' '
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 5557657..19a5075 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -303,6 +303,25 @@ test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '
  test_cmp expect actual
 '
 
+test_expect_success 'ignore spaces in %(if) atom usage' '
+ git for-each-ref --format="%(refname:short): %(if)%(HEAD)%(then)Head ref%(else)Not Head ref%(end)" >actual &&
+ cat >expect <<-\EOF &&
+ master: Head ref
+ side: Not Head ref
+ odd/spot: Not Head ref
+ double-tag: Not Head ref
+ foo1.10: Not Head ref
+ foo1.3: Not Head ref
+ foo1.6: Not Head ref
+ four: Not Head ref
+ one: Not Head ref
+ signed-tag: Not Head ref
+ three: Not Head ref
+ two: Not Head ref
+ EOF
+ test_cmp expect actual
+'
+
 test_expect_success 'check %(if:equals=<string>)' '
  git for-each-ref --format="%(if:equals=master)%(refname:short)%(then)Found master%(else)Not master%(end)" refs/heads/ >actual &&
  cat >expect <<-\EOF &&
@@ -312,7 +331,6 @@ test_expect_success 'check %(if:equals=<string>)' '
  test_cmp expect actual
 '
 
-
 test_expect_success 'check %(if:notequals=<string>)' '
  git for-each-ref --format="%(if:notequals=master)%(refname:short)%(then)Not master%(else)Found master%(end)" refs/heads/ >actual &&
  cat >expect <<-\EOF &&
@@ -322,9 +340,16 @@ test_expect_success 'check %(if:notequals=<string>)' '
  test_cmp expect actual
 '
 
-test_expect_success 'check %(path)' '
- git for-each-ref --format="%(path)" >actual &&
+test_expect_success 'update refs for %(refname:dir) and %(refname:base)' '
+ git update-ref refs/foo HEAD &&
+ git update-ref refs/foodir/bar/boz HEAD
+'
+
+test_expect_success 'check %(refname:dir)' '
+ git for-each-ref --format="%(refname:dir)" >actual &&
  cat >expect <<-\EOF &&
+ refs
+ refs/foodir/bar
  refs/heads
  refs/heads
  refs/odd
@@ -341,9 +366,11 @@ test_expect_success 'check %(path)' '
  test_cmp expect actual
 '
 
-test_expect_success 'check %(path:short)' '
- git for-each-ref --format="%(path:short)" >actual &&
+test_expect_success 'check %(refname:base)' '
+ git for-each-ref --format="%(refname:base)" >actual &&
  cat >expect <<-\EOF &&
+
+ foodir
  heads
  heads
  odd

--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms

Karthik Nayak
Implement %(if), %(then) and %(else) atoms. Used as
%(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the
format string between %(if) and %(then) expands to an empty string, or
to only whitespaces, then the whole %(if)...%(end) expands to the
string following %(then). Otherwise, it expands to the string
following %(else), if any. Nesting of this construct is possible.

This is in preperation for porting over `git branch -l` to use
ref-filter APIs for printing.

Add Documentation and tests regarding the same.

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 Documentation/git-for-each-ref.txt |  47 +++++++++++++-
 ref-filter.c                       | 127 +++++++++++++++++++++++++++++++++++--
 t/t6302-for-each-ref-filter.sh     |  67 +++++++++++++++++++
 3 files changed, 231 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 16b4ac5..7345acb 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -134,9 +134,17 @@ align::
  `<position>` is either left, right or middle, default being
  left 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.
+ no alignment is performed.
+
+if::
+ Used as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).
+ If there is an atom with value or string literal after the
+ %(if) then everything after the %(then) is printed, else if
+ the %(else) atom is used, then everything after %(else) is
+ printed. We ignore space when evaluating the string before
+ %(then), this is useful when we use the %(HEAD) atom which
+ prints either "*" or " " and we want to apply the 'if'
+ condition only on the 'HEAD' ref.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
@@ -169,6 +177,19 @@ the date by adding one of `:default`, `:relative`, `:short`, `:local`,
 `:iso8601`, `:rfc2822` or `:raw` to the end of the fieldname; e.g.
 `%(taggerdate:relative)`.
 
+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 (i.e. one of
+`--shell`, `--perl`, `--python`, `--tcl` is used), 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.
 
 EXAMPLES
 --------
@@ -256,6 +277,26 @@ eval=`git for-each-ref --shell --format="$fmt" \
 eval "$eval"
 ------------
 
+
+An example to show the usage of %(if)...%(then)...%(else)...%(end).
+This prefixes the current branch with a star.
+
+------------
+#!/bin/sh
+
+git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  %(end)%(refname:short)" refs/heads/
+------------
+
+
+An example to show the usage of %(if)...%(then)...%(end).
+This adds a red color to authorname, if present
+
+------------
+#!/bin/sh
+
+git for-each-ref --format="%(refname)%(if)%(authorname)%(then)%(color:red)%(end) %(authorname)"
+------------
+
 SEE ALSO
 --------
 linkgit:git-show-ref[1]
diff --git a/ref-filter.c b/ref-filter.c
index dbd8fce..95f007c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,6 +55,9 @@ static struct {
  { "color" },
  { "align" },
  { "end" },
+ { "if" },
+ { "then" },
+ { "else" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -69,10 +72,16 @@ struct contents {
  struct object_id oid;
 };
 
+struct if_then_else {
+ unsigned int then_atom : 1,
+ else_atom : 1,
+ condition_satisfied : 1;
+};
+
 struct ref_formatting_stack {
  struct ref_formatting_stack *prev;
  struct strbuf output;
- void (*at_end)(struct ref_formatting_stack *stack);
+ void (*at_end)(struct ref_formatting_stack **stack);
  void *at_end_data;
 };
 
@@ -216,13 +225,14 @@ static void pop_stack_element(struct ref_formatting_stack **stack)
  *stack = prev;
 }
 
-static void end_align_handler(struct ref_formatting_stack *stack)
+static void end_align_handler(struct ref_formatting_stack **stack)
 {
- struct align *align = (struct align *)stack->at_end_data;
+ struct ref_formatting_stack *cur = *stack;
+ struct align *align = (struct align *)cur->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_utf8_align(&s, align->position, align->width, cur->output.buf);
+ strbuf_swap(&cur->output, &s);
  strbuf_release(&s);
 }
 
@@ -236,6 +246,97 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s
  new->at_end_data = &atomv->u.align;
 }
 
+static void if_then_else_handler(struct ref_formatting_stack **stack)
+{
+ struct ref_formatting_stack *cur = *stack;
+ struct ref_formatting_stack *prev = cur->prev;
+ struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
+
+ if (if_then_else->else_atom) {
+ /*
+ * There is an %(else) atom: we need to drop one state from the
+ * stack, either the %(else) branch if the condition is satisfied, or
+ * the %(then) branch if it isn't.
+ */
+ if (if_then_else->condition_satisfied) {
+ strbuf_reset(&cur->output);
+ pop_stack_element(&cur);
+ } else {
+ strbuf_swap(&cur->output, &prev->output);
+ strbuf_reset(&cur->output);
+ pop_stack_element(&cur);
+ }
+ } else if (!if_then_else->condition_satisfied)
+ /*
+ * No %(else) atom: just drop the %(then) branch if the
+ * condition is not satisfied.
+ */
+ strbuf_reset(&cur->output);
+
+ *stack = cur;
+ free(if_then_else);
+}
+
+static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
+{
+ struct ref_formatting_stack *new;
+ struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);
+
+ push_stack_element(&state->stack);
+ new = state->stack;
+ new->at_end = if_then_else_handler;
+ new->at_end_data = if_then_else;
+}
+
+static int is_empty(const char *s){
+ while (*s != '\0') {
+ if (!isspace(*s))
+ return 0;
+ s++;
+ }
+ return 1;
+}
+
+static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
+{
+ struct ref_formatting_stack *cur = state->stack;
+ struct if_then_else *if_then_else = NULL;
+
+ if (cur->at_end == if_then_else_handler)
+ if_then_else = (struct if_then_else *)cur->at_end_data;
+ if (!if_then_else)
+ die(_("format: %%(then) atom used without an %%(if) atom"));
+ if (if_then_else->then_atom)
+ die(_("format: %%(then) atom used more than once"));
+ if_then_else->then_atom = 1;
+ /*
+ * If there exists non-empty string between the 'if' and
+ * 'then' atom then the 'if' condition is satisfied.
+ */
+ if (cur->output.len && !is_empty(cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ strbuf_reset(&cur->output);
+}
+
+static void else_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
+{
+ struct ref_formatting_stack *prev = state->stack;
+ struct if_then_else *if_then_else = NULL;
+
+ if (prev->at_end == if_then_else_handler)
+ if_then_else = (struct if_then_else *)prev->at_end_data;
+ if (!if_then_else)
+ die(_("format: %%(else) atom used without an %%(if) atom"));
+ if (!if_then_else->then_atom)
+ die(_("format: %%(else) atom used without a %%(then) atom"));
+ if (if_then_else->else_atom)
+ die(_("format: %%(else) atom used more than once"));
+ if_then_else->else_atom = 1;
+ push_stack_element(&state->stack);
+ state->stack->at_end_data = prev->at_end_data;
+ state->stack->at_end = prev->at_end;
+}
+
 static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
 {
  struct ref_formatting_stack *current = state->stack;
@@ -243,14 +344,17 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta
 
  if (!current->at_end)
  die(_("format: %%(end) atom used without corresponding atom"));
- current->at_end(current);
+ current->at_end(&state->stack);
+
+ /*  Stack may have been popped within at_end(), hence reset the current pointer */
+ current = state->stack;
 
  /*
  * 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) {
+ if (!current->prev->prev) {
  quote_formatting(&s, current->output.buf, state->quote_style);
  strbuf_swap(&current->output, &s);
  }
@@ -920,6 +1024,15 @@ static void populate_value(struct ref_array_item *ref)
  } else if (!strcmp(name, "end")) {
  v->handler = end_atom_handler;
  continue;
+ } else if (!strcmp(name, "if")) {
+ v->handler = if_atom_handler;
+ continue;
+ } else if (!strcmp(name, "then")) {
+ v->handler = then_atom_handler;
+ continue;
+ } else if (!strcmp(name, "else")) {
+ v->handler = else_atom_handler;
+ continue;
  } else
  continue;
 
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index fe4796c..617fa1f 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -255,4 +255,71 @@ test_expect_success 'reverse version sort' '
  test_cmp expect actual
 '
 
+test_expect_success 'improper usage of %(if), %(then), %(else) and %(end) atoms' '
+ test_must_fail git for-each-ref --format="%(if)" &&
+ test_must_fail git for-each-ref --format="%(then)" &&
+ test_must_fail git for-each-ref --format="%(else)" &&
+ test_must_fail git for-each-ref --format="%(if) %(else)" &&
+ test_must_fail git for-each-ref --format="%(then) %(else)" &&
+ test_must_fail git for-each-ref --format="%(if) %(else)" &&
+ test_must_fail git for-each-ref --format="%(if) %(then) %(else)"
+'
+
+test_expect_success 'check %(if)...%(then)...%(end) atoms' '
+ git for-each-ref --format="%(if)%(authorname)%(then)%(authorname): %(refname)%(end)" >actual &&
+ cat >expect <<-\EOF &&
+ A U Thor: refs/heads/master
+ A U Thor: refs/heads/side
+ A U Thor: refs/odd/spot
+
+ A U Thor: refs/tags/foo1.10
+ A U Thor: refs/tags/foo1.3
+ A U Thor: refs/tags/foo1.6
+ A U Thor: refs/tags/four
+ A U Thor: refs/tags/one
+
+ A U Thor: refs/tags/three
+ A U Thor: refs/tags/two
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '
+ git for-each-ref --format="%(if)%(authorname)%(then)%(authorname)%(else)No author%(end): %(refname)" >actual &&
+ cat >expect <<-\EOF &&
+ A U Thor: refs/heads/master
+ A U Thor: refs/heads/side
+ A U Thor: refs/odd/spot
+ No author: refs/tags/double-tag
+ A U Thor: refs/tags/foo1.10
+ A U Thor: refs/tags/foo1.3
+ A U Thor: refs/tags/foo1.6
+ A U Thor: refs/tags/four
+ A U Thor: refs/tags/one
+ No author: refs/tags/signed-tag
+ A U Thor: refs/tags/three
+ A U Thor: refs/tags/two
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'ignore spaces in %(if) atom usage' '
+ git for-each-ref --format="%(refname:short): %(if)%(HEAD)%(then)Head ref%(else)Not Head ref%(end)" >actual &&
+ cat >expect <<-\EOF &&
+ master: Head ref
+ side: Not Head ref
+ odd/spot: Not Head ref
+ double-tag: Not Head ref
+ foo1.10: Not Head ref
+ foo1.3: Not Head ref
+ foo1.6: Not Head ref
+ four: Not Head ref
+ one: Not Head ref
+ signed-tag: Not Head ref
+ three: Not Head ref
+ two: Not Head ref
+ EOF
+ test_cmp expect actual
+'
+
 test_done
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 02/10] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)

Karthik Nayak
In reply to this post by Karthik Nayak
Implement %(if:equals=<string>) wherein the if condition is only
satisfied if the value obtained between the %(if:...) and %(then) atom
is the same as the given '<string>'.

Similarly, implement (if:notequals=<string>) wherein the if condition
is only satisfied if the value obtained between the %(if:...) and
%(then) atom is differnt from the given '<string>'.

Add tests and Documentation for the same.

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 Documentation/git-for-each-ref.txt |  4 +++-
 ref-filter.c                       | 28 ++++++++++++++++++++++++----
 t/t6302-for-each-ref-filter.sh     | 18 ++++++++++++++++++
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 7345acb..206d646 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -144,7 +144,9 @@ if::
  printed. We ignore space when evaluating the string before
  %(then), this is useful when we use the %(HEAD) atom which
  prints either "*" or " " and we want to apply the 'if'
- condition only on the 'HEAD' ref.
+ condition only on the 'HEAD' ref. Append ":equals=<string>" or
+ ":notequals=<string>" to compare the value between the
+ %(if:...) and %(then) atoms with the given string.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 95f007c..359c76d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -73,6 +73,8 @@ struct contents {
 };
 
 struct if_then_else {
+ const char *if_equals,
+ *not_equals;
  unsigned int then_atom : 1,
  else_atom : 1,
  condition_satisfied : 1;
@@ -281,6 +283,14 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat
 {
  struct ref_formatting_stack *new;
  struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);
+ const char *valp;
+
+ if (skip_prefix(atomv->s, "equals=", &valp))
+ if_then_else->if_equals = valp;
+ else if (skip_prefix(atomv->s, "notequals=", &valp))
+ if_then_else->not_equals = valp;
+ else if (atomv->s[0])
+ die(_("format: unknown format if:%s"), atomv->s);
 
  push_stack_element(&state->stack);
  new = state->stack;
@@ -309,11 +319,19 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st
  if (if_then_else->then_atom)
  die(_("format: %%(then) atom used more than once"));
  if_then_else->then_atom = 1;
+
  /*
- * If there exists non-empty string between the 'if' and
- * 'then' atom then the 'if' condition is satisfied.
+ * If the 'equals' or 'notequals' attribute is used then
+ * perform the required comparison. If not, only non-empty
+ * strings satisfy the 'if' condition.
  */
- if (cur->output.len && !is_empty(cur->output.buf))
+ if (if_then_else->if_equals) {
+ if (!strcmp(if_then_else->if_equals, cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ } else if (if_then_else->not_equals) {
+ if (strcmp(if_then_else->not_equals, cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ } else if (cur->output.len && !is_empty(cur->output.buf))
  if_then_else->condition_satisfied = 1;
  strbuf_reset(&cur->output);
 }
@@ -1024,8 +1042,10 @@ static void populate_value(struct ref_array_item *ref)
  } else if (!strcmp(name, "end")) {
  v->handler = end_atom_handler;
  continue;
- } else if (!strcmp(name, "if")) {
+ } else if (match_atom_name(name, "if", &valp)) {
  v->handler = if_atom_handler;
+ if (valp)
+ v->s = xstrdup(valp);
  continue;
  } else if (!strcmp(name, "then")) {
  v->handler = then_atom_handler;
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 617fa1f..f45ac1f 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -322,4 +322,22 @@ test_expect_success 'ignore spaces in %(if) atom usage' '
  test_cmp expect actual
 '
 
+test_expect_success 'check %(if:equals=<string>)' '
+ git for-each-ref --format="%(if:equals=master)%(refname:short)%(then)Found master%(else)Not master%(end)" refs/heads/ >actual &&
+ cat >expect <<-\EOF &&
+ Found master
+ Not master
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'check %(if:notequals=<string>)' '
+ git for-each-ref --format="%(if:notequals=master)%(refname:short)%(then)Not master%(else)Found master%(end)" refs/heads/ >actual &&
+ cat >expect <<-\EOF &&
+ Found master
+ Not master
+ EOF
+ test_cmp expect actual
+'
+
 test_done
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 03/10] ref-filter: add support for %(refname:dir) and %(refname:base)

Karthik Nayak
In reply to this post by Karthik Nayak
Add the options `:dir` and `:base` to the %(refname) atom. The `:dir`
option gives the directory (the part after $GIT_DIR/) of the ref
without the refname. The `:base` option gives the base directory of
the given ref (i.e. the directory following $GIT_DIR/refs/).

Add tests and documentation for the same.
---
 Documentation/git-for-each-ref.txt |  4 +++-
 ref-filter.c                       | 23 +++++++++++++++++++
 t/t6302-for-each-ref-filter.sh     | 47 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 206d646..b8d33a1 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -92,7 +92,9 @@ refname::
  The name of the ref (the part after $GIT_DIR/).
  For a non-ambiguous short name of the ref append `:short`.
  The option core.warnAmbiguousRefs is used to select the strict
- abbreviation mode.
+ abbreviation mode. For the base directory of the ref (i.e. foo
+ in refs/foo/bar/boz) append `:base`. For the entire directory
+ path append `:dir`.
 
 objecttype::
  The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index 359c76d..94d8754 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1105,6 +1105,29 @@ static void populate_value(struct ref_array_item *ref)
  else
  v->s = "<>";
  continue;
+ } else if (!strcmp(formatp, "dir") &&
+   (starts_with(name, "refname"))) {
+ const char *sp, *ep, *tmp;
+
+ sp = tmp = ref->refname;
+ /*  Obtain refs/foo/bar/ from refname refs/foo/bar/abc */
+ do {
+ ep = tmp;
+ tmp = strchr(ep + 1, '/');
+ } while (tmp);
+ v->s = xstrndup(sp, ep - sp);
+ continue;
+ } else if (!strcmp(formatp, "base") &&
+   (starts_with(name, "refname"))) {
+ const char *sp, *ep;
+
+ if (skip_prefix(ref->refname, "refs/", &sp)) {
+ ep = strchr(sp, '/');
+ if (!ep)
+ continue;
+ v->s = xstrndup(sp, ep - sp);
+ }
+ continue;
  } else
  die("unknown %.*s format %s",
     (int)(formatp - name), name, formatp);
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index f45ac1f..19a5075 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -340,4 +340,51 @@ test_expect_success 'check %(if:notequals=<string>)' '
  test_cmp expect actual
 '
 
+test_expect_success 'update refs for %(refname:dir) and %(refname:base)' '
+ git update-ref refs/foo HEAD &&
+ git update-ref refs/foodir/bar/boz HEAD
+'
+
+test_expect_success 'check %(refname:dir)' '
+ git for-each-ref --format="%(refname:dir)" >actual &&
+ cat >expect <<-\EOF &&
+ refs
+ refs/foodir/bar
+ refs/heads
+ refs/heads
+ refs/odd
+ refs/tags
+ refs/tags
+ refs/tags
+ refs/tags
+ refs/tags
+ refs/tags
+ refs/tags
+ refs/tags
+ refs/tags
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'check %(refname:base)' '
+ git for-each-ref --format="%(refname:base)" >actual &&
+ cat >expect <<-\EOF &&
+
+ foodir
+ heads
+ heads
+ odd
+ tags
+ tags
+ tags
+ tags
+ tags
+ tags
+ tags
+ tags
+ tags
+ EOF
+ test_cmp expect actual
+'
+
 test_done
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 04/10] ref-filter: modify "%(objectname:short)" to take length

Karthik Nayak
In reply to this post by Karthik Nayak
Add support for %(objectname:short=<length>) which would print the
abbreviated unique objectname of given length. When no length is
specified 7 is used. The minimum length is 4.

Add tests and documentation for the same.

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 Documentation/git-for-each-ref.txt |  4 +++-
 ref-filter.c                       | 30 ++++++++++++++++++++++--------
 t/t6300-for-each-ref.sh            | 22 ++++++++++++++++++++++
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index b8d33a1..c713ec0 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -104,7 +104,9 @@ objectsize::
 
 objectname::
  The object name (aka SHA-1).
- For a non-ambiguous abbreviation of the object name append `:short`.
+ For a non-ambiguous abbreviation of the object name append
+ `:short`.  The length can be explicitly specified by appending
+ `:short=<length>`.  Minimum length being 4.
 
 upstream::
  The name of a local ref which can be considered ``upstream''
diff --git a/ref-filter.c b/ref-filter.c
index 94d8754..de4d451 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -464,14 +464,28 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo
 static int grab_objectname(const char *name, const unsigned char *sha1,
     struct atom_value *v)
 {
- if (!strcmp(name, "objectname")) {
- char *s = xmalloc(41);
- strcpy(s, sha1_to_hex(sha1));
- v->s = s;
- return 1;
- }
- if (!strcmp(name, "objectname:short")) {
- v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
+ const char *p;
+
+ if (match_atom_name(name, "objectname", &p)) {
+ /*  No arguments given, copy the entire objectname */
+ if (!p) {
+ char *s = xmalloc(41);
+ strcpy(s, sha1_to_hex(sha1));
+ v->s = s;
+ } else {
+ unsigned int length = DEFAULT_ABBREV;
+
+ if (skip_prefix(p, "short", &p)) {
+ if (p[0] == '=')
+ if (strtoul_ui(p + 1, 10, &length))
+ die(_("positive length expected with objectname:%s"), p + 1);
+ } else
+ die(_("format: unknown option entered with objectname:%s"), p);
+
+ if (length < MINIMUM_ABBREV)
+ length = MINIMUM_ABBREV;
+ v->s = xstrdup(find_unique_abbrev(sha1, length));
+ }
  return 1;
  }
  return 0;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7c9bec7..6fc569e 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '
  test_cmp expected actual
 '
 
+cat >expected <<EOF
+$(git rev-parse --short=1 HEAD)
+EOF
+
+test_expect_success 'Check objectname:short format when size is less than MINIMUM_ABBREV' '
+ git for-each-ref --format="%(objectname:short=1)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<EOF
+$(git rev-parse --short=10 HEAD)
+EOF
+
+test_expect_success 'Check objectname:short format' '
+ git for-each-ref --format="%(objectname:short=10)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'Check objectname:short format for invalid input' '
+ test_must_fail git for-each-ref --format="%(objectname:short=-1)" refs/heads
+'
+
 test_expect_success 'Check for invalid refname format' '
  test_must_fail git for-each-ref --format="%(refname:INVALID)"
 '
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 05/10] ref-filter: adopt get_head_description() from branch.c

Karthik Nayak
In reply to this post by Karthik Nayak
Copy the implementation of get_head_description() from branch.c.  This
gives a description of the HEAD ref if called. This is used as the
refname for the HEAD ref whenever the FILTER_REFS_DETACHED_HEAD option
is used. Make it public because we need it to calculate the length of
the HEAD refs description in branch.c:calc_maxwidth() when we port
branch.c to use ref-filter APIs.

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 builtin/branch.c |  4 ++++
 ref-filter.c     | 38 ++++++++++++++++++++++++++++++++++++--
 ref-filter.h     |  2 ++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index b7a60f4..67ef9f1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -355,6 +355,10 @@ static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
  strbuf_release(&subject);
 }
 
+/*
+ * This is duplicated in ref-filter.c, will be removed when we adopt
+ * ref-filter's printing APIs.
+ */
 static char *get_head_description(void)
 {
  struct strbuf desc = STRBUF_INIT;
diff --git a/ref-filter.c b/ref-filter.c
index de4d451..a3cc3af 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
 #include "utf8.h"
 #include "git-compat-util.h"
 #include "version.h"
+#include "wt-status.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -916,6 +917,37 @@ static inline char *copy_advance(char *dst, const char *src)
  return dst;
 }
 
+char *get_head_description(void)
+{
+ struct strbuf desc = STRBUF_INIT;
+ struct wt_status_state state;
+ memset(&state, 0, sizeof(state));
+ wt_status_get_state(&state, 1);
+ if (state.rebase_in_progress ||
+    state.rebase_interactive_in_progress)
+ strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+    state.branch);
+ else if (state.bisect_in_progress)
+ strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
+    state.branch);
+ else if (state.detached_from) {
+ /* TRANSLATORS: make sure these match _("HEAD detached at ")
+   and _("HEAD detached from ") in wt-status.c */
+ if (state.detached_at)
+ strbuf_addf(&desc, _("(HEAD detached at %s)"),
+ state.detached_from);
+ else
+ strbuf_addf(&desc, _("(HEAD detached from %s)"),
+ state.detached_from);
+ }
+ else
+ strbuf_addstr(&desc, _("(no branch)"));
+ free(state.branch);
+ free(state.onto);
+ free(state.detached_from);
+ return strbuf_detach(&desc, NULL);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -954,9 +986,11 @@ static void populate_value(struct ref_array_item *ref)
  name++;
  }
 
- if (starts_with(name, "refname"))
+ if (starts_with(name, "refname")) {
  refname = ref->refname;
- else if (starts_with(name, "symref"))
+ if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+ refname = get_head_description();
+ } else if (starts_with(name, "symref"))
  refname = ref->symref ? ref->symref : "";
  else if (starts_with(name, "upstream")) {
  const char *branch_name;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..4aea594 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -106,5 +106,7 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);
 struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
+/*  Get the current HEAD's description */
+char *get_head_description(void);
 
 #endif /*  REF_FILTER_H  */
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 06/10] ref-filter: introduce format_ref_array_item()

Karthik Nayak
In reply to this post by Karthik Nayak
To allow column display, we will need to first render the output in a
string list to allow print_columns() to compute the proper size of
each column before starting the actual output. Introduce the function
format_ref_array_item() that does the formatting of a ref_array_item
to an strbuf.

show_ref_array_item() is kept as a convenience wrapper around it which
obtains the strbuf and prints it the standard output.

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 ref-filter.c | 16 ++++++++++++----
 ref-filter.h |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index a3cc3af..38e4424 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1756,10 +1756,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting
  }
 }
 
-void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+   int quote_style, struct strbuf *final_buf)
 {
  const char *cp, *sp, *ep;
- struct strbuf *final_buf;
  struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
 
  state.quote_style = quote_style;
@@ -1789,9 +1789,17 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
  }
  if (state.stack->prev)
  die(_("format: %%(end) atom missing"));
- final_buf = &state.stack->output;
- fwrite(final_buf->buf, 1, final_buf->len, stdout);
+ strbuf_addbuf(final_buf, &state.stack->output);
  pop_stack_element(&state.stack);
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
+{
+ struct strbuf final_buf = STRBUF_INIT;
+
+ format_ref_array_item(info, format, quote_style, &final_buf);
+ fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ strbuf_release(&final_buf);
  putchar('\n');
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 4aea594..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
 int verify_ref_format(const char *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/*  Based on the given format and quote_style, fill the strbuf */
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+   int quote_style, struct strbuf *final_buf);
 /*  Print the ref using the given format and quote_style */
 void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style);
 /*  Callback function for parsing the sort option */
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 07/10] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams

Karthik Nayak
In reply to this post by Karthik Nayak
Borrowing from branch.c's implementation print "[gone]" whenever an
unknown upstream ref is encountered instead of just ignoring it.

This makes sure that when branch.c is ported over to using ref-filter
APIs for printing, this feature is not lost.

Make changes to t/t6300-for-each-ref.sh to reflect this change.

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 ref-filter.c            | 4 +++-
 t/t6300-for-each-ref.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 38e4424..6a38089 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)
  char buf[40];
 
  if (stat_tracking_info(branch, &num_ours,
-       &num_theirs, NULL))
+       &num_theirs, NULL)) {
+ v->s = "[gone]";
  continue;
+ }
 
  if (!num_ours && !num_theirs)
  v->s = "";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 6fc569e..4f620bf 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -359,7 +359,7 @@ test_expect_success 'Check that :track[short] cannot be used with other atoms' '
 
 test_expect_success 'Check that :track[short] works when upstream is invalid' '
  cat >expected <<-\EOF &&
-
+ [gone]
 
  EOF
  test_when_finished "git config branch.master.merge refs/heads/master" &&
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)

Karthik Nayak
In reply to this post by Karthik Nayak
Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").

Add test and documentation for the same.
---
 Documentation/git-for-each-ref.txt |  6 ++++--
 ref-filter.c                       | 28 +++++++++++++++++++++++-----
 t/t6300-for-each-ref.sh            |  9 +++++++++
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index c713ec0..a38cbf6 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -114,8 +114,10 @@ upstream::
  `refname` above.  Additionally respects `:track` to show
  "[ahead N, behind M]" and `:trackshort` to show the terse
  version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
- or "=" (in sync).  Has no effect if the ref does not have
- tracking information associated with it.
+ or "=" (in sync).  Append `:track,nobracket` to show tracking
+ information without brackets (i.e "ahead N, behind M").  Has
+ no effect if the ref does not have tracking information
+ associated with it.
 
 push::
  The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 6a38089..6044eb0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)
  if (!strcmp(formatp, "short"))
  refname = shorten_unambiguous_ref(refname,
       warn_ambiguous_refs);
- else if (!strcmp(formatp, "track") &&
+ else if (skip_prefix(formatp, "track", &valp) &&
+ strcmp(formatp, "trackshort") &&
  (starts_with(name, "upstream") ||
   starts_with(name, "push"))) {
  char buf[40];
+ unsigned int nobracket = 0;
+
+ if (!strcmp(valp, ",nobracket"))
+ nobracket = 1;
 
  if (stat_tracking_info(branch, &num_ours,
        &num_theirs, NULL)) {
- v->s = "[gone]";
+ if (nobracket)
+ v->s = "gone";
+ else
+ v->s = "[gone]";
  continue;
  }
 
  if (!num_ours && !num_theirs)
  v->s = "";
  else if (!num_ours) {
- sprintf(buf, "[behind %d]", num_theirs);
+ if (nobracket)
+ sprintf(buf, "behind %d", num_theirs);
+ else
+ sprintf(buf, "[behind %d]", num_theirs);
  v->s = xstrdup(buf);
  } else if (!num_theirs) {
- sprintf(buf, "[ahead %d]", num_ours);
+ if (nobracket)
+ sprintf(buf, "ahead %d", num_ours);
+ else
+ sprintf(buf, "[ahead %d]", num_ours);
  v->s = xstrdup(buf);
  } else {
- sprintf(buf, "[ahead %d, behind %d]",
+ if (nobracket)
+ sprintf(buf, "ahead %d, behind %d",
+ num_ours, num_theirs);
+ else
+ sprintf(buf, "[ahead %d, behind %d]",
  num_ours, num_theirs);
  v->s = xstrdup(buf);
  }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 4f620bf..7ab8bf8 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '
 '
 
 cat >expected <<EOF
+ahead 1
+EOF
+
+test_expect_success 'Check upstream:track,nobracket format' '
+ git for-each-ref --format="%(upstream:track,nobracket)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<EOF
 >
 EOF
 
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 09/10] branch: use ref-filter printing APIs

Karthik Nayak
In reply to this post by Karthik Nayak
Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.

Introduce get_format() which gets the format required for printing of
refs. Make amendments to print_ref_list() to reflect these changes.

Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().

Mentored-by: Christian Couder <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 builtin/branch.c         | 261 ++++++++++++-----------------------------------
 t/t6040-tracking-info.sh |   2 +-
 2 files changed, 66 insertions(+), 197 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 67ef9f1..4d8b570 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -35,12 +35,12 @@ static unsigned char head_sha1[20];
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
- GIT_COLOR_RESET,
- GIT_COLOR_NORMAL, /* PLAIN */
- GIT_COLOR_RED, /* REMOTE */
- GIT_COLOR_NORMAL, /* LOCAL */
- GIT_COLOR_GREEN, /* CURRENT */
- GIT_COLOR_BLUE, /* UPSTREAM */
+ "%(color:reset)",
+ "%(color:reset)", /* PLAIN */
+ "%(color:red)", /* REMOTE */
+ "%(color:reset)", /* LOCAL */
+ "%(color:green)", /* CURRENT */
+ "%(color:blue)", /* UPSTREAM */
 };
 enum color_branch {
  BRANCH_COLOR_RESET = 0,
@@ -271,192 +271,6 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
  return(ret);
 }
 
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
- int show_upstream_ref)
-{
- int ours, theirs;
- char *ref = NULL;
- struct branch *branch = branch_get(branch_name);
- const char *upstream;
- struct strbuf fancy = STRBUF_INIT;
- int upstream_is_gone = 0;
- int added_decoration = 1;
-
- if (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) {
- if (!upstream)
- return;
- upstream_is_gone = 1;
- }
-
- if (show_upstream_ref) {
- ref = shorten_unambiguous_ref(upstream, 0);
- if (want_color(branch_use_color))
- strbuf_addf(&fancy, "%s%s%s",
- branch_get_color(BRANCH_COLOR_UPSTREAM),
- ref, branch_get_color(BRANCH_COLOR_RESET));
- else
- strbuf_addstr(&fancy, ref);
- }
-
- if (upstream_is_gone) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
- else
- added_decoration = 0;
- } else if (!ours && !theirs) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s]"), fancy.buf);
- else
- added_decoration = 0;
- } else if (!ours) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, theirs);
- else
- strbuf_addf(stat, _("[behind %d]"), theirs);
-
- } else if (!theirs) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
- else
- strbuf_addf(stat, _("[ahead %d]"), ours);
- } else {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
-    fancy.buf, ours, theirs);
- else
- strbuf_addf(stat, _("[ahead %d, behind %d]"),
-    ours, theirs);
- }
- strbuf_release(&fancy);
- if (added_decoration)
- strbuf_addch(stat, ' ');
- free(ref);
-}
-
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-     struct ref_filter *filter, const char *refname)
-{
- struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
- const char *sub = _(" **** invalid ref ****");
- struct commit *commit = item->commit;
-
- if (!parse_commit(commit)) {
- pp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);
- sub = subject.buf;
- }
-
- if (item->kind == FILTER_REFS_BRANCHES)
- fill_tracking_info(&stat, refname, filter->verbose > 1);
-
- strbuf_addf(out, " %s %s%s",
- find_unique_abbrev(item->commit->object.sha1, filter->abbrev),
- stat.buf, sub);
- strbuf_release(&stat);
- strbuf_release(&subject);
-}
-
-/*
- * This is duplicated in ref-filter.c, will be removed when we adopt
- * ref-filter's printing APIs.
- */
-static char *get_head_description(void)
-{
- struct strbuf desc = STRBUF_INIT;
- struct wt_status_state state;
- memset(&state, 0, sizeof(state));
- wt_status_get_state(&state, 1);
- if (state.rebase_in_progress ||
-    state.rebase_interactive_in_progress)
- strbuf_addf(&desc, _("(no branch, rebasing %s)"),
-    state.branch);
- else if (state.bisect_in_progress)
- strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
-    state.branch);
- else if (state.detached_from) {
- /* TRANSLATORS: make sure these match _("HEAD detached at ")
-   and _("HEAD detached from ") in wt-status.c */
- if (state.detached_at)
- strbuf_addf(&desc, _("(HEAD detached at %s)"),
- state.detached_from);
- else
- strbuf_addf(&desc, _("(HEAD detached from %s)"),
- state.detached_from);
- }
- else
- strbuf_addstr(&desc, _("(no branch)"));
- free(state.branch);
- free(state.onto);
- free(state.detached_from);
- return strbuf_detach(&desc, NULL);
-}
-
-static void format_and_print_ref_item(struct ref_array_item *item, int maxwidth,
-      struct ref_filter *filter, const char *remote_prefix)
-{
- char c;
- int current = 0;
- int color;
- struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
- const char *prefix = "";
- const char *desc = item->refname;
- char *to_free = NULL;
-
- switch (item->kind) {
- case FILTER_REFS_BRANCHES:
- skip_prefix(desc, "refs/heads/", &desc);
- if (!filter->detached && !strcmp(desc, head))
- current = 1;
- else
- color = BRANCH_COLOR_LOCAL;
- break;
- case FILTER_REFS_REMOTES:
- skip_prefix(desc, "refs/remotes/", &desc);
- color = BRANCH_COLOR_REMOTE;
- prefix = remote_prefix;
- break;
- case FILTER_REFS_DETACHED_HEAD:
- desc = to_free = get_head_description();
- current = 1;
- break;
- default:
- color = BRANCH_COLOR_PLAIN;
- break;
- }
-
- c = ' ';
- if (current) {
- c = '*';
- color = BRANCH_COLOR_CURRENT;
- }
-
- strbuf_addf(&name, "%s%s", prefix, desc);
- if (filter->verbose) {
- int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf);
- strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color),
-    maxwidth + utf8_compensation, name.buf,
-    branch_get_color(BRANCH_COLOR_RESET));
- } else
- strbuf_addf(&out, "%c %s%s%s", c, branch_get_color(color),
-    name.buf, branch_get_color(BRANCH_COLOR_RESET));
-
- if (item->symref) {
- skip_prefix(item->symref, "refs/remotes/", &desc);
- strbuf_addf(&out, " -> %s", desc);
- }
- else if (filter->verbose)
- /* " f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h" */
- add_verbose_info(&out, item, filter, desc);
- if (column_active(colopts)) {
- assert(!filter->verbose && "--column and --verbose are incompatible");
- string_list_append(&output, out.buf);
- } else {
- printf("%s\n", out.buf);
- }
- strbuf_release(&name);
- strbuf_release(&out);
- free(to_free);
-}
-
 static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
 {
  int i, max = 0;
@@ -467,7 +281,10 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
 
  skip_prefix(it->refname, "refs/heads/", &desc);
  skip_prefix(it->refname, "refs/remotes/", &desc);
- w = utf8_strwidth(desc);
+ if (it->kind == FILTER_REFS_DETACHED_HEAD)
+ w = strlen(get_head_description());
+ else
+ w = utf8_strwidth(desc);
 
  if (it->kind == FILTER_REFS_REMOTES)
  w += remote_bonus;
@@ -477,12 +294,51 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
  return max;
 }
 
+static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)
+{
+ struct strbuf fmt = STRBUF_INIT;
+ struct strbuf local = STRBUF_INIT;
+ struct strbuf remote = STRBUF_INIT;
+
+ strbuf_addf(&fmt, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)", branch_get_color(BRANCH_COLOR_CURRENT));
+
+ if (filter->verbose) {
+ strbuf_addf(&local, "%%(align:%d,left)%%(refname:short)%%(end)", maxwidth);
+ strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
+ strbuf_addf(&local, " %%(objectname:short=7) ");
+
+ if (filter->verbose > 1)
+ strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
+    "%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
+    branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));
+ else
+ strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
+
+ strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)"
+    "%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
+    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,
+    remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
+ } else {
+ strbuf_addf(&local, "%%(refname:short)%s", branch_get_color(BRANCH_COLOR_RESET));
+ strbuf_addf(&remote, "%s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
+    branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
+ }
+
+ strbuf_addf(&fmt, "%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)", local.buf, remote.buf);
+
+ strbuf_release(&local);
+ strbuf_release(&remote);
+ return strbuf_detach(&fmt, NULL);
+}
+
 static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)
 {
  int i;
  struct ref_array array;
  int maxwidth = 0;
  const char *remote_prefix = "";
+ struct strbuf out = STRBUF_INIT;
+ char *format;
 
  /*
  * If we are listing more than just remote branches,
@@ -494,12 +350,14 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 
  memset(&array, 0, sizeof(array));
 
- verify_ref_format("%(refname)%(symref)");
  filter_refs(&array, filter, filter->kind | FILTER_REFS_INCLUDE_BROKEN);
 
  if (filter->verbose)
  maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
+ format = build_format(filter, maxwidth, remote_prefix);
+ verify_ref_format(format);
+
  /*
  * If no sorting parameter is given then we default to sorting
  * by 'refname'. This would give us an alphabetically sorted
@@ -511,10 +369,21 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
  sorting = ref_default_sorting();
  ref_array_sort(sorting, &array);
 
- for (i = 0; i < array.nr; i++)
- format_and_print_ref_item(array.items[i], maxwidth, filter, remote_prefix);
+ for (i = 0; i < array.nr; i++) {
+ format_ref_array_item(array.items[i], format, 0, &out);
+ if (column_active(colopts)) {
+ assert(!filter->verbose && "--column and --verbose are incompatible");
+ /* format to a string_list to let print_columns() do its job */
+ string_list_append(&output, out.buf);
+ } else {
+ fwrite(out.buf, 1, out.len, stdout);
+ putchar('\n');
+ }
+ strbuf_release(&out);
+ }
 
  ref_array_clear(&array);
+ free(format);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 3d5c238..97a0765 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -44,7 +44,7 @@ b1 [ahead 1, behind 1] d
 b2 [ahead 1, behind 1] d
 b3 [behind 1] b
 b4 [ahead 2] f
-b5 g
+b5 [gone] g
 b6 c
 EOF
 
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 10/10] branch: implement '--format' option

Karthik Nayak
In reply to this post by 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 <[hidden email]>
Mentored-by: Matthieu Moy <[hidden email]>
Signed-off-by: Karthik Nayak <[hidden email]>
---
 Documentation/git-branch.txt |  7 ++++++-
 builtin/branch.c             | 14 +++++++++-----
 t/t3203-branch-output.sh     | 11 +++++++++++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 03c7af1..5227206 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
  [--list] [-v [--abbrev=<length> | --no-abbrev]]
  [--column[=<options>] | --no-column]
  [(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
- [--points-at <object>] [<pattern>...]
+ [--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' --unset-upstream [<branchname>]
@@ -244,6 +244,11 @@ start-point is either a local or remote-tracking branch.
 --points-at <object>::
  Only list branches of the given object.
 
+--format <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].
+
 Examples
 --------
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 4d8b570..2356e72 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
  N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
  N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
  N_("git branch [<options>] [-r | -a] [--points-at]"),
+ N_("git branch [<options>] [-r | -a] [--format]"),
  NULL
 };
 
@@ -331,14 +332,14 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
  return strbuf_detach(&fmt, NULL);
 }
 
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
 {
  int i;
  struct ref_array array;
  int maxwidth = 0;
  const char *remote_prefix = "";
  struct strbuf out = STRBUF_INIT;
- char *format;
+ char *to_free = NULL;
 
  /*
  * If we are listing more than just remote branches,
@@ -355,7 +356,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
  if (filter->verbose)
  maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
- format = build_format(filter, maxwidth, remote_prefix);
+ if (!format)
+ format = to_free = build_format(filter, maxwidth, remote_prefix);
  verify_ref_format(format);
 
  /*
@@ -383,7 +385,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
  }
 
  ref_array_clear(&array);
- free(format);
+ free(to_free);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
@@ -484,6 +486,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
  enum branch_track track;
  struct ref_filter filter;
  static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
+ const char *format = NULL;
 
  struct option options[] = {
  OPT_GROUP(N_("Generic options")),
@@ -524,6 +527,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
  OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
  N_("print only branches of the object"), 0, parse_opt_object_name
  },
+ OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
  OPT_END(),
  };
 
@@ -582,7 +586,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
  if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
  filter.kind |= FILTER_REFS_DETACHED_HEAD;
  filter.name_patterns = argv;
- print_ref_list(&filter, sorting);
+ print_ref_list(&filter, sorting, format);
  print_columns(&output, colopts, NULL);
  string_list_clear(&output, 0);
  return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index f1ae5ff..a475ff1 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -163,4 +163,15 @@ test_expect_success 'git branch --points-at option' '
  test_cmp expect actual
 '
 
+test_expect_success 'git branch --format option' '
+ cat >expect <<-\EOF &&
+ Refname is (HEAD detached from fromtag)
+ Refname is refs/heads/branch-one
+ Refname is refs/heads/branch-two
+ Refname is refs/heads/master
+ EOF
+ git branch --format="Refname is %(refname)" >actual &&
+ test_cmp expect actual
+'
+
 test_done
--
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Matthieu Moy-2
In reply to this post by Karthik Nayak
Karthik Nayak <[hidden email]> writes:

> +An example to show the usage of %(if)...%(then)...%(else)...%(end).
> +This prefixes the current branch with a star.
> +
> +------------
> +#!/bin/sh
> +
> +git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  %(end)%(refname:short)" refs/heads/

I don't think the #!/bin/sh adds any value here. Just the 'git
for-each-ref' line is sufficient IMHO.

> +An example to show the usage of %(if)...%(then)...%(end).
> +This adds a red color to authorname, if present

I don't think this is such a good example.
%(color:red)%(authorname)%(color:reset) just works even if %(authorname)
is empty.

A better example would be

git for-each-ref --format='%(if)%(authorname)%(then)Authored by %(authorname)%(end)'

which avoids writting "Authored by " with no author.

> -static int is_empty(const char * s){
> +static int is_empty(const char *s){

You still have the { on the declaration line, it should be on the next
line.

> @@ -309,10 +311,14 @@ static int is_empty(const char * s){
>  static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
>  {
>   struct ref_formatting_stack *cur = state->stack;
> - struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
> + struct if_then_else *if_then_else = NULL;
>  
> + if (cur->at_end == if_then_else_handler)
> + if_then_else = (struct if_then_else *)cur->at_end_data;

OK, now the cast is safe since at_end_data has to be of type struct
if_then_else * if at_end is if_then_else_handler.

> + unsigned int nobracket = 0;
> +
> + if (!strcmp(valp, ",nobracket"))
> + nobracket = 1;

The code to parse comma-separated values is different here and
elsewhere. I'd rather have the same code (possibly factored into a
helper function), both to get consistent behavior (you're not allowing
%(upstream:nobracket,track) for example, right?) and consistent code.

>   if (!num_ours && !num_theirs)
>   v->s = "";
>   else if (!num_ours) {
> - sprintf(buf, "[behind %d]", num_theirs);
> + if (nobracket)
> + sprintf(buf, "behind %d", num_theirs);
> + else
> + sprintf(buf, "[behind %d]", num_theirs);

Perhaps use sprintf(buf, "%sbehind %d%s", obracket, num_their, cbracket)
unconditionnally, and set obracket = "" or obracket = "[" once and for
all when you test for "nobracket" above. This avoids these "if
(nobracket)" spread accross the code, but at the price of extra %s in
the format strings.

> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)
>   else
>   v->s = "<>";
>   continue;
> + } else if (!strcmp(formatp, "dir") &&
> +   (starts_with(name, "refname"))) {
> + const char *sp, *ep, *tmp;
> +
> + sp = tmp = ref->refname;
> + /*  Obtain refs/foo/bar/ from refname refs/foo/bar/abc */
> + do {
> + ep = tmp;
> + tmp = strchr(ep + 1, '/');
> + } while (tmp);

To look for the last occurence of '/' you can also use strrchr().
Doesn't it do what you want here?

> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '
>  '
>  
>  cat >expect <<\EOF
> -b1 [origin/master] [ahead 1, behind 1] d
> -b2 [origin/master] [ahead 1, behind 1] d
> -b3 [origin/master] [behind 1] b
> -b4 [origin/master] [ahead 2] f
> -b5 [brokenbase] [gone] g
> +b1 [origin/master: ahead 1, behind 1] d
> +b2 [origin/master: ahead 1, behind 1] d
> +b3 [origin/master: behind 1] b
> +b4 [origin/master: ahead 2] f
> +b5 [brokenbase: gone] g
>  b6 [origin/master] c
>  EOF

Cool!

I didn't go through the patches themselves, but modulo my remarks above
the interdiff looks good. Thanks.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Karthik Nayak
On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
<[hidden email]> wrote:

> Karthik Nayak <[hidden email]> writes:
>
>> +An example to show the usage of %(if)...%(then)...%(else)...%(end).
>> +This prefixes the current branch with a star.
>> +
>> +------------
>> +#!/bin/sh
>> +
>> +git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  %(end)%(refname:short)" refs/heads/
>
> I don't think the #!/bin/sh adds any value here. Just the 'git
> for-each-ref' line is sufficient IMHO.
>

Sure, we can remove that.

>> +An example to show the usage of %(if)...%(then)...%(end).
>> +This adds a red color to authorname, if present
>
> I don't think this is such a good example.
> %(color:red)%(authorname)%(color:reset) just works even if %(authorname)
> is empty.
>
> A better example would be
>
> git for-each-ref --format='%(if)%(authorname)%(then)Authored by %(authorname)%(end)'
>
> which avoids writting "Authored by " with no author.
>

Yeah, will include this.

>> -static int is_empty(const char * s){
>> +static int is_empty(const char *s){
>
> You still have the { on the declaration line, it should be on the next
> line.
>

Oops, will change that.

>> @@ -309,10 +311,14 @@ static int is_empty(const char * s){
>>  static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
>>  {
>>       struct ref_formatting_stack *cur = state->stack;
>> -     struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
>> +     struct if_then_else *if_then_else = NULL;
>>
>> +     if (cur->at_end == if_then_else_handler)
>> +             if_then_else = (struct if_then_else *)cur->at_end_data;
>
> OK, now the cast is safe since at_end_data has to be of type struct
> if_then_else * if at_end is if_then_else_handler.
>
>> +                             unsigned int nobracket = 0;
>> +
>> +                             if (!strcmp(valp, ",nobracket"))
>> +                                     nobracket = 1;
>
> The code to parse comma-separated values is different here and
> elsewhere. I'd rather have the same code (possibly factored into a
> helper function), both to get consistent behavior (you're not allowing
> %(upstream:nobracket,track) for example, right?) and consistent code.
>

Eh, okay, will do that!

>>                               if (!num_ours && !num_theirs)
>>                                       v->s = "";
>>                               else if (!num_ours) {
>> -                                     sprintf(buf, "[behind %d]", num_theirs);
>> +                                     if (nobracket)
>> +                                             sprintf(buf, "behind %d", num_theirs);
>> +                                     else
>> +                                             sprintf(buf, "[behind %d]", num_theirs);
>
> Perhaps use sprintf(buf, "%sbehind %d%s", obracket, num_their, cbracket)
> unconditionnally, and set obracket = "" or obracket = "[" once and for
> all when you test for "nobracket" above. This avoids these "if
> (nobracket)" spread accross the code, but at the price of extra %s in
> the format strings.
>

Okay! will do that.

>> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)
>>                               else
>>                                       v->s = "<>";
>>                               continue;
>> +                     } else if (!strcmp(formatp, "dir") &&
>> +                                (starts_with(name, "refname"))) {
>> +                             const char *sp, *ep, *tmp;
>> +
>> +                             sp = tmp = ref->refname;
>> +                             /*  Obtain refs/foo/bar/ from refname refs/foo/bar/abc */
>> +                             do {
>> +                                     ep = tmp;
>> +                                     tmp = strchr(ep + 1, '/');
>> +                             } while (tmp);
>
> To look for the last occurence of '/' you can also use strrchr().
> Doesn't it do what you want here?
>

Didn't know that, thanks will do that.

>> --- a/t/t6040-tracking-info.sh
>> +++ b/t/t6040-tracking-info.sh
>> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '
>>  '
>>
>>  cat >expect <<\EOF
>> -b1 [origin/master] [ahead 1, behind 1] d
>> -b2 [origin/master] [ahead 1, behind 1] d
>> -b3 [origin/master] [behind 1] b
>> -b4 [origin/master] [ahead 2] f
>> -b5 [brokenbase] [gone] g
>> +b1 [origin/master: ahead 1, behind 1] d
>> +b2 [origin/master: ahead 1, behind 1] d
>> +b3 [origin/master: behind 1] b
>> +b4 [origin/master: ahead 2] f
>> +b5 [brokenbase: gone] g
>>  b6 [origin/master] c
>>  EOF
>
> Cool!
>
> I didn't go through the patches themselves, but modulo my remarks above
> the interdiff looks good. Thanks.
>

Thanks for the review.

--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Karthik Nayak
In reply to this post by Matthieu Moy-2
On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
<[hidden email]> wrote:

>> +                             unsigned int nobracket = 0;
>> +
>> +                             if (!strcmp(valp, ",nobracket"))
>> +                                     nobracket = 1;
>
> The code to parse comma-separated values is different here and
> elsewhere. I'd rather have the same code (possibly factored into a
> helper function), both to get consistent behavior (you're not allowing
> %(upstream:nobracket,track) for example, right?) and consistent code.
>

Speaking of comma-separated values, the only other place we use that is
in the align atom. Also I find this very specific to get a function out of.
Somehow I think this is the simplest way to go about this.

--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Matthieu Moy-2
Karthik Nayak <[hidden email]> writes:

> On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
> <[hidden email]> wrote:
>>> +                             unsigned int nobracket = 0;
>>> +
>>> +                             if (!strcmp(valp, ",nobracket"))
>>> +                                     nobracket = 1;
>>
>> The code to parse comma-separated values is different here and
>> elsewhere. I'd rather have the same code (possibly factored into a
>> helper function), both to get consistent behavior (you're not allowing
>> %(upstream:nobracket,track) for example, right?) and consistent code.
>>
>
> Speaking of comma-separated values, the only other place we use that is
> in the align atom. Also I find this very specific to get a function out of.
> Somehow I think this is the simplest way to go about this.

Well, most pieces of code start with one instance, then two, then
more ;-). When the second instance starts being different from the
first, it doesn't give a good example for the future third instance.

This particular piece of code is so important and I won't fight for a
better factored one, but in general "there are only two instances" is a
dubious argument to avoid refactoring.

Still, I find it weird to force the nobracket to be always at the same
position.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Karthik Nayak
On Thu, Oct 8, 2015 at 10:40 PM, Matthieu Moy
<[hidden email]> wrote:

> Karthik Nayak <[hidden email]> writes:
>
>> On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
>> <[hidden email]> wrote:
>>>> +                             unsigned int nobracket = 0;
>>>> +
>>>> +                             if (!strcmp(valp, ",nobracket"))
>>>> +                                     nobracket = 1;
>>>
>>> The code to parse comma-separated values is different here and
>>> elsewhere. I'd rather have the same code (possibly factored into a
>>> helper function), both to get consistent behavior (you're not allowing
>>> %(upstream:nobracket,track) for example, right?) and consistent code.
>>>
>>
>> Speaking of comma-separated values, the only other place we use that is
>> in the align atom. Also I find this very specific to get a function out of.
>> Somehow I think this is the simplest way to go about this.
>
> Well, most pieces of code start with one instance, then two, then
> more ;-). When the second instance starts being different from the
> first, it doesn't give a good example for the future third instance.
>

Totally agree with you here.

> This particular piece of code is so important and I won't fight for a
> better factored one, but in general "there are only two instances" is a
> dubious argument to avoid refactoring.
>
> Still, I find it weird to force the nobracket to be always at the same
> position.
>

No i mean I could follow up with the way we use it in align, but I don't see
how I can make a function out of this.

--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Matthieu Moy-2
Karthik Nayak <[hidden email]> writes:

> On Thu, Oct 8, 2015 at 10:40 PM, Matthieu Moy
> <[hidden email]> wrote:
>
>> This particular piece of code is so important and I won't fight for a
>> better factored one, but in general "there are only two instances" is a
>> dubious argument to avoid refactoring.
>>
>> Still, I find it weird to force the nobracket to be always at the same
>> position.
>>
>
> No i mean I could follow up with the way we use it in align, but I don't see
> how I can make a function out of this.

Ah, OK. Well, there are already two instances of

        /*  Strip trailing comma */
        if (s[1])
                strbuf_setlen(s[0], s[0]->len - 1);


(for objectname and align), one of which says

        /*
         * TODO: Implement a function similar to strbuf_split_str()
         * which would omit the separator from the end of each value.
         */

I guess it's time to replace this TODO with actual code. A
straightforward implementation would be to call strbuf_split_str and
then use the same "if (s[1])" as you already have. There's probably a
more efficient way.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

Junio C Hamano
In reply to this post by Karthik Nayak
Karthik Nayak <[hidden email]> writes:

> No i mean I could follow up with the way we use it in align, but I don't see
> how I can make a function out of this.

At least you should be able to pre-parse the %(<atom>:<modifier>)
construct, instead of doing strcmp() every time populate_value() is
called, no?  Then your parser would not be doing

>>>> +                             if (!strcmp(valp, ",nobracket"))
>>>> +                                     nobracket = 1;

with 'valp' that scans over 'name' (which is an element of
used_atom[], i.e. a name from valid_atom[] followed by ":<modifier>"
for each customization).  Instead your used_atom[] would be an array
of a data structure that is richer than just a string.

The <modifier> would be split at comma boundary and made into an
array of two field structure, as the most general form of it is

    <modifier> ::= <attrval> | <attrval> ',' <modifier>
    <attrval> ::= <attr> '=' <value> | <attr> | <value>

if we recall the discussion we had while designing %(align), i.e.

 * The most general form is "%(align:position=left,width=32)"

 * The value domain of attributes may be distinct, in which case the
   value itself could imply what attr it is talking about,
   e.g. "%(align:32,left)" is sufficiently clear as '32' cannot be
   position and 'left' cannot be width.

And clearly there can be an attr whose presense alone can imply its
value (iow, a boolean), even though your %(align) does not yet have
such a modifier.  It is easy to imagine that you would later want to
add %(align:position=left,width=32,truncate) that truncates the value
when its display width exceeds '32'.  The 'nobracket' above would be
an another example of a boolean (whose name is negative, which is
not a very good design in general, though).

Then used_atom[] could become something like

    struct {
    const char *str; /* e.g. "align:position=left,32" */
        struct {
        const char *part0; /* everything before '=' */
                const char *part1; /* optional */
        } *modifier;
        int modifier_nr;
    } *used_atom;

and "align:position=left,32" would be parsed into

        .str = "align:position=left,32";
        .modifier = { { "position", "left" }, { "32", NULL } };
        .modifier_nr = 2;

when the format string is read, which is done only once.

The looping over all the refs done in populate_value() could just
use the pre-parsed representation.

Hmm?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams

Matthieu Moy-2
In reply to this post by Karthik Nayak
Karthik Nayak <[hidden email]> writes:

> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)
>   char buf[40];
>  
>   if (stat_tracking_info(branch, &num_ours,
> -       &num_theirs, NULL))
> +       &num_theirs, NULL)) {
> + v->s = "[gone]";

My remark about translation still holds. The string was previously
translated in "branch" and you are removing this translation (well, not
here, but when 09/10 starts using this code).

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms

Matthieu Moy-2
In reply to this post by Karthik Nayak
Karthik Nayak <[hidden email]> writes:

> +static void if_then_else_handler(struct ref_formatting_stack **stack)
> +{
> + struct ref_formatting_stack *cur = *stack;
> + struct ref_formatting_stack *prev = cur->prev;
> + struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
> +

You should add

        if (!if_then_else->then_atom)
                die(_("format: %%(if) atom used without a %%(then) atom"));

here ...

> +static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
> +{
> + struct ref_formatting_stack *cur = state->stack;
> + struct if_then_else *if_then_else = NULL;
> +
> + if (cur->at_end == if_then_else_handler)
> + if_then_else = (struct if_then_else *)cur->at_end_data;
> + if (!if_then_else)
> + die(_("format: %%(then) atom used without an %%(if) atom"));
> + if (if_then_else->then_atom)
> + die(_("format: %%(then) atom used more than once"));
> + if_then_else->then_atom = 1;

... and

        if (if_then_else->else_atom)
                die(_("format: %%(then) atom used after %%(else)"));

here, just in case (adding the two corresponding test_must_fail wouldn't
harm of course).

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
1234