Support a new option "stopNodes". #150

jinq102030 · 2019-03-11T21:52:45Z

This field is an array with 0 or more tagnames. When it encounters a tagname that matches the list of stopNodes, it will treat all the stuff understand this tag as a string.

Purpose / Goal

This is used to support the parsing of certain xml documents where some nodes, such as contains a xhtml structure. We need to treat it as a string and pass it to calling entity.

This field is an array with 0 or more tagnames. When it encounters a tagname that matches the list of stopNodes, it will treat all the stuff understand this tag as a string. This is useful when parsing xml, a part of it is considered as xhtml.

coveralls · 2019-03-11T21:57:09Z

Coverage decreased (-0.7%) to 96.497% when pulling f73ef82 on jinq102030:support_stopNodes into 5d424ab on NaturalIntelligence:master.

coveralls · 2019-03-11T21:57:13Z

Coverage increased (+0.03%) to 97.198% when pulling 7744609 on jinq102030:support_stopNodes into 5d424ab on NaturalIntelligence:master.

TheDeathConqueror · 2019-03-12T02:48:27Z

What if different tags are having the nested tag with the same name? It'll be applicable on all. And what's about the tag inside CDATA. Have you checked the applications where it can be used? I mean, since it is gonna impact the performance of the application, it should be implemented only if it is required by multiple projects. On 12 Mar 2019 3:22 am, "jinq102030" <notifications@github.com> wrote: This field is an array with 0 or more tagnames. When it encounters a tagname that matches the list of stopNodes, it will treat all the stuff understand this tag as a string. Purpose / Goal This is used to support the parsing of certain xml documents where some nodes, such as contains a xhtml structure. We need to treat it as a string and pass it to calling entity. ------------------------------ You can view, comment on, or merge this pull request online at: #150 Commit Summary - Support a new option "stopNodes". File Changes - *M* src/xmlstr2xmlnode.js <https://github.com/NaturalIntelligence/fast-xml-parser/pull/150/files#diff-0> (11) Patch Links: - https://github.com/NaturalIntelligence/fast-xml-parser/pull/150.patch - https://github.com/NaturalIntelligence/fast-xml-parser/pull/150.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#150>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AiuBatkHuuvQizgyFeH3aXgUlkBGrD_Pks5vVtAugaJpZM4bpo2i> .

jinq102030 · 2019-03-12T13:56:18Z

What if different tags are having the nested tag with the same name? It'll be applicable on all. And what's about the tag inside CDATA. Have you checked the applications where it can be used? I mean, since it is gonna impact the performance of the application, it should be implemented only if it is required by multiple projects. On 12 Mar 2019 3:22 am, "jinq102030" notifications@github.com wrote: This field is an array with 0 or more tagnames. When it encounters a tagname that matches the list of stopNodes, it will treat all the stuff understand this tag as a string. Purpose / Goal This is used to support the parsing of certain xml documents where some nodes, such as contains a xhtml structure. We need to treat it as a string and pass it to calling entity. ------------------------------ You can view, comment on, or merge this pull request online at: #150 Commit Summary - Support a new option "stopNodes". File Changes - M src/xmlstr2xmlnode.js https://github.com/NaturalIntelligence/fast-xml-parser/pull/150/files#diff-0 (11) Patch Links: - https://github.com/NaturalIntelligence/fast-xml-parser/pull/150.patch - https://github.com/NaturalIntelligence/fast-xml-parser/pull/150.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#150>, or mute the thread https://github.com/notifications/unsubscribe-auth/AiuBatkHuuvQizgyFeH3aXgUlkBGrD_Pks5vVtAugaJpZM4bpo2i .

Hi TheDeathConqueror,

In case of nested tags from stopNodes, the top most tag (first one encountered during parsing) will win. Effectively parser doesn't go down further into the node when it encounters a stopNode.
Not sure about what you mean by a node in CDATA, CDATA should be considered as blob of data, right?

You reminded me that I probably need to highlight some of these in README.

Thanks!

amitguptagwl · 2019-03-14T00:44:08Z

Thanks for your PR. Can you please add unit tests for following cases?

