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

FSharpAux.IO.SeqIO.Seq.CSV: Incorrect collection formatting #13

Open
ZimmerD opened this issue Jun 24, 2020 · 3 comments
Open

FSharpAux.IO.SeqIO.Seq.CSV: Incorrect collection formatting #13

ZimmerD opened this issue Jun 24, 2020 · 3 comments
Assignees
Labels

Comments

@ZimmerD
Copy link
Member

ZimmerD commented Jun 24, 2020

Description

The FSharpAux.IO.SeqIO.Seq.CSV function with the parameter flatten = false, does not perform as intended.

Repro steps

  1. Define a Record type with a field of type array.
  2. FSharpAux.IO.SeqIO.Seq.CSV function with the parameter flatten = false, using an instance of your type with an array of Length > 20.
  3. Look at the string.

Expected behavior

Nicely formatted output.

Actual behavior

The array is spread accross mutliple lines if it exceeds a certain length. This happens because "sprintf "%A" x" is used to format collections. This introduces line-brakes into the string.

Known workarounds

FSharpAux.IO.SeqIO.Seq.CSV calls FSharpAux.IO.SeqIO.Seq.CSVwith. Simply replace the parameter strFunction with a version that does the formatting correctly.

@ZimmerD ZimmerD added the bug label Jun 24, 2020
@ZimmerD ZimmerD self-assigned this Jun 24, 2020
@ZimmerD
Copy link
Member Author

ZimmerD commented Jun 25, 2020

In this gist you can find a proposed solution.

It fixes the bug but also introduces two little changes:

  1. I emitted the "[|" and "|]" marks at the beginning and ending of e.g. arrays. I think when parsing collections they do no real favor and can therefore be ommited.
  2. In lines 35-38 I assure that the inner separator is different from the outer one. Otherwise selecting ";" as a separator could break the parsing again. We could go for something more explicit but this would change the function signatures.

Both points are of course open for discussion!

@kMutagene
Copy link
Member

@joott , Can you have a lookat this?

@caroott
Copy link
Member

caroott commented Jun 29, 2020

The fix for the flattening of the array looks good to me. Where I'm not exactly sure how to handle it is the "backup" separator. Currently there is a list with commonly used separators and the first one that isn't the separator used for writing is selected. As far as i can see it, only the first two separators in this list are able to be selected, so the third one is obsolete. If this is the course we want to take maybe we should pick two separators only. Another option would be to add an optional parameter for the backup separator, but this would require some rewriting of the functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants