From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mon, 20 Jul 2015 00:02:54 GMT
Subject: Re: [PATCH v3 3/9] ref-filter: support printing N lines from tag annotation
Message-ID: <CAPig+cSXuJ7ohR=afzRbmfLtEuzhGYYTznc2J_qxwgBKwyYu9Q@mail.gmail.com>
In-Reply-To: <1437246749-14423-4-git-send-email-Karthik.188@gmail.com>

On Sat, Jul 18, 2015 at 3:12 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> In 'tag.c' we can print N lines from the annotation of the tag using
> the '-n<num>' option. Copy code from 'tag.c' to 'ref-filter' and
> modify 'ref-filter' to support printing of N lines from the annotation
> of tags.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index 771c48d..82731ac 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1288,7 +1288,48 @@ static void assign_formating(struct ref_array_item *ref, int parsed_atom, struct
>         }
>  }
>
> -void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
> +/* Print 'lines' no of lines of a given oid */

This is difficult to read and understand. I presume you meant "no." as
shorthand for "number" but dropped the period. Even with the period,
it's still hard to read. Perhaps rewrite it as:

    If 'lines' is greater than 0, print that may lines of...

or something.

> diff --git a/ref-filter.h b/ref-filter.h
> index c18781b..7dfdea0 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -87,8 +88,11 @@ 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);
> -/*  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);
> +/*
> + * Print the ref using the given format and quote_style. If lines > 0,
> + * prints the "lines" no of lines of the objected pointed to.
> + */

Same problem. This literal "no" is quite confusing. Perhaps rewrite as above:

    If lines > 0, print that many lines of...

> +void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style, unsigned int lines);

ref-filter.h seems to be setting a precedent for overlong lines.
Wrapping this manually could improve readability.

>  /*  Callback function for parsing the sort option */
>  int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);
>  /*  Default sort option based on refname */
> --
> 2.4.6