flexi-streams-devel

Download

flexi-streams-devel@common-lisp.net

October 2009

1 participants
1 discussions

Re: [flexi-streams-devel] a patch for chineses's cp936(gbk) encoding support.
by Edi Weitz 29 Oct '09

29 Oct '09

On Thu, Oct 29, 2009 at 3:17 PM, jingtao xu <jingtaozf(a)gmail.com> wrote: > hi,Edi Weitz: > Because i want to use drakma as my web development tool,but it use > flexi-streams and could not decode chinese's cp936 characters,so i make a > patch to flexi-streams. > > The sbcl has supported cp936(by files supported by beinghe,which cvs path > is:sbcl/src/code/external-formats/enc-cn.lisp,enc-cn-tbl.lisp). i use file > enc-cn-tbl.lisp here. > > The patches is make by following commands: > diff -urN ~/.sbcl/site/flexi-streams-1.0.7/ ~/Downloads/flexi-streams/ >>flexi-streams-cp936.patch > > I send you both the patch files and the patched codes. > The test codes is added and passed. > The documention is updated. [I've put the mailing list on Cc where I think we should continue this.] First of all, I'd be happy to include this into the flexi-streams distribution. There are a couple of issues with the patch, though. What I saw from briefly looking at it: 1. There are several new functions and macros which don't have a documentation string. 2. There are tabs in the files and sometimes the indentation seems wrong. 3. There are parts of the code which are commented out. If they aren't used, they shouldn't be in there. 4. There's at least one file (seems like a variant of the ASDF system definition) which shouldn't be in there at all. 5. The link in the HTML documentation is wrong. 6. Isn't get-multibyte-mapper the wrong name? You don't get the mapper, do you? 7. And the function itself looks like Scheme to me. I think it'd be easier to understand using normal iteration. 8. I don't understand why the encoding factor is 1.5. Is the comment correct or just copied? 9. I'm not familiar with CP936. Is it correct that there's only a big-endian version? 10. One of the new files contains DOS line endings. There's probably more. If you could go over these and send a cleaned-up version of the patch, I'll review it. FWIW, here are some guidelines: http://weitz.de/patches.html Thanks, Edi.

1 0