node-xml2js icon indicating copy to clipboard operation
node-xml2js copied to clipboard

new builder options: tagNameProcessors and attrNameProcessors

Open jcsahnwaldt opened this issue 6 years ago • 2 comments

  • Added tag and attribute name processing to the builder (similar to what the parser does)
  • Added tests for tag and attribute name processing for the builder (similar to the tests for the parser)
  • Minor refactoring in parser.coffee: processItem was always used in the same way, removed some copied code
  • Added package-lock.json to .gitignore to make git handling simpler. (If you want, we can revert this.)
  • Added a TODO to the line for key, entry of child in builder.coffee - should it be for own key...?
  • Updated README.md.
  • Note: We could also try to add value processors, but I'd rather do that in a separate PR.

Potential issues:

  • Backwards compatibility: Some users may have used the same options object for the parser and the builder in their code. Until now, this worked, because the builder simply ignored tagNameProcessors and attrNameProcessors. After this pull request, such users may run into problems because their builder modifies the names in unexpected ways. Possible remedies:
    • Change the names of the builder options, e.g. builderTagNameProcessors and builderAttrNameProcessors.
      • Rather ugly, I think. In the long run, this will be confusing rather than helpful.
    • Add another option that activates tagNameProcessors and attrNameProcessors in the builder, e.g. builderProcessors: true or version: "0.4.20". Old code doesn't contain this option and thus works as usual.
      • Workable, but maybe users will expect all options to be "versioned" in this way...
    • Simply update the documentation and instruct users not to use the same options object for parser and builder. (With object spread syntax (available since Node.js 8.3.0), it's easy to have a common base options object and extend it with different options for builder and parser.)
      • In my opinion, this is the best solution, so I added a few sentences to README.md

jcsahnwaldt avatar Jun 09 '18 14:06 jcsahnwaldt

Coverage Status

Coverage increased (+0.07%) to 97.708% when pulling b0277eb70228b863655ca0f654baf52ac0205872 on iami-me:add_prop_name_processors into 049242dbefc2d69e8e8ceac566b9e862a58c9ff1 on Leonidas-from-XIV:master.

coveralls avatar Jun 09 '18 14:06 coveralls

Hello guys, when will this be implemented? Thank you 😀

MartinLednar avatar May 24 '23 11:05 MartinLednar