surge-synthesizer / tuning-library

Micro-tuning format parsing and frequency finding as a header-only C+ library
https://surge-synth-team.org/tuning-library/
MIT License
83 stars 15 forks source link

stringRep ends with '\r' #37

Closed adamski closed 3 years ago

adamski commented 3 years ago

With some scl files, I assume those with Windows/DOS line endings, stringRep contains a string ending with \r

wilson5.scl.txt

baconpaul commented 3 years ago

for now at least you can run dos2unix on that file and it should load fine. Presume you know that but in case you don't!

adamski commented 3 years ago

Sure, also easy to fix in our project using the library, just thought I'd report here in case you were not aware of it.

baconpaul commented 3 years ago

I was not! Appreciate the flag and will definitely think about how to fix. I guess just strop \r if they show up is the standard approach. Thank you!

baconpaul commented 3 years ago

Basically I need to replace std::getline with something more like this https://gist.github.com/josephwb/df09e3a71679461fc104

And then add your file to our test suite (presume you are OK if I do that but want to check!)

adamski commented 3 years ago

The file was taken from the zip file of scales from the Scala website.

baconpaul commented 3 years ago

Great. Turns out we already had a test dos le in our test set; I just only checked the parsing not the stripping.

PR and change coming shortly. I'll update surge also so it uses it before the 19 release.

baconpaul commented 3 years ago

OK fix in place along with expanding the test some. Thanks!

baconpaul commented 3 years ago

Oooh one more tweak coming. All our tests in tuning-lib pass with this change but one surge test is failing. Stay tuned!

baconpaul commented 3 years ago

Sneaky extra line at end of file check failed and we test that in surge tests but not tuning lib tests. All good to go now. Updating surge also.