Re: multibyte support (round 3)

[Pádraig Brady]

On 19/09/16 07:11, Assaf Gordon wrote:
> Hello,
> 
> Updated patch attached.
> 
> Improvements from last time ( 
> http://lists.gnu.org/archive/html/coreutils/2016-09/msg00011.html ):
> 
> 1. 'multibyte' and 'mbbuffer' are in gl/ , behave more like gnulib modules.
> Tests cover all items mentioned in Markus Kuhn's UTF-8 decoder page
> (https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt).
> 
> 2. cygwin/UTF-16 surrogates are handled transparently in 'mbbuffer'.
> Applications under cygwin see 'ucs4_t' and don't need to worry about 
> surrogates (but, wcwidth() will present some problem). Tests ensure parsing 
> under cygwin behaves like other systems.
> 
> 3. 'cut' supports multibyte '-c' and '-n -b' (but not multibyte '-d' yet).
> Some tests included.

Very nice work, especially on the tests.

A general point is I'm still of the opinion that
it would be better to have all conversion and checking
in unorm(1), thus simplifying/optimizing the checking/processing
in all other utils.  It would be good to get an idea
of performance/overhead as the patches progress.

I'm thinking this could be merged in the next major
version of coreutils, which would come after the
next minor release which hopefully will be released
in the next few weeks.

A couple of points on very quick review:

is_utf8_locale_name()
  In gnulib, "UTF-8" is commented as the only
  variant that needs to be checked from the
  return of locale_charset()

is_valid_mb_character()
  There can invalid characters in many single byte locales:
  For example 81,8d,90,9d,93 in cp1252, as shown by:
  recode -lh cp1252 | grep -C1 -F "     "

mbtowc_utf16()
  The assert() triggers -Werror=suggest-attribute=noreturn
  when not on cygwin, so it's better to avoid compiling
  that function altogether on other platforms.  I.E. put
  #ifdef HAVE_UTF16_SURROGATES around the function definition
  (and declaration to get a compile error rather than a link error)

thanks!
Pádraig