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

Fix removing trailing newlines by using str::replace in memofs #903

Merged
merged 2 commits into from
Apr 17, 2024
Merged

Fix removing trailing newlines by using str::replace in memofs #903

merged 2 commits into from
Apr 17, 2024

Conversation

kennethloeffler
Copy link
Member

This PR closes #899 by changing the implementation of Vfs::read_to_string_lf_normalized to use str::replace to replace \r\n with \n, rather than joining the result of str::lines.

This is also a small performance win because less work needs to be done when the file only contains LF line endings, and we never need to collect into an intermediate Vec<&str>.

@kennethloeffler kennethloeffler changed the title Fix removing trailing newlines by using String::replace in memofs Fix removing trailing newlines by using str::replace in memofs Apr 17, 2024
Dekkonot
Dekkonot approved these changes Apr 17, 2024
View reviewed changes
Copy link
Member

@Dekkonot Dekkonot 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.

Part of me wonders if we should add more tests just for competency's sake, but I imagine if str::replace fails, our other tests will fail quickly.

@Dekkonot Dekkonot merged commit 4b3470d into rojo-rbx:master Apr 17, 2024
6 checks passed
@kennethloeffler kennethloeffler deleted the read-to-string-lf-normalized-keeps-trailing-newline branch April 17, 2024 18:56
Dekkonot added a commit to Dekkonot/rojo that referenced this pull request Jul 22, 2024
@Dekkonot
Backport rojo-rbx#903 to Rojo 7.4 (Fix removing trailing newlines)
7ead0a1
Dekkonot added a commit that referenced this pull request Jul 22, 2024
@Dekkonot
Backport #893 and #903 to Rojo 7.4 (#946)
3b72124 
As part of prep for a 7.4.2 release, this backports changes to the 7.4.X
branch that we can reasonably ship in 7.4.2 without too many code
changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dekkonot Dekkonot Dekkonot 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.

Rojo unexpectedly trims trailing newline characters when syncing to Studio.
2 participants
@kennethloeffler @Dekkonot