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

Added EOL detect / format #289

Merged
merged 5 commits into from
Apr 7, 2019
Merged

Added EOL detect / format #289

merged 5 commits into from
Apr 7, 2019

Conversation

zekth
Copy link
Contributor

@zekth zekth commented Mar 18, 2019

As files may be used in a Linux env but may come from a windows env, EOL is not the same. Added a FS helper for the detection / format. ATM it gets a string input, maybe coupled with readFileStr ( #276 ) in the future?

fs/eol.ts Outdated
if (!_eol) {
return content;
} else if (_eol === eol) {
return content;
Copy link

Choose a reason for hiding this comment

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

This doesn’t work if the file has mixed newlines, since detect only looks at the first newline.

Copy link
Contributor Author

@zekth zekth Mar 18, 2019

Choose a reason for hiding this comment

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

I was really wondering about this case (for detect). Is that really possible? And in this case, what would you do? Because the file is somehow invalid no? (fixing the mixed format)

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. LGTM

(You should probably have a test for what detect(Mixed) does but this is fine for now. Also I think this shouldn’t be in fs... I feel like we need a “encoding” directory or something.)

@ry ry merged commit 1d53fb4 into denoland:master Apr 7, 2019
@zekth
Copy link
Contributor Author

zekth commented Apr 7, 2019

I'll add the detect mixed in a further PR and let's discuss about the encoding directory in this PR.

@zekth zekth deleted the newline_detect branch April 7, 2019 08:38
@zekth zekth mentioned this pull request Apr 7, 2019
ry pushed a commit to ry/deno that referenced this pull request Oct 9, 2019
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.

3 participants