Skip to content

Handle well-indented XML including comments#795

Merged
kuenishi merged 1 commit into
release/1.4from
bugfix/gh782-acl-xml-format-1.4
Feb 8, 2014
Merged

Handle well-indented XML including comments#795
kuenishi merged 1 commit into
release/1.4from
bugfix/gh782-acl-xml-format-1.4

Conversation

@kuenishi
Copy link
Copy Markdown
Contributor

@kuenishi kuenishi commented Feb 6, 2014

The root cause for #782 was misuse of xmerl_scan:string/2, which doesn't emit spaces, comments between actual xml elements. It leaves them as #xmlText{} and #xmlComment{} - This commit changes to ignore them at walking around parsed dom tree. This diffset also includes a small refactoring that wraps all xmerl_scan:string/2 to one riak_cs_xml funcsion to control its usage.

This also adds regression test as simple eunit test case.

backport of #793

The root cause for #782 was misuse of xmerl_scan:string/2,
which doesn't emit spaces, comments between actual xml elements.
It leaves them as #xmlText{} and #xmlComment{} - This commit
changes to ignore them at walking around parsed dom tree. This
diffset also includes a small refactoring that wraps all
xmerl_scan:string/2 to one riak_cs_xml funcsion to control its
usage.

This also adds regression test as simple eunit test case.

Conflicts:
	src/riak_cs_acl_utils.erl
	src/riak_cs_s3_response.erl
@kuenishi kuenishi added this to the 1.4.5 milestone Feb 6, 2014
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really nitpicky, but why assign this to Xml, instead of just returning the string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without binding Xml the indentation by erlang-mode.el becomes like this:

    "<!-- comments here --> <AccessControlPolicy> <!-- comments here -->"
        "  <Owner> <!-- blah blah blah -->"
        "    <ID>eb874c6afce06925157eda682f1b3c6eb0f3b983bbee3673ae62f41cce21f6b1</ID>"
        " <!-- c -->   <DisplayName>admin</DisplayName> <!--"
        " \n --> </Owner>"

Also it's easy to put some ~p to do some formatting like io_lib:format(Xml, [ Blah, Blah, ... ]) for future improvement. Yes, I was trying that.

@reiddraper
Copy link
Copy Markdown
Contributor

Tests, code and manual testing all pass. +1

kuenishi added a commit that referenced this pull request Feb 8, 2014
Handle well-indented XML including comments
@kuenishi kuenishi merged commit 75d030a into release/1.4 Feb 8, 2014
@kuenishi kuenishi deleted the bugfix/gh782-acl-xml-format-1.4 branch February 8, 2014 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants