On Wed, Nov 25, 2015 at 04:34:17PM +0300, Andrey Chernov wrote:
> On 25.11.2015 15:53, Baptiste Daroussin wrote:
> > What I did for now is set max_month_width to -1 and in ls_strftime I
> > fallback on
> > the plain strftime meaning you keep localized information but the
> > alignement is
> > broken as of now.
>
> It will be enough.
>
> >> 3) wcwidth/wcswidth may return -1 too, it needs to be checked too.
> > done and truncate the name of the month to the latest valid character
>
> I think there is no point for sofisticated truncating, if locale is not
> valid. F.e. you may truncate to just 1 char. The thing above will be
> enough for all such cases.
>
> > Review updated (if you prefer a diff by mail just tell me, given you do not
> > have
> > a phabricator account.)
>
> I don't have phabricator.
>
> This one
> if ((n = max_month_width - wab_months_width[i]) > 0) {
> wcslcat(wab_months[i], L" "/* MAX_ABMON_WIDTH */,
> max_month_width + 1);
> }
> should be
> if ((n = max_month_width - wab_months_width[i]) > 0)
> wcslcat(wab_months[i], L" "/* MAX_ABMON_WIDTH */, n);
>
> I.e. you append n spaces and n is the difference between max and current.
With wcsncat I would agree, but not with wcslcat.
wcslcat works like strlcat:
to quote the manpage:
"It will append at most dstsize - strlen(dst) - 1 characters."
So here with your version it will be n - wcslen(wab_months[i]) -1
which won't fit what we want to do.
btw that makes me figure out that what I want is wcsncat because we do care to
make sure we have appended the right number of spaces at the end of the
abbreviated month based on character width, not the on the len of the wide
string
so I changed it
if ((n = max_month_width - wab_months_width[i]) > 0)
wcsncat(wab_months[i], L" "/* MAX_ABMON_WIDTH */, n);
If you do not have further comments, I will commit that later today.
Then I will work on libarchive and pax (would you like to review the patches for
those?)
Best regards,
Bapt
signature.asc
Description: PGP signature
