Skip to content

Add new config for PrettyPrinter to minimize empty tags #90

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

Merged
merged 3 commits into from
May 24, 2017

Conversation

ashawley
Copy link
Member

@ashawley ashawley commented Jan 25, 2016

This commit containing a unit test was mentioned last year in #46, and I thought the copy constructor using minimizeEmpty=true was broken, but it appears scala-xml does the right thing!

A new parameter to PrettyPrinter is added here for passing minimizeEmpty to the pretty-printed XML:

val pp = new xml.PrettyPrinter(80, 2, minimizeEmpty = true)
val x = <node><leaf></leaf></node>
pp.format(x))
// <node>
//   <leaf/>
// </node> 

ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 10, 2016
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley changed the title Unit test for #46 preserve short/self-closing/empty tags Fix #46 Add new config for PrettyPrinter to minimize empty tags Feb 10, 2016
@ashawley ashawley force-pushed the pretty-print-empty branch from 12eab7c to 89ea693 Compare April 22, 2016 20:03
ashawley added a commit to ashawley/scala-xml that referenced this pull request Apr 22, 2016
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from 89ea693 to 311f0ef Compare June 12, 2016 22:23
ashawley added a commit to ashawley/scala-xml that referenced this pull request Jun 12, 2016
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
debanne added a commit to criteo-forks/scalatest that referenced this pull request Jul 5, 2016
The Junit reports generated by scalatest are considered as invalid
when using for instance these xsd:
https://github.com/jenkinsci/xunit-plugin/blob/master/src/main/resources/org/jenkinsci/plugins/xunit/types/model/xsd/junit-10.xsd
https://github.com/windyroad/JUnit-Schema/blob/master/JUnit.xsd

The problem is that we have "<property ... > </property>" instead of
"<property ... />".

This is why the Jenkins XUnit plugin does not accept reports from
scalatest. This issue had already been raised (see issue scalatest#4 ).

Next step would be to fix org.scala.xml.PrettyPrinter which transforms
"<a .../>" to "<a ...>\n    </a>" when the line is too long. It is
possible that the option "minimizeEmpty" from
scala/scala-xml#90
will completely fix the issue.
@debanne
Copy link

debanne commented Jul 5, 2016

Hello, when the option minimizeEmpty is set to true, is PretttyPrinter still transforming <a ... /> to <a>\n </a> when the line is longer than "width" or does it prevent this behaviour?

@ashawley
Copy link
Member Author

@debanne Thanks for the note. This PR is just for providing a method to use the existing feature to minimize empty tags by adding a class parameter to configure as such. If there are existing issues with the pretty printer's behavior, they aren't taken up here.

So, the splitting of empty tags, making them open-close tags, and then annoyingly spreading them across two lines: That will need to be a separate issue or PR.

Here's one unit test, to get it started, though!:

  @UnitTest
  def prettyPrintAddsNewlines: Unit = {
    val pp = new xml.PrettyPrinter(4, 2)
    val x = <a/>
    // What it does:
    // assertEquals("<a>\n</a>", pp.format(x))
    // What it should do:
    assertEquals("<a/>", pp.format(x))
  }

This assumes I've understood your question.

ashawley added a commit to ashawley/scala-xml that referenced this pull request Sep 20, 2016
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Oct 18, 2016
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Dec 1, 2016
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@SethTisue
Copy link
Member

needs rebase now.

other than that, where do we stand, here...?

ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 4, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley changed the title Fix #46 Add new config for PrettyPrinter to minimize empty tags Add new config for PrettyPrinter to minimize empty tags Feb 4, 2017
@ashawley
Copy link
Member Author

ashawley commented Feb 4, 2017

Rebased.

This fixes half of #46 where it doesn't have a minimize empty option. I've renamed the issue so it doesn't auto-close. The other issues in #46 will have to be taken up separately.

ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 7, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 15, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from f02b6c2 to 6a4b3e7 Compare April 25, 2017 14:00
ashawley added a commit to ashawley/scala-xml that referenced this pull request Apr 25, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from 6a4b3e7 to ef969da Compare April 26, 2017 03:29
ashawley added a commit to ashawley/scala-xml that referenced this pull request Apr 26, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from ef969da to 48a9cfe Compare April 26, 2017 03:41
ashawley added a commit to ashawley/scala-xml that referenced this pull request Apr 26, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from 48a9cfe to 072001f Compare April 28, 2017 14:15
ashawley added a commit to ashawley/scala-xml that referenced this pull request Apr 28, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from 072001f to a8e9085 Compare April 28, 2017 18:28
ashawley added a commit to ashawley/scala-xml that referenced this pull request Apr 28, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from a8e9085 to 2e559bf Compare May 5, 2017 15:59
ashawley added a commit to ashawley/scala-xml that referenced this pull request May 5, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from 2e559bf to 58fc680 Compare May 6, 2017 14:31
ashawley added a commit to ashawley/scala-xml that referenced this pull request May 6, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
@ashawley ashawley force-pushed the pretty-print-empty branch from 58fc680 to 287a2bd Compare May 12, 2017 10:14
ashawley added a commit to ashawley/scala-xml that referenced this pull request May 12, 2017
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
ashawley added 3 commits May 24, 2017 11:27
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	New parameter minimizeEmpty.
	(PrettyPrinter.traverse): Pass config for minimizeTags to
	Utility.serialize().

	* src/test/scala/scala/xml/UtilityTest.scala (issue90): New test.

	* src/test/scala/scala/xml/XMLTest.scala (issue90): New test.
	* src/main/scala/scala/xml/PrettyPrinter.scala (PrettyPrinter):
	Convert class default parameter to alternate constructor.
@ashawley ashawley force-pushed the pretty-print-empty branch from 287a2bd to 8c44f6d Compare May 24, 2017 15:27
@ashawley ashawley merged commit b0a753d into scala:master May 24, 2017
@ashawley ashawley mentioned this pull request Oct 9, 2017
3 tasks
@@ -23,8 +23,11 @@ import Utility.sbToString
* @param width the width to fit the output into
* @param step indentation
*/
class PrettyPrinter(width: Int, step: Int) {
class PrettyPrinter(width: Int, step: Int, minimizeEmpty: Boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please document the expected behaviour when this parameter is set to true? (e.g. in @param)

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.

4 participants