On 08.02.13 08:20, Jiang Xin wrote:
> 2013/2/8 Torsten Bögershausen <tboegi@xxxxxx>:
>> On 08.02.13 03:10, Jiang Xin wrote:
>>> + /* If no error occurs, returns columns really required with utf8_strwidth. */
>>> + if (0 <= columns)
>>> + columns = utf8_strwidth(buf.buf);
>>> + strbuf_release(&buf);
>>> + return columns;
>>> +}
>>> +
>>
>> I don't think we handle the return code from fputs() correctly.
>>
>> Please dee below for specifications on fprintf(),
>> something like the following could do:
>>
>> int utf8_fprintf(FILE *stream, const char *format, ...)
>> {
>> struct strbuf buf = STRBUF_INIT;
>> va_list arg;
>> int columns = 0;
>>
>> va_start (arg, format);
>> strbuf_vaddf(&buf, format, arg);
>> va_end (arg);
>>
>> if (EOF != fputs(buf.buf, stream))
>> columns = utf8_strwidth(buf.buf);
>> strbuf_release(&buf);
>> return columns;
>> }
>
> As fputs() returns a non-negative number (as opposed to 0) on
> successful completion,
> Test fputs() return value as "fputs() >=0" is correct, while "fputs()
> == 0", "fputs() != 0"
> are wrong. I think it's OK, must I send a new re-roll for this?
>
> EOF is defined as (-1) in stdio.h:
>
> #define EOF (-1)
>
>> And as a side note: would fprintf_strwidth() be a better name?
>
> This is a nice candidate.
>
>
Doing a slurp(coffe) followed by a re-read,
I think we are save with the existng code.
However, I feel that the commit message can be improved,
as it fixes problems not only for CJK but e.g. european languages
(and all systems using utf-8)
>Since command usages can be translated, they may not align well especially
>when they are translated to CJK. A wrapper utf8_fprintf can help to return
>the correct columns required.
Since command usages can be translated, they may include utf-8 encoded strings,
and strlen() is different from strwidth().
A wrapper utf8_fprintf() helps to return the correct columns required.
Thanks for working on this.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html