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

Normalize CRLF mod description line endings to use LF instead #26

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

Su5eD
Copy link
Contributor

@Su5eD Su5eD commented Oct 4, 2023

The issue

Minecraft can't properly display CRLF line endings in its GUIs. This leads to them being shown as characters in mod descriptions on the NeoForge mod list screen, as described in the following issue

Expected behavior

Line ending characters should not be visible by any means.

The solution

When parsing mod descriptions, we replace all CRLF line endings with LF. By eliminating the root cause of this issue in FML, we ensure CRLFs will not show up anywhere in game, be it the neoforge mod list screen or other mod GUIs.

Fixes neoforged/NeoForge#104

Before After
broken crlf fancy lf

@TelepathicGrunt
Copy link
Contributor

Would this then resolve this report?
neoforged/NeoForge#104

@PaintNinja
Copy link
Contributor

I think this should also account for macOS which emits a \r without a \n

@Technici4n
Copy link
Member

I think this should also account for macOS which emits a \r without a \n

Really? Source?

@Su5eD
Copy link
Contributor Author

Su5eD commented Oct 4, 2023

Your memory is from the good old times though: Mac OS X, as POSIX-compliant Unix uses the typical Unix LF.
CR is a relict from the "classic" Mac OS, it's not used anymore.
https://superuser.com/a/439443

Well then I guess it doesn't...

@sciwhiz12
Copy link
Member

sciwhiz12 commented Oct 4, 2023

Note that the TOML spec itself (both v1.0.0 and v0.5.0, where NightConfig is stated to be compliant up to the latter former) only recognizes CR LF and CRLF as valid newline indicators, which aligns with Su5eD's finding (as shared via the Discord server) where the TOML parser rejects CR line endings. Therefore trying to account for the ancient CR line ending is moot.

Note to self: reread what you type, dingus.

@Technici4n Technici4n merged commit ea1e834 into neoforged:main Oct 16, 2023
@Su5eD Su5eD deleted the fix/crlf branch October 16, 2023 06:03
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.

Mod description shows line ending char
5 participants