[Windows] wchar_t for executable path

[dimitre]

There is a correction where Windows now uses wchart_t for returning executable name, so different codepages/encodings will be parsed correctly

[dimitre]

Can @artificiel or @oxillo review this one? Thank you

[artificiel]

I think std::filesystem::current_path() supersedes getCurrentExePath()?

also about std::filesystem::path::string() and the risk it throws an exception if non-convertible unicode is present in a windows path, do we have a test in place that can trigger that? such as https://developercommunity.visualstudio.com/t/stdfilesystempathgeneric-string-throws-an-exceptio/721120

and also decide if we let the exception flow knowing it's a transition (as ultimately no .string() calls should be needed when the nativeness of paths is preserved throughout) or try/catch all .string() calls and log errors.

[dimitre]

Thanks. I don't think current_path supersedes getCurrentExePath() , as we have this windows thing happening when opening file windows, and the current_path gets changed, so OF has defaultWorkingDirectory has to be called to set the path again.

Yeah we definetly need try/catch but we can just left without until transition is more complete. OF Core has plenty fs::path.string() out there. I'm trying to make some equivalent functions ending with FS to keep path more intact inside OF Core and what will be left are some addons and projects out there using string functions, backwards compatible.

[artificiel]

ah! but is getCurrentExePath()'s intended function to track the current path, or always return the original binary path on disk? the "current" portion of the name evokes "current working directory", but it is supposed to be more something like absolute_executable_path?

looking naively at the (crossplatform) code it looks as the function will always return the same value?

so why not store of::filesystem::current_path() at ofFileInit() in some immutable and use that for what getCurrentExePath() returns? the binary will not move during execution? or are there cases where getCurrentExePath() is not supposed to return the binary location?

[dimitre]

Yes immutable is a good idea As usual I propose this in another PR, so whenever possible one PR changes one thing and become one commit. If any regression is introduced it can be more precisely found by git bisect later.

What do you think of the change from char to wchart_t ?

[artificiel]

I'm not too familiar with the windows api and the LPSTR stuff but I presume it will pass 8-bit unicode into w_char? (that will then construct a native filesystem::path as the return type) — do we have a test in place with a windows unicode path? (we should have a test with valid windows unicode, as well as invalid like in the link above (to trigger the .string() calls). in all cases as the final solution of using filesystem::current_path() will directly provide a filesystem::path, so as long as this does not work worse than before I guess it's fine!

[ofTheo]

great idea @artificiel! adding a wide string / w_char test to the filesystem tests would be great to have to test wide string behavior.

[dimitre]

Due to the changes in previous PR this one is now really simplified just with the wchar changes, easier to review. I've updated the title and description to reflect the changes. Opinions are welcome @artificiel @c-mendoza @oxillo @ofTheo

[dimitre]

I think this one is good to go.

[danoli3]

Yes was testing some unicode paths in PG and yeah exception. Emojis for folder names kills it! haha

Okay sweet merging this

[danoli3]

Thanks @dimitre