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

Strip carriage returns from lines in settingtypes.txt #11338

Merged
merged 2 commits into from Jun 21, 2021

Conversation

neoh4x0r
Copy link
Contributor

@neoh4x0r neoh4x0r commented Jun 12, 2021

Fixes issue #11327
Carriage returns should be stripped when reading lines from settingstypes.txt.

Example of Biofuel's settingtypes.txt file cause the problem:
https://github.com/Lokrates/Biofuel/blob/master/settingtypes.txt

2021-06-09 08:18:04: ERROR[Main]: Invalid integer setting in /home/sfence/.minetest/mods/biofuel/settingtypes.txt "fuel_production_time (fuel production time) i"t 10 5 360
2021-06-09 08:18:04: ERROR[Main]: Invalid integer setting in /home/sfence/.minet"st/mods/biofuel/settingtypes.txt "biomass_input (required input) int 4 1 99
2021-06-09 08:18:04: ERROR[Main]: Invalid boolean setting in /home/sfence/.minet"st/mods/biofuel/settingtypes.txt "refinery_output (bottle output) bool false
2021-06-09 08:18:04: ERROR[Main]: Invalid boolean setting in /home/sfence/.minet"st/mods/biofuel/settingtypes.txt "food_fuel (food waste) bool false

Linux uses LF (0x0A, /n) for line endings, but windows uses CRLF (0x0D0A, /r/n).
Lua strips the LF (which is fine on linux, if only using LF), but when the file uses CRLF, the CR gets left behind.

I suspect that when this happens on a windows system -- lua will strip the CRLF completely.

The best solution (cross-platform) is just to remove any stray carriage returns.

…settingstypes.txt

  ** solves issue of CRLF breaking settings.
@neoh4x0r neoh4x0r changed the title + fixes issues #11327 -- strip carriage returns from line in settingtypes.txt Resolve issue #11327 -- strip carriage returns from line in settingtypes.txt Jun 12, 2021
@SmallJoker SmallJoker added @ Builtin Bugfix 🐛 PRs that fix a bug Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines labels Jun 12, 2021
@sfan5 sfan5 changed the title Resolve issue #11327 -- strip carriage returns from line in settingtypes.txt Strip carriage returns from lines in settingtypes.txt Jun 20, 2021
@rubenwardy
Copy link
Member

rubenwardy commented Jun 21, 2021

I don't get why *line doesn't already handle \r\n correctly, I guess it's not written in a truly platform independent way

Edit:

So, Lua's file:read("*line") uses setvbuf with a mode of _IOLBF to read lines. Not sure what the behaviour of this is exactly

Lua's io.lines([file]) uses a custom implementation that only looks for \n

Yay, consistency

I wonder if it worth having our own iterator to go over lines, and looking for other instances of *line and io.lines()

@rubenwardy
Copy link
Member

Surprisingly, this is the only place that manually handles reading lines in the Lua builtin. So I'm fine with this fix

Ideally, this should be fixed in the Lua API as it's complete nonsense that the lines iterator doesn't support \r\n

@rubenwardy rubenwardy merged commit 9d2e7fc into minetest:master Jun 21, 2021
@rubenwardy rubenwardy linked an issue Jun 21, 2021 that may be closed by this pull request
0xLiso pushed a commit to 0xLiso/minetest-shadowmap that referenced this pull request Jun 25, 2021
Co-authored-by: SmallJoker <SmallJoker@users.noreply.github.com>
0xLiso pushed a commit to 0xLiso/minetest-shadowmap that referenced this pull request Jun 25, 2021
Co-authored-by: SmallJoker <SmallJoker@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Builtin Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settingtype.txt line ending
3 participants