From owner-freebsd-current@freebsd.org Wed Nov 25 12:53:31 2015 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 48A5CA37BB4 for ; Wed, 25 Nov 2015 12:53:31 +0000 (UTC) (envelope-from baptiste.daroussin@gmail.com) Received: from mail-wm0-x236.google.com (mail-wm0-x236.google.com [IPv6:2a00:1450:400c:c09::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id CBA3314E4; Wed, 25 Nov 2015 12:53:30 +0000 (UTC) (envelope-from baptiste.daroussin@gmail.com) Received: by wmec201 with SMTP id c201so254954331wme.0; Wed, 25 Nov 2015 04:53:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=+wv9cjUitLZELur9lX8yiTU0N1V5ORiojiedGJ3HwNQ=; b=d/1cKB90IPuA0Gg+FwX152ypNCYYVFh+z2Ozf0bbG2SbKIrFpAU5azKJvvSOKtvT8f aeS/11aJMOyA1BsXcerNG07cDkf1h6bKwl1qFFxwcGtET2usx+dK9LbpzaevXHyhnnIP SjATclnOwUqt1c+/aJsdNLyXFC4O2DZ8dqLOgbnwxebtf9d2C3TNNCUTOZJNOlCcm0HA LKx9on5WPyMA5pJd11aFVygIFWtKa/+uz1hvPmaCOAyDkp+9XnEJ895fiyonrD1uG7mY YLtM9oNi2V3JTdfqmrNfwVzV8Wlm78iu+mki6YJ/m6WNETqwplgghnIBHcrOhO9A5qaq DJfQ== X-Received: by 10.194.93.6 with SMTP id cq6mr43249038wjb.165.1448456008869; Wed, 25 Nov 2015 04:53:28 -0800 (PST) Received: from ivaldir.etoilebsd.net ([2001:41d0:8:db4c::1]) by smtp.gmail.com with ESMTPSA id n127sm3392339wmf.12.2015.11.25.04.53.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Nov 2015 04:53:27 -0800 (PST) Sender: Baptiste Daroussin Date: Wed, 25 Nov 2015 13:53:25 +0100 From: Baptiste Daroussin To: Andrey Chernov Cc: Ed Schouten , Jilles Tjoelker , "Sergey V. Dyatko" , FreeBSD Current Subject: Re: /bin/ls formatting broken for non-C(?) locales Message-ID: <20151125125325.GB77370@ivaldir.etoilebsd.net> References: <20151120110556.6e20a71f@laptop.minsk.domain> <20151120104253.GA21071@ivaldir.etoilebsd.net> <20151120110212.GB21071@ivaldir.etoilebsd.net> <20151120122352.GA5751@stack.nl> <20151121003541.GF21071@ivaldir.etoilebsd.net> <5650DACA.2090501@freebsd.org> <20151125001513.GC70014@ivaldir.etoilebsd.net> <56550F69.8050609@freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="QKdGvSO+nmPlgiQ/" Content-Disposition: inline In-Reply-To: <56550F69.8050609@freebsd.org> User-Agent: Mutt/1.5.24 (2015-08-30) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Nov 2015 12:53:31 -0000 --QKdGvSO+nmPlgiQ/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 25, 2015 at 04:31:21AM +0300, Andrey Chernov wrote: > On 25.11.2015 3:15, Baptiste Daroussin wrote: > > On Sat, Nov 21, 2015 at 11:57:46PM +0300, Andrey Chernov wrote: > >> On 21.11.2015 15:18, Ed Schouten wrote: > >>> Hi Baptiste, > >>> > >>> I suppose you should use the wcswidth() function somewhere to compute > >>> the visible width of the month name. Some characters may be > >>> double-width, others may have no effective width at all. > >>> > >> > >> I agree. Checking error return of wide chars functions with some > >> fallback will be good too. > >=20 > > I have updated the code https://reviews.freebsd.org/D4239 > >=20 > > Tested by modifying some locales to add double width and zero width uni= code in > > the locales > >=20 > > Also added the error checking for the return of wide chars functions. F= or now I > > haven't added fallback, suggestions welcome if needed. >=20 > 1) For just 1 char in wcswidth(&wab_months[i][j], 1); it is better to > use another function wcwidth(wab_months[i][j]); done >=20 > 2) By fallback I mean something which not stops ls working with > incorrect for some reason locale, like setting max_width_month to > MAX_ABMON_WIDTH on error return (from > mbstowcs/wcwidth/wcswidth/wcswidth) and exit from > populate_abbreviated_month(). Not that easy to provide a fallback (or better to say I can't find a proper= way) it if fails on mbstocws then ab_months is not populated so unusable. What I did for now is set max_month_width to -1 and in ls_strftime I fallba= ck on the plain strftime meaning you keep localized information but the alignemen= t is broken as of now. >=20 > 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 >=20 > 4) The whole processing looks overcomplicated and not effective. What > about this instead? > for (i =3D 0; i < 12; i++) { > count wcswidth() of each month and store it in wab_months_width[]. > count max_width_month. > } > for (i =3D 0; i < 12; i++) { > if ((n =3D max_width_month - wab_months_width[i]) > 0) > call wcscat(wab_months[i], L" ") n times. > } Done, along with your optimisation from your next mail >=20 > 5) If there is no %b is strftime() format, there is no sense to spend > CPU cycles on from populate_abbreviated_month(), so it should be called > only once inside ls_strftime() on first %b instead of calling it in > printtime() for all cases. done Review updated (if you prefer a diff by mail just tell me, given you do not= have a phabricator account.) Thanks for your patience! and reviews! Bapt --QKdGvSO+nmPlgiQ/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlZVr0UACgkQ8kTtMUmk6ExkGwCdEPI4qHzdEI8InCdy9zqBuw/T 0TAAn1f47EYsZ2MmngPDaPqdMN/5pqgI =bZP1 -----END PGP SIGNATURE----- --QKdGvSO+nmPlgiQ/--