search

Shinmera / plump

Practically Lenient and Unimpressive Markup Parser for Common Lisp
https://shinmera.github.io/plump
zlib License
120 stars 21 forks source link

*stream* should track cl:*standard-output* #8

Closed PuercoPop closed 9 years ago

PuercoPop commented 9 years ago

While helping out someone over at #lisp I saw a subtle bug with serialize. It is even more subtle because serialize doesn't use *stream* default value by default. Below is a test case illustrating the bug.

;; (ql:quickload :plump)
(ql:quickload :plump :silent t)

(ql:quickload :drakma)
(defvar *root* (plump:parse (babel:octets-to-string (drakma:http-request "http://adndevblog.typepad.com/autocad/rss.xml"))))

(plump:serialize *root*)
(plump:serialize *root* plump:*stream*) ; <- Doesn't write to the standard-output
(plump:serialize *root* nil)

Related: http://lisptips.com/post/127524780849/when-a-synonym-stream-is-useful

Shinmera commented 9 years ago

This makes no sense to me. The documentation explicitly says that *stream* is for the usage within serialize-object, not serialize. serialize itself always rebinds *stream*, ensuring that it has the proper value. If anything, *stream* should not be bound to a value at all by default to make sure people don't stupidly use it as a parameter.

Your patch also makes no sense as in the second case it never actually uses the stream argument and rebinds *stream* to *stream* (??). Even if it actually did that, it would still conflict with the documentation as it would break passing T as stream.

This just won't do.

PuercoPop commented 9 years ago

If anything, stream should not be bound to a value at all by default

Yes, that also seems like a reasonable approach.

Your patch also makes no sense as in the second case it never actually uses the stream argument and rebinds stream to stream

You are correct, that let block is superflous and should be removed.

it would still conflict with the documentation as it would break passing T as stream.

It does not break the existing functionality. Passing t works fine as t is an output stream designator denoting standard output. Feel free to try it yourself.

(plump:serialize *root* t) ; Prints normally
(let ((*standard-output* (make-string-output-stream)))
  (plump:serialize *root* t))  ; <- Won't print anything
Shinmera commented 9 years ago

Ah, right. I forgot that write-string takes a designator rather than a direct stream. My bad.