Baseurl for xml (as well as html) #388
Conversation
| @@ -317,7 +317,7 @@ xmlParserOption getParserOptions(v8::Local<v8::Object> props) { | |||
| ret |= getParserOption(props, "cdata", XML_PARSE_NOCDATA, false); // 16384: merge CDATA as text nodes | |||
|
|
|||
| ret |= getParserOption(props, "noxincnode", XML_PARSE_NOXINCNODE); // 32768: do not generate XINCLUDE START/END nodes | |||
| ret |= getParserOption(props, "xinclude", XML_PARSE_NOXINCNODE, false); // 32768: do not generate XINCLUDE START/END nodes | |||
| ret |= getParserOption(props, "xincnode", XML_PARSE_NOXINCNODE, false); // 32768: do not generate XINCLUDE START/END nodes | |||
|
|
|||
There was a problem hiding this comment.
As the commit message describes, I am having trouble getting xinclude to work, and tried to fix it with this change. This looks like a typo to me: the xinclude property is already defined above, and I think this one should be xincnode. But, it didn't fix my problem, and so I'm not sure. If I'm wrong, let me know, and I'll take this out of the PR.
There was a problem hiding this comment.
Yes this is a typo. The xinclude option is defined here. Good catch.
|
Hello? |
|
In case someone comes along needing this feature, I published this as @klortho/libxmljs |
|
@teleological @rc0x03 could you take a look? This also needs tests. |
|
Since |
|
I just added a test for baseUrl on fromXml, but
I'm afraid I don't really have time right now, nor am I confident to do it -- that NAN code looks a little scary. |
|
Tests pass CI. |
|
Tests look good. Squash the commits and we can merge this. |
|
Done |
Fix fromXML baseURL and `xincnode` option typo.
This PR comprises two changes. I'll describe them as line comments below.
The main purpose of this PR is to add the baseUrl option to FromXml. I noticed that this (and a few other options) were already added to FromHtml a long time ago, so I added them to FromXml in the same way.
The two methods are almost identical, and probably should be refactored to move the common code into a separate function that they both call. If it had been done that way originally, then users of FromXml would have benefited from these extra options added to FromHtml.
So, some of the changes were just for the purposes of harmonizing the FromHtml and FromXml methods. I didn't want to factor out into a separate function, though -- it seemed like too big a change. In lieu of that, I just tried to get the two functions to be as similar as possible.