This repository has been archived by the owner on Jan 2, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Detect the presence of a sep=; line in CSV imports, and set the delim…
…iter accordingly
- Loading branch information
MarkBaker
committed
Mar 17, 2016
1 parent
a806b79
commit 79f9521
Showing
1 changed file
with
19 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
79f9521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->delimiter = substr($line, 4, 1);
Why is the delimiter assumed to be the 5th character?
79f9521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because
That's the standard when a sep= is used
What character would you expect it to be?
79f9521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the logic in the case where
sep=
is defined, but in the case wheresep=
isn't present, this seems to just pick the 5th character in the first row of data. I believe there should be a check for "sep=", something like:I had to add this check in order to get a "plain old" CSV file to parse (or maybe I'm just crazy)
79f9521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and that's been raised as an issue
I'm testing changes to it at the moment
79f9521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, my apologies! I didn't put enough effort into searching issues