Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Jul 9, 2015, 12:58 UTC
- Message-ID
- <vpq380x8p3a.fsf@anie.imag.fr>
- In-Reply-To
- <1436437671-25600-1-git-send-email-karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 5 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.
Not really an issue, but you're wrapping your text at ~60 characters. The common use is to wrap around 70 to 80. Using Emacs, auto-fill-mode or M-q does this automatically. If you use another text editor, it can probably do that for you too.
Show 5 quoted lines
> 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 ++++++++++++++++++-
I think this would deserve a test and documentation. Even though your motivation is for an internal implementation, some users may want to use the feature in 'git for-each-ref --format=...'.
Show 25 quoted lines
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> 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=")) {
> + const char *valp, *short_refname = NULL;
> + int val, len;
> +
> + skip_prefix(formatp, "shortalign=", &valp);
> + val = atoi(valp);You're silently accepting %(refname:shortalign=foo) and %(refname:shortalign=). I think it would be better to reject such cases explicitly.
-- Matthieu Moy http://www-verimag.imag.fr/~moy/