FEX-Emu / FEX

A fast usermode x86 and x86-64 emulator for Arm64 Linux
https://fex-emu.com
MIT License
2.36k stars 123 forks source link

Utils/StringUtil: Fixes ltrim and adds a unittest #4171

Open Sonicadvance1 opened 12 hours ago

Sonicadvance1 commented 12 hours ago

ltrim had the issue that it would always consume the left-most character even if it wasn't whitespace. So FEXLoader would turn in to EXLoader, even without any whitespace in the string.

Adds a test to ensure this doesn't occur again.

pmatos commented 10 hours ago

Nice - lgtm. :) We don't likely use this in any critical place; otherwise, I would wonder how anything works.

pmatos commented 9 hours ago

Is there a reason we are not using std::erase_if from c++20, instead of erase+find_if? https://en.cppreference.com/w/cpp/string/basic_string/erase2

Or the equivalent using std::remove_if, if we need to use iterators.

Sonicadvance1 commented 9 hours ago

Is there a reason we are not using std::erase_if from c++20, instead of erase+find_if? https://en.cppreference.com/w/cpp/string/basic_string/erase2

Or the equivalent using std::remove_if, if we need to use iterators.

Haven't really looked at it, but I assume the predication would have different behaviour. I'm obviously not a very big C++ buff, using things for convenience rather than knowing every detail of it.

pmatos commented 7 hours ago

Is there a reason we are not using std::erase_if from c++20, instead of erase+find_if? https://en.cppreference.com/w/cpp/string/basic_string/erase2 Or the equivalent using std::remove_if, if we need to use iterators.

Haven't really looked at it, but I assume the predication would have different behaviour. I'm obviously not a very big C++ buff, using things for convenience rather than knowing every detail of it.

Ah, actually I have tried it and it wouldn't work because erase_if would remove all white space instead of just trimming it, so it seems like your approach is the best unless you want to get into views and ranges etc. But it's simpler as you have it. Sorry for the noise.