search

KoffeinFlummi / armake2

Successor to armake written in Rust
GNU General Public License v2.0
50 stars 17 forks source link

Fixed two more CRLF-issues #29

Closed Krzmbrzl closed 5 years ago

Krzmbrzl commented 5 years ago

Okay I just saw that I messed things up - Should have compiled it on my machine before blindly creating a PR xD

The problem seems to be that the replace-method actually changes the String (other than e.g. in Java) and is thus not permitted on immutable objects.

Krzmbrzl commented 5 years ago

Alright I just found the lines() function that splits a string into lines and takes care of the LF vs CRLF problem automatically. I will update some more code to use this function as this is actually more readably and doesn't require extra hacking on our side...

JonBons commented 5 years ago

Status on this? It looks like the appveyor build failed due to OpenSSL which I believe was fixed by https://github.com/KoffeinFlummi/armake2/pull/37.

You can probably use the project files I linked in https://github.com/synixebrett/HEMTT/issues/109 in order to test these changes.

Krzmbrzl commented 5 years ago

This should work fine - and it does fail because of the AppVeyor OpenSSL thing. Which actually got fixed in the master branch already - but thanks anyways :)

Krzmbrzl commented 5 years ago

I will do the checking once I have the time for it - can't really give an ETA though

Krzmbrzl commented 5 years ago

@JonBons The problem is not fixed with this PR - see #39

Krzmbrzl commented 5 years ago

The used lines() function actually doesn't handle old mac-style newlines (single \r)

Recent events showed that some people still use that format though. Not sure if this makes it worth bothering with it...

KoffeinFlummi commented 5 years ago

Merged because lines() is cleaner than manual string handling. In my opinion handling old Mac newlines is somewhat ridiculous. The change is ancient, and in my opinion it is not worth to make the code uglier again (by reverting to manual string handling) to handle this for one user. However, the preprocessor should probably error out cleanly if those newlines are not supported.