Re: [PATCH v10 04/13] utf8: add function to align a string into given strbuf

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 13, 2015, 19:08 UTC
Message-ID: <CAPig+cS+rS=xKX-LrJSTqFgaopuLzbh3hqURFWXfzMXH=XUk2w@mail.gmail.com>
In-Reply-To: <1439129506-9989-5-git-send-email-Karthik.188@gmail.com>

On Sun, Aug 9, 2015 at 10:11 AM, Karthik Nayak <karthik.188@gmail.com> wrote:

> Add strbuf_utf8_align() which will align a given string into a strbuf
> as per given align_type and width. If the width is greater than the
> string length then no alignment is performed.

In addition to Junio's valuable comments...

Show 20 quoted lines

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/utf8.h b/utf8.h
> index 5a9e94b..db8ca63 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -55,4 +55,17 @@ int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
>   */
>  int is_hfs_dotgit(const char *path);
>
> +typedef enum {
> +       ALIGN_LEFT,
> +       ALIGN_MIDDLE,
> +       ALIGN_RIGHT
> +} align_type;
> +
> +/*
> + * Align the string given and store it into a strbuf as per the type
> + * and width.
> + */

Please extend this documentation to state explicitly that this function preserves (does not truncate) the input string if it is wider than 'width'. That's quite important information for the caller to know.

(Aside: I could easily see this function being extended to support optional truncation, but that's a separate topic, and something that can be done by someone else when the feature is actually needed; it's not your responsibility.)

> +void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,
> +                      const char *s);
> +
>  #endif