-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
More fs::path inside core #7786
Conversation
ok one more ready to review. opinions are welcome @artificiel @ofTheo |
lots of stuff — phew! one small thing: in the freeimage the explicit #ifdef OF_OS_WINDOWS
retValue = FreeImage_SaveU(fif, bmp, fileName.wstring().c_str(), quality);
#else
retValue = FreeImage_Save(fif, bmp, fileName.string().c_str(), quality);
#endif but it can be made tighter by using #ifdef OF_OS_WINDOWS
retValue = FreeImage_SaveU(fif, bmp, fileName.c_str(), quality);
#else
retValue = FreeImage_Save(fif, bmp, fileName.c_str(), quality);
#endif (the example here is pretty much the use case!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha! that's a good one: plain auto uriStr = _fileName;
works because anyway c_str()
is called on it... uriStr
is actually superfluous and fileName
can be used directly in the code
I've thought the same but it failed in vs here: |
ah! yes if one thing to think about (in the larger project of accepting all possible windows unicode paths) is that anytime so with this in mind, freeimage stuff is good to go (because it handles wstring) but for instance xmlsettings (which does not support wstring paths) maybe: bool ofXml::save(const of::filesystem::path & file) const{
try {
// stuff
} catch (...) {
ofLogError("ofXml does not support windows wide unicode paths containing unconvertible characters");
}
} it seems hard to test as it depends on the encoding of the file, so simply pasting a test string might not trigger the exception (because some conversion would occur at the text editor point). this gives an example: but it would be good to install a test that sends such as "bad" string to the various path-handling functions and trigger problems. |
Great. I think we should identify on core where this kind of conversions happen and treat in a separate PR. |
ofPathToString PR here : |
The idea behind this PR is to start using fs::path directly converted to c_str() to library functions like FreeImage, FreeType etc, so the native encoding of OS is further preserved.