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

Can not set empty enclosure for CSV #1509

Closed
matejjurancic opened this issue Jun 4, 2020 · 6 comments
Closed

Can not set empty enclosure for CSV #1509

matejjurancic opened this issue Jun 4, 2020 · 6 comments

Comments

@matejjurancic
Copy link

This is:

- [x] a bug report
- [ ] a feature request
- [x] **not** a usage question

What is the expected behavior?

When enclosure is set to empty string, no enclosure should be written to CSV.

What is the current behavior?

Empty enclosure is changed to double quotes in PhpOffice\PhpSpreadsheet\Writer\Csv::setEnclosure(). Code was recently changed from

if ($pValue == '') {
    $pValue = null;
}
$this->enclosure = $pValue;

to

$this->enclosure = $pValue ? $pValue : '"';

and now changes empty string to double quotes.

What are the steps to reproduce?

<?php

require __DIR__ . '/vendor/autoload.php';

$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
$writer = new \PhpOffice\PhpSpreadsheet\Writer\Csv($spreadsheet);
$writer->setEnclosure('');

Which versions of PhpSpreadsheet and PHP are affected?

1.13.0
Change has not been documented in the Changelog.

@oleibman
Copy link
Collaborator

oleibman commented Jun 9, 2020

My apologies - it was my pull request that introduced this change. It did not break any existing tests. and I don't understand your use case. Can you please give me a testable example of how you are using this so that I can revert this behavior and add an appropriate test.

@matejjurancic
Copy link
Author

I am actually using Laravel Excel, where the CSV configuration parameters are only passed to phpspreadsheet.
Basically all I want to do is generate a CSV file without enclosures, such as this:

2020-06-03;000123;06.53;14.22
2020-06-03;000234;07.12;15.44
...

I used to achieve this using $writer->setEnclosure('');, but this now sets enclosure to " and I get such content:

"2020-06-03";"000123";"06.53";"14.22"
"2020-06-03";"000234";"07.12";"15.44"
...

CSV files that my app generates are imported in another app. That app doesn't allow enclosures so files can't be imported. I know ... 🙄

@nickfls
Copy link

nickfls commented Jun 16, 2020

@oleibman are there any plans to merge it back any time soon?

@crnkovic
Copy link

I would love an update for plans to merge for this as well.

@acacha
Copy link

acacha commented Jun 19, 2020

Temporarily downgrading to 1.12.0 solves the issue

$ composer update
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals

  • Downgrading phpoffice/phpspreadsheet (1.13.0 => 1.12.0): Loading from cache

MarkBaker pushed a commit that referenced this issue Jun 19, 2020
* Fix For #1509

User expected no CSV enclosures after $writer->setEnclosure(''),
which had been changed to be consistent with $reader->setEnclosure('').
Writer will now omit enclosures after code above; no change to Reader.
Tests have been added for this condition.

* Add Option to Write CSV Enclosure Only When Required

Allowing the user to specify no enclosure when writing a CSV can lead to
a situation where PhpSpreadsheet (likewise Excel) will not read the
resulting file as intended, e.g. if any cell contains a delimiter character.
This is demonstrated in new test TestBadReread.
No existing setting will rectify this situation.

A better choice would be to add an option to write the enclosure
only when it is needed, which is what Excel does. The RFC4180 spec at
https://tools.ietf.org/html/rfc4180
states when it is needed - when the cell contains the delimiter,
or the enclosure, or a newline.
New test TestGoodReread demonstrates that the file is read as intended.

The documentation has been updated to describe the new function,
and to change the write example where the enclosure is set to null.

* Scrutinizer Suggestions

3 minor changes, all in tests.
@PowerKiKi
Copy link
Member

Closing as fixed in ce6ac1f

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

No branches or pull requests

6 participants