Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Borzenkov Andrey]

Пн, 21.02.2005 17:30:25 you wrote:
>> Bart Schaefer wrote:
>> I tried defining ZLE_UNICODE_SUPPORT and building on RHEL3 (really CentOS,
>> but that shouldn't matter)

> Generally speaking, I'm not that suprised, since there are large numbers
> of odds and ends still to clear up (I haven't tried turning it on yet).
> 
> > and the build died on a syntax error in zle.h
> > (apparently the "int_t" typedef does not exist?).
> 
> That should have been wint_t.  That's guaranteed to hold either a
> wchar_t or a WEOF.
> 
> Clint and I have added various type and functions like this without there
> being any tests in configure: this is one reason we're currently relying
> on the compiler offering support. 

BTW wctype.h was missing

 If we want to relax that assumption
> later, we'll probably have to work out what to do if wchar_t is defined
> and wint_t isn't, etc.  I suppose the answer in this case is probably
> just stick with wchar_t.  But that's all Zukunftsmusik.
>

I played with this patch over weekend (with fixed wint_t and couple of other obvious mismatches). So far I was not able to make zsh work with ZLE_UNICODE_SUPPORT.

The problem seems to be that currently zle returns to zsh array of wchar_t; and zsh of course has no idea what to do with it (and treats it as array of char anyway). I started to attempt to fix it but then I realized I do not know (and there is nothing in patch) what was intended:

- convert the whole zsh to operate on wchar_t instead of char
- convert wchar_t to something else (mbchar back? UTF-8?) before passing it to main zsh code.

I would rather prefer the latter and make it UTF-8. This retains maximum compatibility, allows keep string processing functions and allows definition of character constants (zsh is full of them). It is not possible to portably define constant with wchar_t because run-time locale is not neccessarily the same as compile-time locale (consider ASCII vs EBCDIC).

BTW I finally switched to UTF-8 and stock zsh (without ZLE_UNICODE_SUPPORT enabled) displayes garbage instead of Russian file names and crashes when I attempt to complete them. Is it expected? IIRC there were some patches to allow at least passing multibyte through.

Regards

-andrey

PS Would somebody kindly look into the issue with mail.ru addresses? I can subscribe from this address and get list messages but apparently nothing I post using "arvidjaar (at) mail dot ru" ever gets posted. Thank you.
__________
www.newmail.ru -- бесплатная почта, бесплатный хостинг.

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Peter Stephenson]

Borzenkov Andrey wrote:
> The problem seems to be that currently zle returns to zsh array of wchar_t; a
> nd zsh of course has no idea what to do with it (and treats it as array of ch
> ar anyway). I started to attempt to fix it but then I realized I do not know 
> (and there is nothing in patch) what was intended:
> 
> - convert wchar_t to something else (mbchar back? UTF-8?) before passing it t
> o main zsh code.

This should happen already.  zleread() now calls zlegetline() (instead
of just returning the line it had already assembled) which calls
zlelineasstring() in zle_utils.c.  For ZLE_UNICODE_SUPPORT this should
convert the wchar_t array to a metafied multibyte string as used by the
rest of the shell.

I haven't tested the combined effect of the patches so far at all, since
I know there are bits that aren't fixed up yet.  The trouble is it
really has to be all or nothing inside zle itself; a mix of bits that
use multibyte strings and bits that use wide characters won't work at
all.  I'd be satisfied to being with to get an interface that looks like
it ought to work.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

On Monday 21 February 2005 17:29, Peter Stephenson wrote:
> Borzenkov Andrey wrote:
> > The problem seems to be that currently zle returns to zsh array of
> > wchar_t; a nd zsh of course has no idea what to do with it (and treats it
> > as array of ch ar anyway). I started to attempt to fix it but then I
> > realized I do not know (and there is nothing in patch) what was intended:
> >
> > - convert wchar_t to something else (mbchar back? UTF-8?) before passing
> > it t o main zsh code.
>
> This should happen already.  zleread() now calls zlegetline() (instead
> of just returning the line it had already assembled) which calls
> zlelineasstring() in zle_utils.c.  For ZLE_UNICODE_SUPPORT this should
> convert the wchar_t array to a metafied multibyte string as used by the
> rest of the shell.
>

right, missed this.

> I haven't tested the combined effect of the patches so far at all, since
> I know there are bits that aren't fixed up yet.

attached patch makes it so far as being able to start zsh -f and really input 
something. The relevant parts are actually selfinsert (wchar_t != wint_t) and 
zlelineasstring (length was computed incorrectly and final NULL metafied). 
Everything else is just part of yesterday's futile attempts :)

Oh, and for some reasons mbrtowc(&outchar, &cnull, 1, &ps) segfaulted on me 
(Mandrake cooker).

I can't commit it as for some reason ssh access to CVS hangs. 

-andrey

[-- Attachment #2: unicode.diff --]
[-- Type: text/x-diff, Size: 5379 bytes --]

Index: Src/system.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/system.h,v
retrieving revision 1.28
diff -u -p -r1.28 system.h
--- Src/system.h	18 Feb 2005 13:57:27 -0000	1.28
+++ Src/system.h	21 Feb 2005 19:08:51 -0000
@@ -689,6 +689,7 @@ extern short ospeed;
  */
 #if defined(HAVE_WCHAR_H) && defined(HAVE_WCTOMB) && defined (__STDC_ISO_10646__)
 # include <wchar.h>
+# include <wctype.h>
 
 /*
  * More stringent requirements to enable complete Unicode conversion
Index: Src/Zle/zle.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle.h,v
retrieving revision 1.8
diff -u -p -r1.8 zle.h
--- Src/Zle/zle.h	21 Feb 2005 11:20:24 -0000	1.8
+++ Src/Zle/zle.h	21 Feb 2005 19:08:52 -0000
@@ -47,6 +47,7 @@ typedef wint_t   ZLE_INT_T;
 #define ZLENL	L'\n'
 #define ZLENUL	L'\0'
 #define ZLETAB	L'\t'
+#define ZLESPC	L' '
 
 #define DIGIT_1		L'1'
 #define DIGIT_9		L'9'
@@ -75,6 +76,7 @@ typedef int ZLE_INT_T;
 #define ZLENL	'\n'
 #define ZLENUL	'\0'
 #define ZLETAB	'\t'
+#define ZLESPC	' '
 
 #define DIGIT_1		'1'
 #define DIGIT_9		'9'
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.59
diff -u -p -r1.59 zle_main.c
--- Src/Zle/zle_main.c	18 Feb 2005 13:57:28 -0000	1.59
+++ Src/Zle/zle_main.c	21 Feb 2005 19:08:53 -0000
@@ -752,6 +752,7 @@ getrestchar(int inchar)
     char buf[MB_CUR_MAX], *ptr;
     wchar_t outchar;
     int ret;
+    mbstate_t ps;
 
     /*
      * We are guaranteed to set a valid wide last character,
@@ -764,7 +765,8 @@ getrestchar(int inchar)
 	return lastchar_wide = WEOF;
 
     /* reset shift state by converting null */
-    mbrtowc(&outchar, &cnull, 1, &ps);
+    //mbrtowc(&outchar, &cnull, 1, &ps);
+    memset (&ps, '\0', sizeof (ps));
 
     ptr = buf;
     *ptr++ = inchar;
@@ -785,7 +787,7 @@ getrestchar(int inchar)
 	    return lastchar_wide = WEOF;
 	*ptr++ = inchar;
     }
-    return lastchar_wide = (wint_t)outchar;
+    return lastchar_wide = (ZLE_INT_T)outchar;
 }
 /**/
 #endif
Index: Src/Zle/zle_misc.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_misc.c,v
retrieving revision 1.16
diff -u -p -r1.16 zle_misc.c
--- Src/Zle/zle_misc.c	18 Feb 2005 17:31:11 -0000	1.16
+++ Src/Zle/zle_misc.c	21 Feb 2005 19:08:53 -0000
@@ -61,9 +61,13 @@ mod_export int
 selfinsert(UNUSED(char **args))
 {
 #ifdef ZLE_UNICODE_SUPPORT
+    /* wint_t and wchar_t not neccessarily the same size */
+    wchar_t tmp;
+
     if (!lastchar_wide_valid)
 	getrestchar(lastchar);
-    doinsert(&lastchar_wide, 1);
+    tmp = lastchar_wide;
+    doinsert(&tmp, 1);
 #else
     char s = lastchar;
     doinsert(&s, 1);
@@ -921,7 +925,7 @@ executenamedcommand(char *prmt)
 #ifdef ZLE_UNICODE_SUPPORT
 		    if (!lastchar_wide_valid)
 			getrestchar(0);
-		    if (iswcntrl(lastchar))
+		    if (iswcntrl(lastchar_wide))
 #else
 		    if (icntrl(lastchar))
 #endif
Index: Src/Zle/zle_tricky.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_tricky.c,v
retrieving revision 1.49
diff -u -p -r1.49 zle_tricky.c
--- Src/Zle/zle_tricky.c	18 Feb 2005 17:31:11 -0000	1.49
+++ Src/Zle/zle_tricky.c	21 Feb 2005 19:08:55 -0000
@@ -159,12 +159,12 @@ int hascompwidgets;
 static int
 usetab(void)
 {
-    unsigned char *s = zleline + zlecs - 1;
+    ZLE_STRING_T s = zleline + zlecs - 1;
 
     if (keybuf[0] != '\t' || keybuf[1])
 	return 0;
-    for (; s >= zleline && *s != '\n'; s--)
-	if (*s != '\t' && *s != ' ')
+    for (; s >= zleline && *s != ZLENL; s--)
+	if (*s != ZLETAB && *s != ZLESPC)
 	    return 0;
     if (compfunc) {
 	wouldinstab = 1;
@@ -866,7 +866,7 @@ addx(char **ptmp)
 	(instring && (zleline[zlecs] == '"' || zleline[zlecs] == '\'')) ||
 	(addspace = (comppref && !iblank(zleline[zlecs])))) {
 	*ptmp = (char *)zleline;
-	zleline = (unsigned char *)zhalloc(strlen((char *)zleline) + 3 +
+	zleline = (ZLE_STRING_T)zhalloc(strlen((char *)zleline) + 3 +
 					   addspace);
 	memcpy(zleline, *ptmp, zlecs);
 	zleline[zlecs] = 'x';
Index: Src/Zle/zle_utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_utils.c,v
retrieving revision 1.18
diff -u -p -r1.18 zle_utils.c
--- Src/Zle/zle_utils.c	18 Feb 2005 13:57:28 -0000	1.18
+++ Src/Zle/zle_utils.c	21 Feb 2005 19:08:55 -0000
@@ -111,22 +111,22 @@ zlelineasstring(ZLE_STRING_T instr, int 
 {
 #ifdef ZLE_UNICODE_SUPPORT
     char *s;
-    char *mb_cursor;
     int i, j;
     size_t mb_len = 0;
 
-    mb_cursor = s = zalloc(inll * MB_CUR_MAX);
+    s = zalloc(inll * MB_CUR_MAX + 1);
 
-    for(i=0;i<=inll;i++) {
+    for(i=0; i < inll; i++) {
 	if (outcs != NULL && i == incs)
 	    *outcs = mb_len;
-	j = wctomb(mb_cursor, instr[i]);
+	j = wctomb(s + mb_len, instr[i]);
 	if (j == -1) {
 	    /* invalid char; what to do? */
 	} else {
 	    mb_len += j;
 	}
     }
+    s[mb_len] = '\0';
 
     if (outll != NULL)
 	*outll = mb_len;
@@ -135,7 +135,7 @@ zlelineasstring(ZLE_STRING_T instr, int 
 	unsigned char *ret =
 	    (unsigned char *) metafy((char *) s, mb_len, META_HEAPDUP);
 
-	zfree((char *)s, inll * MB_CUR_MAX);
+	zfree((char *)s, inll * MB_CUR_MAX + 1);
 
 	return ret;
     }

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Peter Stephenson]

Andrey Borzenkov wrote:
> attached patch makes it so far as being able to start zsh -f and really input
> something. The relevant parts are actually selfinsert (wchar_t != wint_t) and
> zlelineasstring (length was computed incorrectly and final NULL metafied). 
> Everything else is just part of yesterday's futile attempts :)

Great, thanks.

> Oh, and for some reasons mbrtowc(&outchar, &cnull, 1, &ps) segfaulted on me 
> (Mandrake cooker).

Hmmm... that was the only way I could see to ensure the shift state was
initialised.  The documentation suggested ps was otherwise assumed to be
valid, which it can't be for the first character.  However, I may be
missing the intention.

> I can't commit it as for some reason ssh access to CVS hangs. 

Same for me at the moment, but I'll commit it if I get there first.
If you do, could you change the C++ comment to a C comment (we need
to support original ISO C)?

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Peter Stephenson]

Peter Stephenson wrote:
> Andrey Borzenkov wrote:
> > Oh, and for some reasons mbrtowc(&outchar, &cnull, 1, &ps) segfaulted on me
> > (Mandrake cooker).
> 
> Hmmm... that was the only way I could see to ensure the shift state was
> initialised.  The documentation suggested ps was otherwise assumed to be
> valid, which it can't be for the first character.  However, I may be
> missing the intention.

I looked further.  The glibc documentation says

       An mbstate_t  object  a  can  be
       initialized to the initial state by zeroing it, for example using

              memset(&a, 0, sizeof(a));

but the standard gives no indication this is portable (though clearly
it would be a perfectly acceptable extension).  The standard does refer
obscurely to a "zero-valued mbstate_t" object in the documentation for
mbsinit() (which, maddeningly, tests whether the object is in the
initial state but doesn't set it).  However, that's not the same thing
as saying you can memset() an mbstate_t value to zero --- for example, a
zero-valued floating point number is not necessarily represented by a
set of zero bytes.  Still, maybe that's the best we're going to get?

Alternatively, since we're not going to be using this in multiple
threads, we could use a NULL pointer and rely on the internal state.
However, I would then be worried about what happened after we'd read
an invalid character; we might end up screwing up input permanently.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Oliver Kiddle]

Peter wrote:
>        An mbstate_t  object  a  can  be
>        initialized to the initial state by zeroing it, for example using
> 
>               memset(&a, 0, sizeof(a));
> 
> but the standard gives no indication this is portable (though clearly

I think you can safely take it to be portable. I've seen it many times
before (though the last argument is usually written sizeof(mbstate_t))
and just using grep now shows that quite a good few open source programs
use it.

> Alternatively, since we're not going to be using this in multiple
> threads, we could use a NULL pointer and rely on the internal state.

Surely you might use it in another function call while in the middle of
using it. It's probably safest not to rely on the single internal state.

Oliver

Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Bart Schaefer]

I tried defining ZLE_UNICODE_SUPPORT and building on RHEL3 (really CentOS,
but that shouldn't matter) and the build died on a syntax error in zle.h
(apparently the "int_t" typedef does not exist?).

In file included from ../../Src/Zle/zle.mdh:18,
                 from ../../Src/Zle/complete.mdh:15,
                 from zutil.mdh:15,
                 from zutil.c:30:
../../Src/Zle/zle.h:33: syntax error before "ZLE_INT_T"
../../Src/Zle/zle.h:33: warning: type defaults to `int' in declaration of `ZLE_INT_T'
../../Src/Zle/zle.h:33: warning: data definition has no type or storage class
In file included from ../../Src/Zle/zle.mdh:33,
                 from ../../Src/Zle/complete.mdh:15,
                 from zutil.mdh:15,
                 from zutil.c:30:
../../Src/Zle/zle_main.epro:14: syntax error before "lastchar_wide"
../../Src/Zle/zle_main.epro:14: warning: type defaults to `int' in declaration of `lastchar_wide'
../../Src/Zle/zle_main.epro:14: warning: data definition has no type or storage class
../../Src/Zle/zle_main.epro:38: syntax error before "getfullchar"
../../Src/Zle/zle_main.epro:38: warning: type defaults to `int' in declaration of `getfullchar'
../../Src/Zle/zle_main.epro:38: warning: data definition has no type or storage class
../../Src/Zle/zle_main.epro:40: syntax error before "getrestchar"
../../Src/Zle/zle_main.epro:40: warning: type defaults to `int' in declaration of `getrestchar'
../../Src/Zle/zle_main.epro:40: warning: data definition has no type or storage class
In file included from ../../Src/Zle/zle.mdh:41,
                 from ../../Src/Zle/complete.mdh:15,
                 from zutil.mdh:15,
                 from zutil.c:30:
../../Src/Zle/zle_vi.epro:13: syntax error before "vigetkey"
../../Src/Zle/zle_vi.epro:13: warning: type defaults to `int' in declaration of `vigetkey'
../../Src/Zle/zle_vi.epro:13: warning: data definition has no type or storage class

(P.S. I'm about to be incommunicado for the next week or so.  As in, this
is probably my last message for a while.)

Re: Latest CVS + ZLE_UNICODE_SUPPORT on RHEL3

[Peter Stephenson]

Bart Schaefer wrote:
> I tried defining ZLE_UNICODE_SUPPORT and building on RHEL3 (really CentOS,
> but that shouldn't matter)

Generally speaking, I'm not that suprised, since there are large numbers
of odds and ends still to clear up (I haven't tried turning it on yet).

> and the build died on a syntax error in zle.h
> (apparently the "int_t" typedef does not exist?).

That should have been wint_t.  That's guaranteed to hold either a
wchar_t or a WEOF.

Clint and I have added various type and functions like this without there
being any tests in configure: this is one reason we're currently relying
on the compiler offering support.  If we want to relax that assumption
later, we'll probably have to work out what to do if wchar_t is defined
and wint_t isn't, etc.  I suppose the answer in this case is probably
just stick with wchar_t.  But that's all Zukunftsmusik.


Index: Src/Zle/zle.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle.h,v
retrieving revision 1.7
diff -u -r1.7 zle.h
--- Src/Zle/zle.h	18 Feb 2005 13:57:28 -0000	1.7
+++ Src/Zle/zle.h	21 Feb 2005 10:49:02 -0000
@@ -30,7 +30,7 @@
 #ifdef ZLE_UNICODE_SUPPORT
 typedef wchar_t ZLE_CHAR_T;
 typedef wchar_t *ZLE_STRING_T;
-typedef int_t   ZLE_INT_T;
+typedef wint_t   ZLE_INT_T;
 #define ZLE_CHAR_SIZE	sizeof(wchar_t)
 
 /*

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************