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

Added Win32 UNICODE wmain support #903

Merged
merged 7 commits into from
May 11, 2017
Merged

Added Win32 UNICODE wmain support #903

merged 7 commits into from
May 11, 2017

Conversation

becrux
Copy link
Contributor

@becrux becrux commented May 6, 2017

Description

Added alternate main definition, available on Win32 platforms, when UNICODE
support is enabled. argv command line is a wchar_t, so Catch run interface should
be adapted in order to manage it.

@becrux
Copy link
Contributor Author

becrux commented May 6, 2017

In case it is accepted, I guess a specific build on AppVeyor should be added, with "/SUBSYSTEM:CONSOLE /ENTRY:wmainCRTStartup" as parameters. I could do that, if I'm allowed.

@horenmar
Copy link
Member

horenmar commented May 9, 2017

I not against the change, but I have to ask what is the intended benefits of these changes? Catch will not be able to work with utf-8 encoded arguments anyway (well, not at this point) and tests should be able to have UNICODE defined regardless of the entry point.

Is there anything else that also changes when wmain is used as entry point?

@becrux
Copy link
Contributor Author

becrux commented May 9, 2017

The problem occurs if someone uses wmainCRTStartup in its project, or MinGW-w64 GCC with -municode flag.

The are no benefits in terms of performance, or for the UNICODE support in the strict meaning (as you said), but it will save a developer from writing the same code that I wrote in order to adapt the wmain interface to the Catch::Session run method.

Short answer: it's just a convenience to have it working even when the above switches are ON, and you don't have to care about adapting the code, cause wmain has wchar_t **, and not char **, as argv type.

@horenmar
Copy link
Member

I took another look and if you add proper configurations to AppVeyor, Ill merge it in.

If we add proper utf-8 support to Catch we might to go back and add error checking for invalid conversion, but right now non-ascii character set might break things anyway, no need for malformed utf.

@becrux
Copy link
Contributor Author

becrux commented May 10, 2017

I've added proper configurations in appveyor.yml.

@becrux
Copy link
Contributor Author

becrux commented May 11, 2017

Thanks for fixing appveyor.yml!

@horenmar horenmar merged commit b8443e6 into catchorg:master May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants