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

Pass options to Builder::XmlMarkup #35

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davispuh
Copy link
Contributor

This PR adds option :builder which is directly passed to Builder::XmlMarkup
So it's possible to pass options like :indent and others. For example

Gyoku::xml({:some => { :new => "user" }}, {:builder => { :indent => 2} })

will produce

<some>
  <new>user</new>
</some>

To make indentation work correctly with Builder I had to make changes in that gem, so this won't work unless jimweirich/builder#44 is merged

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.37%) when pulling d55cb6a on davispuh:builder_options into 93264ee on savonrb:master.

@tjarratt
Copy link
Contributor

I'm not sure we actually want to do this. What problem were you actually trying to address @davispuh ? Most consumers of the XML gyoku produces shouldn't care how the xml is indented.

If the intention was to change this for pretty printing, then I agree we want to do something to make this work better, but a better approach might be to introduce a to_pretty_xml method that would indent so it's human readable.

Also, terribly sorry no one has responded to this issue for two months. That's a long time to leave a PR hanging.

@tjarratt
Copy link
Contributor

tjarratt commented Jan 4, 2014

Okay, I thought about this some more, and it makes a lot more sense to me. Let's get this merged in once jimweirich/builder#44 is merged.

@tjarratt tjarratt closed this Jan 4, 2014
@tjarratt tjarratt reopened this Jan 4, 2014
@kristianmandrup
Copy link

+1 for the ability to add Builder options such as indenting for pretty output (Perhaps also on a global level via class method config?). Could you please push a version with this feature to rubygems ;)
Thanks.

@tjarratt
Copy link
Contributor

Sure! I'll see about pushing a new version to rubygems later today.

Sent From A Very Small Keyboard

On Jan 28, 2014, at 6:54, Kristian Mandrup [email protected] wrote:

+1 for the ability to add Builder options such as indenting for pretty output (Perhaps also on a global level via class method config?). Could you please push a version with this feature to rubygems ;)
Thanks.


Reply to this email directly or view it on GitHub.

@kristianmandrup
Copy link

Did you?

@tjarratt
Copy link
Contributor

@kristianmandrup Oh, sorry -- I just noticed that the PR to the builder gem wasn't merged in yet.

@chewi
Copy link

chewi commented Nov 11, 2015

That pull request doesn't seem to be going anywhere fast. Is it time to consider the alternative in the mean time?

@Jeiwan Jeiwan mentioned this pull request Dec 9, 2015
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

Successfully merging this pull request may close these issues.

5 participants