Single and multiple stop nodes
When the stop node has nothing inside, or it is a self-closing tag.
With and without attributes.
When a stop node has CDATA, other nested tags, or another stop node inside.
Stop node on the root level

I'm just wondering what should happen when we convert back to XML. In my understanding, we'll get the same XML.

jinq102030 · 2019-03-15T18:31:23Z

Sorry for the late response. Thanks for the good tests.

Yes, I tested the cases of 1 or more stopNodes, they are fine.
I have not tested the case of stopNode happening to be empty node, will do.
With and without attribute
Going to test the case of stopNode tag containing a CDATA. In the case of nested stopNodes, it will essentially stop parsing into that node when it encounter a stopNode.
That's a good corner case.

Should I just run the test them in my local environment or add some tests along with the PR? Thanks.

amitguptagwl · 2019-03-16T01:53:22Z

Yes, we need to add the tests along with PR, to ensure that nothing is broken any future changes.

jinq102030 · 2019-03-16T22:04:31Z

Tried to run the mocha spec/attr_spec.js, got the following error even though I did npm install. Wonder what was wrong. Need help!

fast-xml-parser jqian$ mocha spec/attr_spec.js 


  XMLParser
    1) should parse attributes with valid names
    2) should parse attributes with newline char
    3) should not decode HTML entities / char by default
    4) should decode HTML entities / char
    5) should parse Boolean Attributes
    6) should not remove xmlns when namespaces are not set to be ignored
    7) should remove xmlns when namespaces are set to be ignored
    8) should not parse attributes with name start with number
    9) should not parse attributes with invalid char
    10) should not parse attributes in closing tag
    11) should err for invalid atributes
    12) should validate xml with atributes
    13) should validate xml atribute has '>' in value
    14) should not validate xml with invalid atributes
    15) should not validate xml with invalid attributes when duplicate attributes present
    16) should not validate xml with invalid attributes when no space between 2 attributes
    17) should not validate a tag with attribute presents without value 
    18) should not validate xml with invalid attributes presents without value
    19) should validate xml with attributeshaving openquote in value


  0 passing (23ms)
  19 failing

  1) XMLParser
       should parse attributes with valid names:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:25:9)

  2) XMLParser
       should parse attributes with newline char:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:48:9)

  3) XMLParser
       should not decode HTML entities / char by default:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:71:9)

  4) XMLParser
       should decode HTML entities / char:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:96:9)

  5) XMLParser
       should parse Boolean Attributes:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:127:9)

  6) XMLParser
       should not remove xmlns when namespaces are not set to be ignored:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:151:9)

  7) XMLParser
       should remove xmlns when namespaces are set to be ignored:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:176:9)

  8) XMLParser
       should not parse attributes with name start with number:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:195:9)

  9) XMLParser
       should not parse attributes with invalid char:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:209:9)

  10) XMLParser
       should not parse attributes in closing tag:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:222:9)

  11) XMLParser
       should err for invalid atributes:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:234:9)

  12) XMLParser
       should validate xml with atributes:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:241:9)

  13) XMLParser
       should validate xml atribute has '>' in value:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:248:9)

  14) XMLParser
       should not validate xml with invalid atributes:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:260:9)

  15) XMLParser
       should not validate xml with invalid attributes when duplicate attributes present:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:273:9)

  16) XMLParser
       should not validate xml with invalid attributes when no space between 2 attributes:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:285:9)

  17) XMLParser
       should not validate a tag with attribute presents without value :
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:297:9)

  18) XMLParser
       should not validate xml with invalid attributes presents without value:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:311:9)

  19) XMLParser
       should validate xml with attributeshaving openquote in value:
     ReferenceError: expect is not defined
      at Context.<anonymous> (spec/attr_spec.js:325:9)

amitguptagwl · 2019-03-17T02:38:15Z

The tests are written in jasmine. You can find that in package.json. You can run them by npm test.

jinq102030 · 2019-03-17T15:24:49Z

Thanks Amit for your suggestion. Added the tests spec/stopNodes_spec.js.

amitguptagwl

Please add 2 more tests

amitguptagwl · 2019-03-18T02:35:37Z

spec/stopNodes_spec.js

+    });
+
+    it("1b. 3. should support more than one stopNodes, with or without attr", function() {
+        const xmlData = `<issue><title>test 1</title><fix1 lang="en"><p>p 1</p><div class="show">div 1</div></fix1><fix2><p>p 2</p><div class="show">div 2</div></fix2></issue>`;


Please write a test when fix2 is inside fix1. In this case, fix2 should be empty in the result.

Done. Thanks for the suggestion.

amitguptagwl · 2019-03-18T02:36:51Z

spec/stopNodes_spec.js

+    });
+
+    it("2. stop node has nothing in it", function() {
+        const xmlData = `<issue><title>test 1</title><fix1></fix1></issue>`;


Please add a test when fix1 is a self closing tag.

Done. Thanks for the suggestion.

jinq102030 · 2019-03-22T23:14:55Z

Hi Amit, thanks for the review comments, they are implemented. Could you check? Thanks.

amitguptagwl · 2019-03-23T05:44:09Z

Thanks for the changes. Everything looks perfect. I'll approve the changes soon. However, it may take some time to publish.

jinq102030 · 2019-03-23T15:50:00Z

Thanks Amit!

Support a new option "stopNodes".

f73ef82

This field is an array with 0 or more tagnames. When it encounters a tagname that matches the list of stopNodes, it will treat all the stuff understand this tag as a string. This is useful when parsing xml, a part of it is considered as xhtml.

Add spec (Thanks Amit!) and fixed a bug.

4be30be

amitguptagwl requested changes Mar 18, 2019

View reviewed changes

Add more test cass as Amit suggested.

7744609

amitguptagwl merged commit 03900d9 into NaturalIntelligence:master Mar 23, 2019

sverweij mentioned this pull request Oct 4, 2019

stopNodes not in type definitions yet #191

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Support a new option "stopNodes". #150

Support a new option "stopNodes". #150

jinq102030 commented Mar 11, 2019

coveralls commented Mar 11, 2019

coveralls commented Mar 11, 2019 •

edited

Loading

TheDeathConqueror commented Mar 12, 2019 via email

jinq102030 commented Mar 12, 2019

amitguptagwl commented Mar 14, 2019

jinq102030 commented Mar 15, 2019

amitguptagwl commented Mar 16, 2019

jinq102030 commented Mar 16, 2019

amitguptagwl commented Mar 17, 2019

jinq102030 commented Mar 17, 2019

amitguptagwl left a comment

amitguptagwl Mar 18, 2019

jinq102030 Mar 21, 2019

amitguptagwl Mar 18, 2019

jinq102030 Mar 21, 2019

jinq102030 commented Mar 22, 2019

amitguptagwl commented Mar 23, 2019

jinq102030 commented Mar 23, 2019

Support a new option "stopNodes". #150

Support a new option "stopNodes". #150

Conversation

jinq102030 commented Mar 11, 2019

Purpose / Goal

coveralls commented Mar 11, 2019

coveralls commented Mar 11, 2019 • edited Loading

TheDeathConqueror commented Mar 12, 2019 via email

jinq102030 commented Mar 12, 2019

amitguptagwl commented Mar 14, 2019

jinq102030 commented Mar 15, 2019

amitguptagwl commented Mar 16, 2019

jinq102030 commented Mar 16, 2019

amitguptagwl commented Mar 17, 2019

jinq102030 commented Mar 17, 2019

amitguptagwl left a comment

Choose a reason for hiding this comment

amitguptagwl Mar 18, 2019

Choose a reason for hiding this comment

jinq102030 Mar 21, 2019

Choose a reason for hiding this comment

amitguptagwl Mar 18, 2019

Choose a reason for hiding this comment

jinq102030 Mar 21, 2019

Choose a reason for hiding this comment

jinq102030 commented Mar 22, 2019

amitguptagwl commented Mar 23, 2019

jinq102030 commented Mar 23, 2019

coveralls commented Mar 11, 2019 •

edited

Loading