-
-
Notifications
You must be signed in to change notification settings - Fork 202
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
Improve the handling of path string #528
Conversation
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.
This checks out
But make sure you test on Windows with a model path that has Unicode characters in it . This caused a bunch of problems in the past
I will check on Windows but it uses std::wstring on Windows. |
I have checked it on Windows having multi-byte characters on the path and noticed that the current path handling wouldn't work under some conditions so I have added a code to handle multi-byte characters properly. Please review again, @royshil |
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.
looks good!
Closes #525
Given
std::string s
,s.c_str()
will be released whens
is released. If you want to uses.c_str()
later, you have to holds
until you uses.c_str()
.On Windows, char paths are stored as multi-byte characters which Windows defines and it is hard to manipulate them with C++ standard libraries (codecvt was deprecated in C++17) so I propose to convert multi-byte characters with Windows-specific API that is guaranteed to work with multi-byte characters on Windows.