‘http-get*’ and all that

‘http-get*’ and all that

[Ludovic Courtès]

Hello!

"Andy Wingo" <wingo@pobox.com> skribis:

>       (http-get): Redefine in terms of http-get.  Deprecate the
>       #:extra-headers argument in favor of #:headers.  Allow a body.  Add a
>       #:streaming? argument, subsuming the functionality of http-get*.
>       (http-get*): Deprecate.

‘http-get*’ was added in 2.0.7, so it doesn’t seem wise to deprecate it
just a couple of months later, no?

As for adding another keyword instead of another procedure, that’s fine,
but not strikingly more elegant either, IMO.

In terms of process, I’d prefer more discussion.  For instance, while we
briefly discussed (ice-9 iconv) on IRC, posting an RFC here, or at least
a note, would have helped give the feeling that people are in the loop,
and may have turned up useful feedback.

WDYT?

(That being said, I’m more than happy to see you back on the good hack. :-))

Thanks,
Ludo’.

Re: ‘http-get*’ and all that

[Andy Wingo]

Hi :)

On Fri 11 Jan 2013 17:53, ludo@gnu.org (Ludovic Courtès) writes:

> "Andy Wingo" <wingo@pobox.com> skribis:
>
>>       (http-get): Redefine in terms of http-get.  Deprecate the
>>       #:extra-headers argument in favor of #:headers.  Allow a body.  Add a
>>       #:streaming? argument, subsuming the functionality of http-get*.
>>       (http-get*): Deprecate.
>
> ‘http-get*’ was added in 2.0.7, so it doesn’t seem wise to deprecate it
> just a couple of months later, no?

In many ways it's better to deprecate early while there are few users,
and the change was recent.  It's not like the interfaces are actually
going away for a while.

> As for adding another keyword instead of another procedure, that’s fine,
> but not strikingly more elegant either, IMO.

We would have had to add 3 or 4 additional procedures: http-put*,
http-post*, etc.  It was actually less work to add #:streaming? to
http-get, and at that point http-get* is superfluous.

When making these choices, I used Python's "requests" module as a guide:
http://docs.python-requests.org/en/latest/  It's actually quite nice to
use.

> In terms of process, I’d prefer more discussion.  For instance, while we
> briefly discussed (ice-9 iconv) on IRC, posting an RFC here, or at least
> a note, would have helped give the feeling that people are in the loop,
> and may have turned up useful feedback.

Sure, no problem.  I had assumed that the old guile-user discussion was
sufficient (http://thread.gmane.org/gmane.lisp.guile.user/9373), but I
can't blame you for forgetting about a thread last April ;)

Any other points on the code?

Regards,

Andy
-- 
http://wingolog.org/

Re: ‘http-get*’ and all that

[Ludovic Courtès]

Howdy!

Andy Wingo <wingo@pobox.com> skribis:

> On Fri 11 Jan 2013 17:53, ludo@gnu.org (Ludovic Courtès) writes:
>
>> "Andy Wingo" <wingo@pobox.com> skribis:
>>
>>>       (http-get): Redefine in terms of http-get.  Deprecate the
>>>       #:extra-headers argument in favor of #:headers.  Allow a body.  Add a
>>>       #:streaming? argument, subsuming the functionality of http-get*.
>>>       (http-get*): Deprecate.
>>
>> ‘http-get*’ was added in 2.0.7, so it doesn’t seem wise to deprecate it
>> just a couple of months later, no?
>
> In many ways it's better to deprecate early while there are few users,
> and the change was recent.  It's not like the interfaces are actually
> going away for a while.

Right.  It still gives a bad impression, I think, but it’s technically
manageable.

>> As for adding another keyword instead of another procedure, that’s fine,
>> but not strikingly more elegant either, IMO.
>
> We would have had to add 3 or 4 additional procedures: http-put*,
> http-post*, etc.  It was actually less work to add #:streaming? to
> http-get, and at that point http-get* is superfluous.

OK, I see.  Uniformity among http-{post,put,get} is probably a good idea.

> When making these choices, I used Python's "requests" module as a guide:
> http://docs.python-requests.org/en/latest/  It's actually quite nice to
> use.

Interesting (I admit this is not part of my culture ;-)).

>> In terms of process, I’d prefer more discussion.  For instance, while we
>> briefly discussed (ice-9 iconv) on IRC, posting an RFC here, or at least
>> a note, would have helped give the feeling that people are in the loop,
>> and may have turned up useful feedback.
>
> Sure, no problem.  I had assumed that the old guile-user discussion was
> sufficient (http://thread.gmane.org/gmane.lisp.guile.user/9373), but I
> can't blame you for forgetting about a thread last April ;)

Oh, I confess I had forgotten about that one, sorry.

> Any other points on the code?

No!  :-)

Ludo’.

Re: ‘http-get*’ and all that

[Daniel Hartwig]

On 12 January 2013 02:49, Andy Wingo <wingo@pobox.com> wrote:
>> ‘http-get*’ was added in 2.0.7, so it doesn’t seem wise to deprecate it
>> just a couple of months later, no?
>
> In many ways it's better to deprecate early while there are few users,
> and the change was recent.  It's not like the interfaces are actually
> going away for a while.

Quite right.

Looks like the copy-pasta left an inappropriate “(when extra-headers
…)” around the deprecation warning in http-get*.

