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

Improve csv functions by mentioning that enclosure character will be ignore #4190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions reference/filesystem/functions/fgetcsv.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
<para>
The <parameter>enclosure</parameter> parameter sets the field enclosure character.
It must be a single byte character.
If the CSV does not use an enclosure character, the parameter will be ignored.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this sentence is useful information. I mean, if the CSV does not use a separator (i.e. only one column), the separator will also be "ignored".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this sentence, because while upgrading a package from php 8.3 to 8.4, I was getting this error:

ValueError: str_getcsv(): Argument #3 ($enclosure) must be a single character

The code that triggers the error is:

$columns = str_getcsv($line, $this->separator, $this->enclosure, $this->escape);

So I started scratching my head and went back to reading the php doc which says:

2024-12-04_121007

So the current doc says you must set an enclosure character. Maybe for you it's obvious, that you can set it to any character, and, if the character is not present, it will be ignored. However it was not for me and I had to disturb the dev team by opening an issue on GitHub.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm aware of the background. :)

But like I've said, in the general case you cannot know whether enclosure characters are used (they are needed whenever the separator character or a line break are contained in a field value). And I've just checked latest LibreOffice Calc, and when importing a CSV, you need to choose between double- or single-quotes as enclosure character, even if the file is tab separated (and so rarely uses enclosure characters).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think adding the extra string will do harm?

</para>
</listitem>
</varlistentry>
Expand Down