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

Normalise line endings #99

Merged
merged 1 commit into from
Feb 1, 2020

Conversation

moretrim
Copy link
Contributor

Many country & province history files had mixed (CR/LF together with LF)
or inappropriate (i.e. lone CR) line endings. With this change all those
files use consistent line endings.

Something along the lines of git diff --ignore-space-at-eol Development is suggested to review the changes: it’ll take care of the
CR/LF noise by hiding all of it. This also goes some way into
demonstrating that most changes are intended conversions to native line
endings.

This should only leave lone CR changes in the diffs to review. (Unless
you are a time traveller using classic Mac OS I suppose. Hi!)


The province changes were tested (i.e. borders and some of the province data looks right once in the game). I also looked at the child labour and political rights of a couple countries to make sure they were correct, as there is a CR line ending change around these in some of the country files.

Many country & province history files had mixed (CR/LF together with LF)
or inappropriate (i.e. lone CR) line endings. With this change all those
files use consistent line endings.

Something along the lines of `git diff --ignore-space-at-eol
Development` is suggested to review the changes: it’ll take care of the
CR/LF noise by hiding all of it. This also goes some way into
demonstrating that most changes are intended conversions to native line
endings.

This should only leave lone CR changes in the diffs to review. (Unless
you are a time traveller using classic Mac OS I suppose. Hi!)
@moretrim
Copy link
Contributor Author

Obviously literally reviewing all files is out of the question. So together with the diff incantation here’s the other part of the puzzle to figure out the result is all right:

$ file HPM/**/*.{txt,csv} | grep -i 'terminator'
HPM/history/units/1861/NIP_oob.txt:                                    ASCII text, with no line terminators
HPM/news/news_battle_over_historical.txt:                              ASCII text, with no line terminators
HPM/localisation/00_HPM_decisions.csv:                                 Non-ISO extended-ASCII text, with very long lines, with LF, NEL line terminators
HPM/localisation/00_HPM_events.csv:                                    Non-ISO extended-ASCII text, with very long lines, with LF, NEL line terminators
HPM/localisation/00_HPM_News.csv:                                      Non-ISO extended-ASCII text, with very long lines, with LF, NEL line terminators
HPM/localisation/02_NNM_modifiers.csv:                                 Non-ISO extended-ASCII text, with very long lines, with LF, NEL line terminators
HPM/localisation/03_PDM_misc.csv:                                      ISO-8859 text, with LF, NEL line terminators

The first two files are described so because they consist of only one (non-terminated) line. The files that are alleged to contain mixed LF/NEL terminators really only contain LF terminators (expected from my Linux system), as well as a couple 0x85 characters which are really the ellipsis ('…') in the Windows-1252 encoding we expect from those localisation files.

@arkhometha arkhometha merged commit b7b9ace into arkhometha:Development Feb 1, 2020
arkhometha added a commit that referenced this pull request Feb 9, 2020
* Fix to FCS event chain firing wrong production events by @Libeccio-DD -  #120
* Fixes for the population of Saxony by @Libeccio-DD - #115
* Fix for the social/political movements icons by @Libeccio-DD - #121
* Work on normalizing line endings by @moretrim #99 - may need a review since the last round of script-made standardization to the files.
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