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

CSV export compliant with RFC4180 #8

Closed
halueda opened this issue Apr 5, 2019 · 5 comments
Closed

CSV export compliant with RFC4180 #8

halueda opened this issue Apr 5, 2019 · 5 comments

Comments

@halueda
Copy link
Contributor

halueda commented Apr 5, 2019

Release 1.4 document says

The CSV files exported are compatible with Excel. A limit of 32K is imposed on individual fields, beyond which they are truncated and terminated with '…'. New line characters within fields are replaced by semicolons. Fields which contain commas are wrapped in double quotation marks, and any double quotation marks within the field are replaced by single quotation marks.

I think RFC 4180 based conversion is better.
https://www.ietf.org/rfc/rfc4180.txt

  • if there is a new line in a column, then the field should be enclused in "" and no conversion about new line. It means one record can become multiple lines in CSV.
  • if there is a double quote in a column, then the field should be enclosed in "" and a double quote should be converted to two double quotes.

Excel and many libraries for csv can read such CSV file. And we don't have to worry about converting back ; to newline and ' to " .

halueda added a commit to halueda/XstReader that referenced this issue Apr 18, 2019
- No replacement about \r \n, instead the column is wrapped by "".
- A doublequote is replaced with two doublequotes.
- No commna replaced because it is not necessary.
@Dijji
Copy link
Owner

Dijji commented May 8, 2019

I specifically didn't want new line characters to create multiple rows in Excel, simply because I wanted to test it on large input sets, and creating additional rows made verification significantly harder.

Replacing " with "" might well make sense, though I would need to test it.

Do you have any specific reason for these requests?

Dijji

@halueda
Copy link
Contributor Author

halueda commented May 9, 2019

IMHO, exporting task should preserve as much information as possible.
Deleting newline changes meaning, especially original message header.

I understand about test, but excel and other CVS related tools can read without problem and
test with such tools is far better than checking with human eyes.

My patch was WIP and not checked, sorry.
Now I have correct source code to solve this issue, and soon commit it.
It is checked by exporting folders and reading by excel. The forlders have >10,000 messages including plain text and HTML with multilingual text in Japanese.
--HAL

@fabien-gigante
Copy link

I had the same need that @halueda, trying to preverse as much as possible all the values.

I believe the commit from @halueda is checking twice \n (instead of - I assume - checking \n and \r ) :

... value.Contains("\n") || value.Contains("\n") ...

Also for some reason, Excel doesn't open the CSV correctly, and requires the current locale separator instead of a plain comma. I'm not sure if Excel complies with the RFC 4180 with such a behavior. I had to locally modify the code like so to make it work for me. Even though this is probably not a change you want to incorporate officially (or through a setting only maybe).

        private void AddCsvValue(StringBuilder sb, string value, ref bool hasValue)
        {

            string comma = System.Globalization.CultureInfo.CurrentCulture.TextInfo.ListSeparator; // ",";
            if (hasValue) sb.Append(comma);
            if (value != null) 
            {
                 bool escape = value.Contains(comma) || value.Contains("\"") || value.Contains("\r") || value.Contains("\n");
                 if (escape) value = "\"" + value.Replace("\"", "\"\"") + "\"";
                 sb.Append(value);
            }
            hasValue = true;
        }

Dijji added a commit that referenced this issue May 21, 2020
@Dijji
Copy link
Owner

Dijji commented May 21, 2020

Thank you for reminding me of the work done by halueda on multilanguage support when exporting to csv files. I should have merged the changes long ago. So now I have. However, I did not include the changes to add the body in some form to the csv file, as there are other, higher fidelity, ways to get this information already.

I have also incorporated your change to make the comma culturally sensitive in the changes for the next release. I don't mind diverging somewhat from an RFC that is observed strictly almost nowhere, as I think that Excel is almost certainly the dominant scenario for reading such files.

Thank you for raising the issues that you encountered, and fixing them too!

Dijji

@Dijji
Copy link
Owner

Dijji commented May 21, 2020

Changes released as version 1.11

@halueda halueda closed this as completed May 22, 2020
just1fi3d pushed a commit to just1fi3d/XstReader that referenced this issue Jul 31, 2020
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

No branches or pull requests

3 participants