Skip to content

Feature/fix line ending#7

Merged
Jeehut merged 3 commits intoFlineDev:stablefrom
phoney:feature/fix-line-ending
Aug 15, 2016
Merged

Feature/fix line ending#7
Jeehut merged 3 commits intoFlineDev:stablefrom
phoney:feature/fix-line-ending

Conversation

@phoney
Copy link
Copy Markdown
Contributor

@phoney phoney commented Aug 3, 2016

This pull request adds code to optionally sniff line endings of CSV files. The user can specify the line ending if they know it or let CSVImporter determine it automatically. Some test were added.

phoney added 3 commits August 3, 2016 15:09
Calling code can optionally specify the line ending. If they don't specify the line ending then it will be sniffed automatically. By passing the correct line ending to the streamReader() no line ending characters are included in the lines returned from the streamReader. Note that any files with mixed line endings will result in undefined behavior. But it was always like that.
If the correct lineEnding is set for the streamReader no line ending characters will be returned as part of lines. So there's no need to check for them in the line contents.
@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Aug 4, 2016

Looks good at a first glance, thanks! 👍

Will have a detailed look and merge soon.

@phoney
Copy link
Copy Markdown
Contributor Author

phoney commented Aug 4, 2016

OK, sounds great.

One comment: I made the lineEndingForFile() method private. It could be public.

@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Aug 15, 2016

I just had a detailed look and except for some code style issues (mostly wrongly set whitespaces) it is good. Merging it and tackling the code style changes later on. Good work! 👍

@Jeehut Jeehut merged commit fd9af0f into FlineDev:stable Aug 15, 2016
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

Successfully merging this pull request may close these issues.

2 participants