Re: ‘http-get*’ and all that

[Andy Wingo]

On Sat 12 Jan 2013 05:09, Daniel Hartwig <mandyke@gmail.com> writes:

> Looks like the copy-pasta left an inappropriate “(when extra-headers
> …)” around the deprecation warning in http-get*.

Fixed, thanks.

Andy
-- 
http://wingolog.org/

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

[Ludovic Courtès]

"Andy Wingo" <wingo@pobox.com> skribis:

> commit f05bb8494c9636cd7a44aaf7d4e08f4b66004b6e
> Author: Andy Wingo <wingo@pobox.com>
> Date:   Thu Jan 10 22:50:27 2013 +0100
>
>     add bytevector->string and string->bytevector in new (ice-9 iconv) module
>     
>     * module/Makefile.am:
>     * module/ice-9/iconv.scm: New module implementing procedures to encode
>       and decode representations of strings as bytes.
>     
>     * test-suite/Makefile.am:
>     * test-suite/tests/iconv.test: Add tests.
>     
>     * doc/ref/api-data.texi: Add docs.

Nice, this was long overdue, I guess.  :-)

> +@node Representing Strings as Bytes
> +@subsubsection Representing Strings as Bytes

[...]

> +@deffn string->bytevector string encoding [#:conversion-strategy='error]

An optional instead of keyword argument would look nicer, IMO.

> +Encode @var{string} as a sequence of bytes.
> +
> +The string will be encoded in the character set specified by the
> +@var{encoding} string.  If the string has characters that cannot be
> +represented in the encoding, by default this procedure raises an
> +@code{encoding-error},

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.)

> +@deffn bytevector->string bytevector encoding 

#:conversion-strategy is missing here.

> +The Scheme interface to this function is @code{encode-string}, from the

string->bytevector

> +The Scheme interface to this function is @code{decode-string}.

bytevector->string

> +(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.  ;-)

Thanks!

Ludo’.

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

[Andy Wingo]

Hi!

On Fri 11 Jan 2013 18:15, ludo@gnu.org (Ludovic Courtès) writes:

> "Andy Wingo" <wingo@pobox.com> skribis:
>
>> +@deffn 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
>> +@var{encoding} string.  If the string has characters that cannot be
>> +represented in the encoding, by default this procedure raises an
>> +@code{encoding-error},
>
> 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?

>> +@deffn 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/

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

[Ludovic Courtès]

Hi!

Andy Wingo <wingo@pobox.com> skribis:

> On Fri 11 Jan 2013 18:15, ludo@gnu.org (Ludovic Courtès) writes:
>
>> "Andy Wingo" <wingo@pobox.com> skribis:
>>
>>> +@deffn 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?

Both, IMO.

> 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.

Agreed.

> 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 :)

That was my reasoning: there won’t be any other option here.

>>> +Encode @var{string} as a sequence of bytes.
>>> +
>>> +The string will be encoded in the character set specified by the
>>> +@var{encoding} string.  If the string has characters that cannot be
>>> +represented in the encoding, by default this procedure raises an
>>> +@code{encoding-error},
>>
>> 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?

Likewise.  When using iconv(3) in C, it’s possible to know at which
position the input failed to be converted.

>> 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.

You make it sound like I’m the Brigitte Bardot of docstrings!  :-)

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

That’d be ideal, but I wonder whether there are tools around that would
end up displaying raw Texinfo, such as anything that use
‘procedure-documentation’.  Perhaps that can be solved easily.

Thanks,
Ludo’.

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

[Mark H Weaver]

ludo@gnu.org (Ludovic Courtès) writes:
> Andy Wingo <wingo@pobox.com> skribis:
>> Incidentally, it would be nice to start using texinfo in docstrings, and
>> use (texinfo serialize) to render them.
>
> That’d be ideal, but I wonder whether there are tools around that would
> end up displaying raw Texinfo, such as anything that use
> ‘procedure-documentation’.  Perhaps that can be solved easily.

FWIW, I agree with Andy that it would be good to support texinfo markup
in docstrings.  At present, the docstrings for procedures implemented in
C are much nicer looking than the Scheme ones.  This does not bode well
for future docstrings as we move more code from C to Scheme.

I've been sad to see the recent patches that strip texinfo markup from
docstrings.  That's going in the wrong direction, IMO.  Instead, we
should be bringing Scheme docstrings up to par with their C
counterparts.

Regarding 'procedure-documentation', I think it should return the
rendered version, just like it does for procedures defined in C.

What do you think?

      Mark

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

[Ludovic Courtès]

Hi!

Mark H Weaver <mhw@netris.org> skribis:

> FWIW, I agree with Andy that it would be good to support texinfo markup
> in docstrings.  At present, the docstrings for procedures implemented in
> C are much nicer looking than the Scheme ones.

Hmm, they just look the same since they are already rendered.

> I've been sad to see the recent patches that strip texinfo markup from
> docstrings.  That's going in the wrong direction, IMO.

That’s going in the direction of having docstrings usable /now/, where
no rendering occurs.

> Regarding 'procedure-documentation', I think it should return the
> rendered version, just like it does for procedures defined in C.

Yes, probably.

One problem to keep in mind is that (texinfo ...) is only able to render
a small subset of Texinfo.  That’s probably OK for docstrings, though.

Ludo’.