Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option removeUnusedNamespaces does not work as documented #1

Closed
codeworrior opened this issue Aug 5, 2020 · 1 comment
Closed

Option removeUnusedNamespaces does not work as documented #1

codeworrior opened this issue Aug 5, 2020 · 1 comment

Comments

@codeworrior
Copy link

  • First of all, the implementation contains a typo. It checks for a non-empty array of used namespaces before removing the unused ones. I guess the intention was to check for unused to be non-empty.

    Fails to remove namespace 'n' from

    <a xmlns:n="..." />
  • Second, the implementation only checks for namespaces that are used for elements, not for namespaces that are used for attributes.

    Unintentionally removes namespace 'n' from

     <a xmlns:n="..." n:attr="value" />
  • last but not least, as the regexp based approach does not understand the structure of the XML, it cannot detected when a local namespace is unnecessary in a subtree, it can only decide globally.

    Does not remove 2nd declaration of namespace 'n' in

     <a>
         <b1 xmlns:n="...">
             <n:c/>
         </b1>
         <b2 xmlns:n="...">
         </b2>
     </a>

    This does not cause errors, but does not achieve the optimal reduction.

The 2nd issue is the most important one for our use case in UI5 as we have some special namespaces that are used only for attributes (e.g. for custom data).

kristian pushed a commit that referenced this issue Aug 6, 2020
Adresses issue #1. Fixes a bug which caused unused namespaces to not be
removed, if no other namespace was used. Fixes a bug which caused
namespaces to be removed, which have been in used by attributes.

Adds a comment to the README.md that namespace removal will *not*
consider namespaces which are only used for a certain sub-tree of the
document.
kristian added a commit that referenced this issue Aug 6, 2020
Adresses issue #1. Fixes a bug which caused unused namespaces to not be
removed, if no other namespace was used. Fixes a bug which caused
namespaces to be removed, which have been in used by attributes.

Adds a comment to the README.md that namespace removal will *not*
consider namespaces which are only used for a certain sub-tree of the
document.
@kristian
Copy link
Owner

kristian commented Aug 6, 2020

Fixed the bugs with 197305a and version 2.0.1. I was well aware of the sub-tre limitation, therefore I mentioned that only namespaces are removed that are not used anywhere in the document. But i think it‘s a good idea to point this out even more clearly int he README.md at least, which I did. Thanks @codeworrior!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants