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

Remove support for CR #161

Merged
merged 1 commit into from
Aug 3, 2018
Merged

Remove support for CR #161

merged 1 commit into from
Aug 3, 2018

Conversation

stasm
Copy link
Contributor

@stasm stasm commented Aug 3, 2018

Fix #154.

The second commit normalizes the parsing of all supported line endings to LF. We already use LF to represent blank lines in multiline patterns. Normalizing line endings in the parsed results removes surprises and ensures consistency across platforms. Serializers can implement an option to serialize using CRLF if so desired.Moved to #163.

@Pike
Copy link
Contributor

Pike commented Aug 3, 2018

I'd like to take your offer to handle the CR-removal and the newline-normalization in two different issues.

The CR one is pretty simple, the line ending died long before we introduced fluent. There's maybe a tad of "I can use my old java properties files, 'cause they work", but really, no.

The newline normalization is something that deserves a deeper look, and maybe also a bit more external input. That seems like something in scope of an RFC.

@stasm
Copy link
Contributor Author

stasm commented Aug 3, 2018

Sounds good!

@stasm
Copy link
Contributor Author

stasm commented Aug 3, 2018

I removed the second commit from this PR and I'll file an RFC on Monday.

@stasm stasm requested a review from Pike August 3, 2018 16:39
Copy link
Contributor

@Pike Pike left a comment

Choose a reason for hiding this comment

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

Yes, I think that's good to do.

Changelog, please, with that, r=me.

@stasm stasm changed the title Remove support for CR and normalize all line endings to LF Remove support for CR Aug 3, 2018
@stasm stasm requested a review from Pike August 3, 2018 16:59
@stasm stasm merged commit c045c5c into projectfluent:master Aug 3, 2018
@stasm stasm deleted the remove-cr branch August 3, 2018 17:30
@stasm
Copy link
Contributor Author

stasm commented Aug 3, 2018

I filed #163 to discuss the normalization of EOLs.

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