Skip to content
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

DolphinQt: Handle non-ASCII characters in Windows cmd arguments #9104

Merged
merged 2 commits into from Oct 19, 2020

Conversation

JosJuice
Copy link
Member

CommandLineParse expects UTF-8 strings. (QApplication, on the other hand, seems to be designed so that you can pass in the char** argv untouched on Windows and get proper Unicode handling.)

@shuffle2
Copy link
Contributor

shuffle2 commented Sep 21, 2020

...where are you seeing a problem?
https://code.woboq.org/qt5/qtbase/src/winmain/qtmain_win.cpp.html
(this is already handled by qtmain)

@shuffle2
Copy link
Contributor

I'm guessing you used cmake, and dolphin's cmake seems to be lacking this currently: https://stackoverflow.com/questions/14115024/how-to-link-qtmain-in-cmake-with-qt5

@JosJuice
Copy link
Member Author

WinMain is calling WideCharToMultiByte, which converts to the system codepage, i.e. not UTF-8.

My test case is placing a file named "Pokémon Channel.rvz" in the same folder as Dolphin and running Dolphin -e "Pokémon Channel.rvz" in cmd. It works in this PR but not in master.

I was not using CMake.

@shuffle2
Copy link
Contributor

OK, it seems an improvement would be to not use qtmain and only this PR, then?

@JosJuice
Copy link
Member Author

If qtmain doesn't have any purpose other than parsing command line arguments, then I suppose so. I'm not very familiar with what it does.

@shuffle2
Copy link
Contributor

Yea (link in my post above is the complete qtmain.lib sources).
To remove it from dolphin, just need to remove it from being linked and make #ifdef _WIN32 around the main definition to let windows build use a wWinMain instead.

CommandLineParse expects UTF-8 strings. (QApplication, on the
other hand, seems to be designed so that you can pass in the
char** argv untouched on Windows and get proper Unicode handling.)
@JosJuice
Copy link
Member Author

Does this look good?

@shuffle2
Copy link
Contributor

yea

@leoetlino leoetlino merged commit 9887534 into dolphin-emu:master Oct 19, 2020
10 checks passed
@MisakaMikotoDolphin
Copy link

Thank you! I had given up hope that this issue would be solved. Thanks again for fixing this niggling problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants