This is ok nicm
I wouldn't be surprised if WIDECHAR build of libedit has other issues,
getting it going is still on my list of things to do :-).
On Mon, Apr 04, 2011 at 08:30:42PM +0200, Stefan Sperling wrote:
> On Sun, Apr 03, 2011 at 11:26:52PM +0100, Nicholas Marriott wrote:
> > Hi
> >
> > We don't currently build a wide char libedit but comments inline:
> >
> > On Sun, Apr 03, 2011 at 11:46:24PM +0200, Stefan Sperling wrote:
> > > These callers in libedit might get confused if wcwidth() returns -1.
> > >
> > > Note how the result ct_visual_width() is used in refresh.c:
> > >
> > > refresh.c: h += ct_visual_width(*cp);
> > >
> > >
> > > Index: chartype.c
> > > ===================================================================
> > > RCS file: /cvs/src/lib/libedit/chartype.c,v
> > > retrieving revision 1.1
> > > diff -u -p -r1.1 chartype.c
> > > --- chartype.c 30 Jun 2010 00:05:35 -0000 1.1
> > > +++ chartype.c 3 Apr 2011 21:37:10 -0000
> > > @@ -258,6 +258,9 @@ protected int
> > > ct_visual_width(Char c)
> > > {
> > > int t = ct_chr_class(c);
> > > +#ifdef WIDECHAR
> > > + int w;
> > > +#endif
> > > switch (t) {
> > > case CHTYPE_ASCIICTL:
> > > return 2; /* ^@ ^? etc. */
> > > @@ -267,7 +270,8 @@ ct_visual_width(Char c)
> > > return 0; /* Should this be 1 instead? */
> > > #ifdef WIDECHAR
> > > case CHTYPE_PRINT:
> > > - return wcwidth(c);
> > > + w = wcwidth(c);
> > > + return (w == -1 ? 0 : w);
> >
> > Yes makes sense, ok nicm.
> >
> > > case CHTYPE_NONPRINT:
> > > if (c > 0xffff) /* prefer standard 4-byte display over 5-byte */
> > > return 8; /* \U+12345 */
> > > Index: refresh.c
> > > ===================================================================
> > > RCS file: /cvs/src/lib/libedit/refresh.c,v
> > > retrieving revision 1.11
> > > diff -u -p -r1.11 refresh.c
> > > --- refresh.c 30 Jun 2010 00:05:35 -0000 1.11
> > > +++ refresh.c 3 Apr 2011 21:39:42 -0000
> > > @@ -160,6 +160,9 @@ re_putc(EditLine *el, Int c, int shift)
> > > int i, w = Width(c);
> > > ELRE_DEBUG(1, (__F, "printing %5x '%c'\r\n", c, c));
> > >
> > > + if (w == -1)
> > > + w = 0;
> >
> > Would it be better maybe to do the check in the Width() macro itself?
>
> Yes indeed. New diff below.
>
> I've also tested the WILDCHAR build now.
> It has some problems unrelated to this diff.
>
> Index: chartype.c
> ===================================================================
> RCS file: /cvs/src/lib/libedit/chartype.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 chartype.c
> --- chartype.c 30 Jun 2010 00:05:35 -0000 1.1
> +++ chartype.c 3 Apr 2011 21:37:10 -0000
> @@ -258,6 +258,9 @@ protected int
> ct_visual_width(Char c)
> {
> int t = ct_chr_class(c);
> +#ifdef WIDECHAR
> + int w;
> +#endif
> switch (t) {
> case CHTYPE_ASCIICTL:
> return 2; /* ^@ ^? etc. */
> @@ -267,7 +270,8 @@ ct_visual_width(Char c)
> return 0; /* Should this be 1 instead? */
> #ifdef WIDECHAR
> case CHTYPE_PRINT:
> - return wcwidth(c);
> + w = wcwidth(c);
> + return (w == -1 ? 0 : w);
> case CHTYPE_NONPRINT:
> if (c > 0xffff) /* prefer standard 4-byte display over 5-byte */
> return 8; /* \U+12345 */
> Index: chartype.h
> ===================================================================
> RCS file: /cvs/src/lib/libedit/chartype.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 chartype.h
> --- chartype.h 30 Jun 2010 00:05:35 -0000 1.1
> +++ chartype.h 4 Apr 2011 18:23:52 -0000
> @@ -106,7 +106,7 @@
>
> #define Strtol(p,e,b) wcstol(p,e,b)
>
> -#define Width(c) wcwidth(c)
> +#define Width(c) (wcwidth(c) == -1 ? 0 : wcwidth(c))
>
> #else /* NARROW */
