[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/4] Add implementation of "transcoded ports"
From: |
Andreas Rottmann |
Subject: |
Re: [PATCH 2/4] Add implementation of "transcoded ports" |
Date: |
Sun, 21 Nov 2010 23:07:11 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
address@hidden (Ludovic Courtès) writes:
> 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.
>
Done.
>> +SCM_DEFINE (scm_transcoded_port,
>> + "transcoded-port", 2, 0, 0,
>> + (SCM port, SCM transcoder),
>> + "")
>
> Docstring please. :-)
>
Added.
>> + SCM_VALIDATE_STRUCT (SCM_ARG1, transcoder);
>
> This type check is too weak.
>
I've restructured the code now so the type checking is done (implictly)
in Scheme.
>> + /* SCM_CLR_PORT_OPEN_FLAG (port); */
>
> Meaning of this comment?
>
I've replaced it with a (hopefully) more meaningful 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?
>
The file has a header which contains this: "coding: iso-8859-1;". The
strings in the source should hence (IIUC) be in latin-1 encoding -- at
least that's what they are in my git.
>> + (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’. :-)
>
As I split the patch into the series, I obviously got the ordering
wrong; sorry. The "transcoded-port" patch is now the last in the
series.
> Can you send an updated patch (or pair of patches)?
>
I'll send the updated series as follow-up to this mail.
Regards, Rotty
--
Andreas Rottmann -- <http://rotty.yi.org/>
- Some work on the R6RS I/O libraries, Andreas Rottmann, 2010/11/15
- Re: Some work on the R6RS I/O libraries, Ludovic Courtès, 2010/11/19
- [PATCH 2/4] Add implementation of "transcoded ports", Andreas Rottmann, 2010/11/20
- Re: [PATCH 2/4] Add implementation of "transcoded ports", Ludovic Courtès, 2010/11/20
- Re: [PATCH 2/4] Add implementation of "transcoded ports",
Andreas Rottmann <=
- [PATCH 1/4] Turn `(rnrs io ports)' into an R6RS library, Andreas Rottmann, 2010/11/21
- [PATCH 2/4] Reorganize the R6RS I/O condition types, Andreas Rottmann, 2010/11/21
- [PATCH 4/4] Add implementation of "transcoded ports", Andreas Rottmann, 2010/11/21
- Re: [PATCH 4/4] Add implementation of "transcoded ports", Ludovic Courtès, 2010/11/24
- Re: [PATCH 4/4] Add implementation of "transcoded ports", Andreas Rottmann, 2010/11/24
- Re: [PATCH 4/4] Add implementation of "transcoded ports", Ludovic Courtès, 2010/11/25
- [PATCH 3/4] Work towards a more complete implementation of `(rnrs io ports)', Andreas Rottmann, 2010/11/21
- Re: [PATCH 3/4] Work towards a more complete implementation of `(rnrs io ports)', Ludovic Courtès, 2010/11/23
- Re: [PATCH 3/4] Work towards a more complete implementation of `(rnrs io ports)', Andreas Rottmann, 2010/11/23
- Re: [PATCH 3/4] Work towards a more complete implementation of `(rnrs io ports)', Ludovic Courtès, 2010/11/24