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.

Sign up for GitHub

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

Jump to bottom

Remove the Unicode BOM, if present. (#97) #103

Merged
merged 2 commits into from
Aug 4, 2022
Merged

Remove the Unicode BOM, if present. (#97) #103

merged 2 commits into from
Aug 4, 2022

Conversation

lardieri
Copy link
Contributor

@lardieri lardieri commented Aug 2, 2022

CSV files saved by Excel and Airtable include the BOM when encoding to UTF-8.

@lardieri
Remove the Unicode BOM, if present. (#97)
35426c6 
CSV files saved by Excel and Airtable include the BOM when encoding to UTF-8.
DivineDominion
DivineDominion requested changes Aug 3, 2022
View reviewed changes
Copy link
Contributor

@DivineDominion DivineDominion left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, but I personally would prefer a change to the "bom" constant to make its meaning more obvious. What do you think?

Unrelated to that, would you want to merge your 0.6.2 fixes with this repo, too, so we can release that to CocoaPods proper?

SwiftCSV/CSV.swift Outdated Show resolved Hide resolved
SwiftCSVTests/utf8_with_bom.csv Show resolved Hide resolved
@lardieri
Copy link
Contributor Author

lardieri commented Aug 3, 2022

I've added additional documentation around the BOM.

I'm happy to submit 0.6.2 for release, but you will need to create a branch in your repo for me to open PR against, since it forks from a previous point (039cc27).

DivineDominion
DivineDominion approved these changes Aug 4, 2022
View reviewed changes
Copy link
Contributor

@DivineDominion DivineDominion left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for the doc comment!

@DivineDominion
Copy link
Contributor

DivineDominion commented Aug 4, 2022 •
edited
Loading

I've opened the branch v0.7. On top of the v0.6.1 release, it includes delimiter guessing, which was compatible with the v0.6 code. Will that be causing any trouble? Happy to discuss in another PR! :)

@DivineDominion DivineDominion merged commit 591d3e7 into swiftcsv:master Aug 4, 2022
@DivineDominion
Copy link
Contributor

Also, thanks a lot for contributing to SwiftCSV! We've invited you to join the SwiftCSV GitHub organization – no pressure to accept! If you'd like more information on what that means, check out our contributor guidelines.

@DivineDominion DivineDominion mentioned this pull request Aug 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DivineDominion DivineDominion DivineDominion approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lardieri @DivineDominion