guile-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-25-g99


From: Andy Wingo
Subject: Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-25-g990b11c
Date: Fri, 11 Jan 2013 21:28:44 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux)

Hi!

On Fri 11 Jan 2013 18:15, address@hidden (Ludovic Courtès) writes:

> "Andy Wingo" <address@hidden> skribis:
>
>> address@hidden string->bytevector string encoding 
>> [#:conversion-strategy='error]
>
> An optional instead of keyword argument would look nicer, IMO.

Nicer in the docs or nicer to use?  I'll change it anyway.

Just as a meta-level thing, I think we should prefer keywords over
optional arguments unless we can convince ourselves that there won't be
any other options.  If you end up having two independent optional
arguments, you'd have been better off going with keywords from the
beginning.

In this case probably there won't be another optional argument, but I
couldn't mentally rule it out, so I went with the keyword.  Anyway,
doesn't matter much :)

>> +Encode @var{string} as a sequence of bytes.
>> +
>> +The string will be encoded in the character set specified by the
>> address@hidden string.  If the string has characters that cannot be
>> +represented in the encoding, by default this procedure raises an
>> address@hidden,
>
> I think this doesn’t leave a way to know which character in STRING could
> not be converted.  It would be ideal if that information could be
> provided as part of the exception, as is the case with ports (tested
> with ‘test-decoding-error’ in ports.test.)

You sure?  It's implemented using ports in the general case.  And what
about the case for bytevector->string?

>> address@hidden bytevector->string bytevector encoding 
>
> #:conversion-strategy is missing here.

Indeed, fixed.

>> +(define-module (ice-9 iconv)
>> +  #:use-module (rnrs bytevectors)
>> +  #:use-module (ice-9 binary-ports)
>> +  #:use-module ((ice-9 rdelim) #:select (read-delimited))
>> +  #:export (string->bytevector
>> +            bytevector->string
>> +            call-with-encoded-output-string))
>
> Not a single docstring.  Now I feel ashamed when asking Nala to add
> docstrings.  ;-)

There are only three functions!  You make it sound like I'm in the back
room strangling kittens :P  Anyway, fixed.

Incidentally, it would be nice to start using texinfo in docstrings, and
use (texinfo serialize) to render them.

Andy
-- 
http://wingolog.org/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]