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

Add an option for self-closing tags #31

Merged
merged 1 commit into from
Dec 22, 2020
Merged

Add an option for self-closing tags #31

merged 1 commit into from
Dec 22, 2020

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Dec 22, 2020

Use <tag /> instead of <tag></tag>

@maik
Copy link
Owner

maik commented Dec 22, 2020

Hi!

Thank you very much for your suggestion. Could you, please, explain the reasoning behind your solution?

Best,
Maik

@orgads
Copy link
Contributor Author

orgads commented Dec 22, 2020

Sure. There is a condition text_content.nil?, which is never satisfied, because text_content is assigned an empty string in case it is empty. Looks like it was overlooked in the original implementation.

@maik
Copy link
Owner

maik commented Dec 22, 2020

I am afraid I still do not get your point. Currently, XmlSimple emits <tag></tag> for empty tags and you want it to emit <tag /> instead?

@orgads
Copy link
Contributor Author

orgads commented Dec 22, 2020

Use backticks for tags.

@maik
Copy link
Owner

maik commented Dec 22, 2020

Sorry, I've fixed the markup.

@orgads
Copy link
Contributor Author

orgads commented Dec 22, 2020

Right. I also wrote it in the commit message, but the description in github didn't show it because it wasn't backticked. I edited the description later.

@maik
Copy link
Owner

maik commented Dec 22, 2020

Now I got it.

I am hesitating a bit to change the default behavior, because the library is widely used and I could imagine that some people rely on the current output format. (even though we all know it is a cardinal sin to process XML without a parser)

So, I suggest not to change the current output format but to add a new option. Using this option users could change the output format, then.

@orgads
Copy link
Contributor Author

orgads commented Dec 22, 2020

Fair enough.

Use <tag /> instead of <tag></tag>
@orgads orgads changed the title Fix handling of empty objects Add an option for self-closing tags Dec 22, 2020
@orgads
Copy link
Contributor Author

orgads commented Dec 22, 2020

Done. I don't know how to update the docs. I added the option to index.html, but I'm not sure about the rest. Can you please handle it?

@maik maik merged commit c5706f0 into maik:master Dec 22, 2020
@maik
Copy link
Owner

maik commented Dec 22, 2020

Wow, that was fast and you've also written text and documentation. Thank you very much!

@orgads orgads deleted the empty-obj branch December 22, 2020 12:27
@orgads
Copy link
Contributor Author

orgads commented Dec 22, 2020

My pleasure :)

@orgads
Copy link
Contributor Author

orgads commented Dec 29, 2020

When do you plan to release?

@maik
Copy link
Owner

maik commented Dec 29, 2020 via email

@orgads
Copy link
Contributor Author

orgads commented Dec 29, 2020

Thanks :)

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.

2 participants