-
-
Notifications
You must be signed in to change notification settings - Fork 690
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
datatable/java: DataTable print with configurable indent + without escaping #1624
datatable/java: DataTable print with configurable indent + without escaping #1624
Conversation
…thout auto-escaping chars
datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTable.java
Outdated
Show resolved
Hide resolved
datatable/java/datatable/src/main/java/io/cucumber/datatable/TablePrinter.java
Outdated
Show resolved
Hide resolved
datatable/java/datatable/src/main/java/io/cucumber/datatable/DataTable.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rewritten the rendering logic from scratch to make the table printer thread-safe. Coincidently this also simplified the code a whole lot. Added a few tests to confirm I didn't break anything.
Thing I'm still wondering about is the naming. I'm not happy about the Printer
suffix.
Comparable bits of code in the JDK are named Formatter
and use the format(Thing) : String
formatTo(Thing, Appendable)
.
Looks good, thanks! The only difference I noticed is that the comparable |
Hi @artysidorenko, Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
I'll have to find some time to make the release. |
Done. Unfortunately there are some breaking changes that precede your change. If you want to continue this in Cucumber JVM. I would suggest branching of this commit in the |
Nevermind that. Branch of |
* updates PrettyFormatter.printStep to process the DataTable if one is present in the step object * uses DataTableFormatter exposed in cucumber/common#1624
Summary
Updates DataTable with a new print method allowing to configure custom indention, and skip escaping the cell contents.
Details
TablePrinter
class configurable, via a newTablePrinterBuilder
class.printFormatted()
to theDataTable
class, as it already includes similar publicly-exposedprint()
methodsTablePrinter
with custom indent + removing cell-escaping.(I'm still not too familiar with the codebase; the above felt like reasonable decisions, but am happy to incorporate feedback if you prefer doing it different)
Motivation and Context
Relates to issue-2320 on cucumber-jvm.
Ultimate aim is to include data-tables in the printed test output. E.g. in the following feature, current behaviour prints out only the step definition, but it skips the info in the table.
How Has This Been Tested?
Added 2 unit tests.
can_print_table_with_escape_characters_regular
confirms the escape-cell behaviour of the originalprint()
method. Andcan_print_table_with_escape_characters_formatted
tests the behaviour of the newprintFormatted()
method.Types of changes
Checklist:
(don't think it requires documentation/porting to other languages as it's a small change?)