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

Printformatting #914

Merged
merged 11 commits into from
Apr 28, 2021
Merged

Printformatting #914

merged 11 commits into from
Apr 28, 2021

Conversation

lwhite1
Copy link
Collaborator

@lwhite1 lwhite1 commented Apr 28, 2021

Description

Deprecated support in CsvWriter for writing Date and DateTime columns using Java's DateTimeFormatter directly.

Replaced them with a new option usePrintFormatters(Boolean) that tells CsvWriter to use any printFormatters defined on all types of columns, unifying the output created by printAll() and other toString() variations with the writing of delimited text files. The new approach also handles missing values flexibly.

If this looks ok, I will add support for the other file output types using WriteOptions: html, json, fixed-width. I'm not really sure how the excel writing works, and i'm happy that way.

The sonar failure is sonar being stupid. It says I should remove the deprecated code some day. Which is true, but not a failure or code-smell today.

Testing

I added 10 tests

lwhite1 added 5 commits April 27, 2021 16:09
This branch is for evaluating the possibility of using ColumnPrintFormatters to optionally format output.

There will be a writeOption called ```usePrintFormatters(boolean)``` that turns the logic on or off. It will be off by default, matching the current status.

The print formatter is set on the column and would then be used by both table.print and table.write, providing more consistent output

PrintFormatters have the advantage of handling missing values so the user does not have to worry about them blowing up a Java NumberFormatter for instance, while allowing the user to set the missing value symbol for each column.
@lwhite1 lwhite1 marked this pull request as draft April 28, 2021 13:38
@lwhite1 lwhite1 marked this pull request as ready for review April 28, 2021 14:35
@lwhite1 lwhite1 requested a review from benmccann April 28, 2021 14:35
@sonarqubecloud
Copy link

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