Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 10, 2015, 16:20 UTC
- Message-ID
- <xmqqk2u8kmre.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1436437671-25600-1-git-send-email-karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 12 quoted lines
> Add support for %(refname:shortalign=X) where X is a number. > This will print a shortened refname aligned to the left > followed by spaces for a total length of X characters. > If X is less than the shortened refname size, the entire > shortened refname is printed. > > Mentored-by: Christian Couder <christian.couder@gmail.com> > Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr> > Signed-off-by: Karthik Nayak <karthik.188@gmail.com> > --- > ref-filter.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-)
This may be enough to support the various existing formats that are offered by "git branch" and/or "git tag", but I do not think if this is the right approach in the longer term, or if we are painting ourselves in a corner we cannot cleanly get out of later [*1*]. Will the "refname" stay to be the only thing that may want alignment padding appended in the future? Will it stay true that we want to align only to the left? Etc., etc.
Cc'ed Duy as %< in the pretty-format was his invention at around a5752342 (pretty: support padding placeholders, %< %> and %><, 2013-04-19).
Show 18 quoted lines
> diff --git a/ref-filter.c b/ref-filter.c
> index dd0709d..3098497 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -10,6 +10,7 @@
> #include "quote.h"
> #include "ref-filter.h"
> #include "revision.h"
> +#include "utf8.h"
>
> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>
> @@ -695,7 +696,23 @@ static void populate_value(struct ref_array_item *ref)
> int num_ours, num_theirs;
>
> formatp++;
> - if (!strcmp(formatp, "short"))
> + if (starts_with(formatp, "shortalign=")) {When adding a new thing to an existing list, we prefer to append it at the end of the list, if there is no other reason not to do so (e.g. "the existing list is sorted in this order, and the new location was chosen to fit the new item to honor the existing ordering rule" is a valid reason to put it at the beginning, if the existing sorting rule dictates that the new thing must come at the beginning).
Show 5 quoted lines
> + const char *valp, *short_refname = NULL; > + int val, len; > + > + skip_prefix(formatp, "shortalign=", &valp); > + val = atoi(valp);
In newer code, we would want to avoid atoi() so that "foo:shortalign=1z" that is a typo of "12" can be caught as an error. Either strtol_i() or strtoul_ui() may be better (we would need to adjust it further when Michael decides to resurrect his numparse thing that has been in the stalled bin for quite a while, though).
Show 10 quoted lines
> + refname = short_refname = shorten_unambiguous_ref(refname,
> + warn_ambiguous_refs);
> + len = utf8_strwidth(refname);
> + if (val > len) {
> + struct strbuf buf = STRBUF_INIT;
> + strbuf_addstr(&buf, refname);
> + strbuf_addchars(&buf, ' ', val - len);
> + free((char *)short_refname);
> + refname = strbuf_detach(&buf, NULL);
> + }What should happen when the display column width of the string is wider? If a user wants to align the refs that are usually usually short start the next thing at the 8th column, which should she use?
"%(refname:shorta=7) %(next item)"
"%(refname:shorta=8)%(next item)"> + } else if (!strcmp(formatp, "short")) > refname = shorten_unambiguous_ref(refname, > warn_ambiguous_refs); > else if (!strcmp(formatp, "track") &&