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

fix parsing CRLF files, part 3 #5

Merged
merged 1 commit into from
Sep 26, 2021
Merged

Conversation

warpfork
Copy link
Owner

Nothing can be easy or nice, can it.

Dogpiling on #4 and #3 . I think this is addressing the last of the things we've discovered through that journey.

Most of the diff is just me commenting crankily, as is natural.

On the plus side, some tests are unskipped now and actually hold up some standards and expectations. That's nice.

Nothing can be easy or nice, can it.

On the plus side, some tests are unskipped now and actually hold up
some standards and expectations.  That's nice.
func normalizeEndings(in []byte) []byte {
if bytes.Count(in, sigilCrLf) == 0 {
return in
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine bytes.Replace is already smart enough to return the input unchanged if there are no matches

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oddly enough, it does specifically the opposite: it always returns a copy.

(I initially had the same expectation you did!)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it's documented to "return a copy", so that makes sense. Weird docs though.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Agreed.

@warpfork warpfork merged commit 91a00c1 into master Sep 26, 2021
@warpfork warpfork deleted the normalizing-linebreaks-pt3 branch September 26, 2021 20:01
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
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