Re: Egg: ws-client

[Vasilij Schneidermann]

Hello Ka-tsùn,

> - This implementation passes all of the Autobahn compliance tests.
> I've included the client and configuration file in an examples/
> directory, but the repository currently has no tests/ because the
> Autobahn tests are usually run from a docker image and take a while to
> finish.

I don't know much about websockets at all, but would it be possible to
at least include a smoke test or sanity check? There are a few eggs
(json-rpc, srfi-18, fuse, gochan, sendfile, spiffy) that spawn a server
process with process-fork, send a request to it and check whether it was
successful. There's even the server-test egg dedicated for this purpose.

> - permessage-deflate presently interfaces with the C zlib directly,
> because we use options for the stream interface that the zlib egg
> doesn't currently expose. This results in some repeated code.

I've hacked on that egg recently and discovered that the author wanted
to expose deflateInit2 and inflateInit2 for a while, so I helped review
their code and they released a new version exposing MAX_WBITS. I've also
submitted a PR demonstrating how this can be used to handle raw deflate,
zlib and gzip data: <https://github.com/r1b/zlib/pull/2>. Perhaps this
is of use.

> - I am relatively new to the language, so I would be extremely
> grateful if anyone would like to take a look at the source & provide
> some feedback! I fear silly mistakes, and also suspect much of the
> code might be made more idiomatic for Scheme.

The code looks fine to me. There's small improvements that can be made
to improve readability:

- `(make-composite-condition (make-property-condition ...) ...)` can be
  replaced with `(condition '(...) ...)`.
- `(case o ((...) #t) (else #f))` can be replaced with `(and (memv o
  '(...)) #t)`. It might not be equivalent in terms of generated code
  though.
- `(if condition consequent)` and `(if condition (begin consequent
  ...))` are equivalent to `(when condition consequent)` and `(when
  condition consequent ...)`.
- The `foreign-lambda*` using memcpy might be replacable with a call to
  `move-memory!` from `(chicken memory)`.

Vasilij

signature.asc
Description: PGP signature