Bug in parse-to-list ?

[syll-dev]

(parse-to-list str) is equivalent to (node->nodelist (parse str)).

if (parse str) fails to read the xml document and returns nil, parse-to-list fails with an unclear error message :

* (parse-to-list "iueiueieiueiu")

debugger invoked on a SB-KERNEL:CASE-FAILURE @54B0750F in thread
#<THREAD tid=49815 "main thread" RUNNING {10048381D3}>:
  NIL fell through ETYPECASE expression. Wanted one of (STRING NODE).

Type HELP for debugger help, or (SB-EXT:EXIT) to exit from SBCL.

restarts (invokable by number or by possibly-abbreviated name):
  0: [ABORT] Exit debugger, returning to top level.

(NODE->NODELIST NIL)
   source: (ETYPECASE NODE
             (STRING NODE)
             (NODE
              (LIST*
               (IF (NODE-NS NODE)
                   (CONS (NODE-NAME NODE) (NODE-NS NODE))
                   (NODE-NAME NODE))
               (NODE-ATTRS NODE)
               (MAPCAR 'NODE->NODELIST (NODE-CHILDREN NODE)))))

The result is the same with this call :

(parse-to-list "iueiueieiueiu" :quash-errors nil)

Could replacing handler-case with ignore-errors in function parse fix the problem ?

(parse "iueiueieiueiu") returns nil.
(parse "iueiueieiueiu" :quash-errors nil) returns nil.
(parse "iueiueieiueiu" :quash-errors t) returns nil.

Is this the intended behaviour ?

Should (node->nodelist nil) return nil ? It fails with the error above for now.

When parse returns nil, should parse-to-list return nil ?

Should (parse-to-list nil) be strictly equivalent to (node->nodelist (parse str)) for convenience or return nil / signal a clearer error when parse returns nil ?

[rpgoldman]

Both parse-to-list and parse are supposed to honor the :quash-errors flag. Presumably they should both treat nil in the same way. @robert-dodier convinced me that it wasn't clear whether XML parsing should accept the empty document or not. That is why (parse "iueiueieiueiu" :quash-errors nil) returns nil, instead of raising an error, I believe.

If that is the case, then it seems to me that parse-to-list should behave the same and accept empty XML documents.

I will see about a quick MR.

[rpgoldman]

See #19

[rpgoldman]

@robert-dodier Is this the change we expected? It seems reasonable to me that (parse "") should return nil instead of signaling an error. But shouldn't a non-XML string raise an error (at least if :quash-errors is nil)?

[robert-dodier]

@rpgoldman I'm afraid I don't understand what "this" refers to. At any rate, I agree that a non-XML string should raise an error when :quash-errors is nil.

I think what I was arguing for, some time ago, is to stretch the JSON spec to say that an empty document is valid, and the parser returns nil for that. That's noncompliant, but XMLS already has another noncompliant behavior, namely to allow multiple objects in a document. I was suggesting that it's more useful to allow both noncompliant behaviors, as opposed to enforcing the spec more carefully and disallowing both. However, if that noncompliance causes headaches in some way, it's okay by me to enforce compliance. I'm not sure where the present discussion falls in all this.

[rpgoldman]

Yes, that’s what I was referring to. We were making it so that empty (or multiple?) documents were OK, and I think I may have inadvertently made it so a string of gibberish was also ok.

I haven’t used XMLS in ages, so none of this is fresh in my memory.

Sounds like it’s ok to fix it so that an error is raised for a non-XML string. Thanks