Re: [PATCH 2/4] Add implementation of "transcoded ports"

[Ludovic Courtès]

Hi!

Andreas Rottmann <address@hidden> writes:

> * libguile/r6rs-ports.c (make_tp, tp_write, tp_fill_input, tp_flush,
>   tp_close, initialize_transcoded_ports, scm_transcoded_port): New
>   functions.
>   (scm_init_r6rs_ports): Call `initialize_transcoded_ports'.
> * module/rnrs/ports.scm (transcoded-port): Remove, this is now
>   implemented in C.
> * test-suite/tests/r6rs-ports.test (8.2.6 Input and output ports): Added a
>   few tests for `transcoded-port'.

Great!  This looks good to me, modulo the minor things below:

> +  /* We can't use scm_c_read() here, since it blocks until the whole
> +     block has been read or EOF */

Please write it “`scm_c_read'” and add a period at the end.

> +SCM_DEFINE (scm_transcoded_port,
> +         "transcoded-port", 2, 0, 0,
> +         (SCM port, SCM transcoder),
> +         "")

Docstring please.  :-)

> +  SCM_VALIDATE_STRUCT (SCM_ARG1, transcoder);

This type check is too weak.

> +  /* SCM_CLR_PORT_OPEN_FLAG (port); */

Meaning of this comment?

> +(with-test-prefix "8.2.6  Input and output ports"
> +  (pass-if "transcoded-port [output]"
> +    (let ((s "Hello\n\304\326\334"))

It seems that it’s not actual UTF-8, or maybe the message mangled it
somehow?

> +           (call-with-port (transcoded-port bv-port (make-transcoder 
> (utf-8-codec)))

I think you forgot the patch that adds ‘make-transcoder’ and
‘utf-8-codec’.  :-)

Can you send an updated patch (or pair of patches)?

Thanks,
Ludo’.