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

Trim values from XML so auto-formatting our XML does not break the autoloader. #1350

Merged
merged 1 commit into from Dec 14, 2020
Merged

Trim values from XML so auto-formatting our XML does not break the autoloader. #1350

merged 1 commit into from Dec 14, 2020

Conversation

woutersamaey
Copy link
Contributor

@woutersamaey woutersamaey commented Dec 14, 2020

If you auto-format an XML file (like in PHPStorm), the value/class name can end up on a new line. This breaks the loading of the class, because Magento tries to create a class that starts with a newline and spaces. This simple trim avoids formatting issues.

@woutersamaey
Copy link
Contributor Author

woutersamaey commented Dec 14, 2020

This PR fixes XML that looks like this:

<adminhtml>
   <rewrite>
      <catalog_product_edit_tab_inventory>
         Storefront_MultiStock_Block_Adminhtml_Catalog_Product_Edit_Tab_Inventory
      </catalog_product_edit_tab_inventory>
   </rewrite>
</adminhtml>

Before the XML need to be like this, but it's hard to read & can't auto-format

<adminhtml>
   <rewrite>
<catalog_product_edit_tab_inventory>Storefront_MultiStock_Block_Adminhtml_Catalog_Product_Edit_Tab_Inventory</catalog_product_edit_tab_inventory>
   </rewrite>
</adminhtml>

Auto-formatting in PHPStorm and other IDEs puts the contents on different lines because the node name / value is too long.

Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish it was possible to apply a broader fix but I don't know of an elegant way to do that. Thanks for the PR!

@github-actions github-actions bot added the Component: Core Relates to Mage_Core label Dec 14, 2020
@Flyingmana
Copy link
Member

some more information on this topic. XML has some informations related handling this in its standard and a special attribute to signal the intent for whitespace handling.
http://www.xmlplease.com/xml/xmlspace/

Why is this relevant? Because this means we dont violate any xml related standard by doing a trim.

For further going changes, there is some "normalize-space()" function mentioned, which can do this.
There are also mentioned XSLT methods to remove whitespaces <xsl:strip-space element="*"/> <xsl:output indention="no"/>

In PHP there is DOMDocument::normalizeDocument ( ) which may be something related to this (but I did not check this further)

Anyway, for this specific case, Iam good with the solution for now.

@Flyingmana Flyingmana merged commit d04ac13 into OpenMage:1.9.4.x Dec 14, 2020
@sreichel sreichel added this to the Release 19.4.9 / 20.0.5 milestone Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants