Drop the CRLF conversion and cover the CRLF path with a fixture - #228
Draft
webdevred wants to merge 1 commit into
Draft
Drop the CRLF conversion and cover the CRLF path with a fixture#228webdevred wants to merge 1 commit into
webdevred wants to merge 1 commit into
Conversation
Converting the shipped examples buys nothing measurable: the rulesets parse identically either way, the tool already keeps the line endings a user's own file came with, and BeamNG ships 150 LF files of 4943 itself. Meanwhile no fixture had a carriage return in it, so the suite never exercised CRLF on a format whose real files mostly are.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The release converted the shipped example files to CRLF. It is not worth keeping. The rulesets parse to the same thing either way, formatting a
.jbeamfile already keeps whatever line endings that file came with, and BeamNG is not consistent about it itself: 150 of the 4943 jbeam files in the stock vehicles are LF, so the game plainly reads them. The script had also drifted from what the packaging step copies, converting a directory that ships nowhere while leaving the rulesets that go into both the installer and the zip alone.What the LF-only fixtures did cost is coverage. Nothing in the ordinary suite contained a carriage return, on a format whose real files are 97 per cent CRLF, and the parser has had a CRLF-specific bug in a block comment before. One fixture now carries CRLF deliberately, pinned as binary so no checkout normalises it away, and the spec asserts that the same file read both ways parses to the same tree. It also checks the fixture still has its carriage returns, so it cannot pass vacuously.
.gitattributesarrives with it, which the repository has never had. Until now the line endings in a working tree depended on each developer'score.autocrlf, which is the trapfixedPointSpecalready carries a workaround for.Still to come on this branch, which is why it is a draft:
dump_astdetects the line endings of the file it is about to overwrite and writes them back. With the conversion gone it can drop that and always write LF, which also removes awriteFilewhose text-mode translation would double the carriage returns on Windows.