Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-18-g904a78f

[Ludovic Courtès]

"Michael Gran" <address@hidden> writes:

> commit 904a78f11d2d11a58d5df365a44c4fbbd4c96df3
> Author: Michael Gran <address@hidden>
> Date:   Wed Jul 29 06:38:32 2009 -0700
>
>     Add 32-bit characters
>     
>     This adds the 32-bit standalone characters.  Strings are still
>     8-bit.  Characters larger than 8-bit can only be entered or
>     displayed in octal format at this point.  At this point, the
>     terminal's display encoding is expected to be Latin-1.

It looks like Unicode is approaching, good news!  :-)

My remark about user-visibility was actually regarding this commit, not
the previous one.

> +#ifndef SCM_WCHAR_DEFINED
> +typedef scm_t_int32 scm_t_wchar;
> +#define SCM_WCHAR_DEFINED
> +#endif

Why is this #ifdef hack needed?

> +#define SCM_CHAR(x) ((scm_t_wchar)SCM_ITAG8_DATA(x))

Please, use GCS style.

> +#define SCM_MAKE_CHAR(x) ({scm_t_int32 _x = (x);                        \
> +      _x < 0                                                            \
> +        ? SCM_MAKE_ITAG8((scm_t_bits)(unsigned char)_x, scm_tc8_char)   \
> +        : SCM_MAKE_ITAG8((scm_t_bits)_x, scm_tc8_char);})

This macro uses a GCC extension, which is not acceptable for Guile.  Can
you please rewrite it in standard C?  (The only risk is multiple
expansion of X, but that's OK.)

Does X < 0 mean ASCII?  And why is it truncated to 8 bits?  A comment
just above indicating the encoding trick would be handy IMO.

(And style isn't OK.)

> +#define SCM_CODEPOINT_MAX (0x10ffff)
> +#define SCM_IS_UNICODE_CHAR(c)                                          \
> +  ((scm_t_wchar)(c)<=0xd7ff ||                                          \
> +   ((scm_t_wchar)(c)>=0xe000 && (scm_t_wchar)(c)<=SCM_CODEPOINT_MAX))

Style.

> +                  if (i<256)
> +                    {
> +                      /* Character is graphic.  Print it.  */
> +                      scm_putc (i, port);
> +                    }

Style (extraneous braces).

> +VM_DEFINE_INSTRUCTION (42, make_char32, "make-char32", 4, 0, 1)
> +{
> +  scm_t_wchar v = 0;
> +  v += FETCH ();
> +  v <<= 8; v += FETCH ();
> +  v <<= 8; v += FETCH ();
> +  v <<= 8; v += FETCH ();
> +  PUSH (SCM_MAKE_CHAR (v));
> +  NEXT;
> +}

The doc will need to be augmented.

> +     ((char? x)
> +         (cond ((<= (char->integer x) #xff)
> +                `(make-char8 ,(char->integer x)))
> +               (else
> +                `(make-char32 ,(char->integer x)))))

Sounds cool!  :-)

Thanks,
Ludo'.