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

[Karthik Nayak <karthik.188@gmail.com>]

On Fri, Aug 14, 2015 at 12:38 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 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...
>
>> 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

Yeah will do :)

-- 
Regards,
Karthik Nayak