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

Adding support for NotSet characterset in VS2010+ (#79) #718

Merged
merged 2 commits into from
Jun 13, 2017

Conversation

tvandijck
Copy link
Contributor

No description provided.

@samsinsane
Copy link
Member

I don't fully understand this API. Default is actually Unicode? I thought Default was always nothing, thus defaulting the value to whatever the action/toolset dictates? Also, just out of curiosity what does NotSet mean in VS? I thought it was just swapping between the A and W functions?

@tvandijck
Copy link
Contributor Author

Apparently there is more to is then just A vs W.... @jrepp might be able to explain

@starkos
Copy link
Member

starkos commented Mar 27, 2017

Default is actually Unicode? I thought Default was always nothing

This one has a bit of history, see this post to the developer forums. I have no objection to revisiting it though.

Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm…not feeling good about this one. I think I would feel better about breaking backward compatibility and using Default for this (which I should have done in the first place; my bad).

@tvandijck
Copy link
Contributor Author

FYI, Not Set is an actual setting, other then not specified, empty, which is what our "Default" does.

image

@starkos starkos dismissed their stale review March 28, 2017 17:25

Did more research; changing my mind

@starkos
Copy link
Member

starkos commented Mar 28, 2017

If you create a new C++ project via the VS project wizard (do we still call them "wizards"? why did we ever call them "wizards"?) in the IDE, it sets CharacterSet to Unicode. So technically our default value is right, from that perspective.

Best I can tell "NotSet" equates to an ASCII or Single-Byte Character Set. Perhaps we could use one of those, more informative names instead? That would make it easier to translate them to the other toolsets as well. What do you think?

@tvandijck
Copy link
Contributor Author

@starkos That sounds like an acceptable alternative yes, I'll adjust the PR, using "ASCII"

@tvandijck
Copy link
Contributor Author

changed, and added some tests.

@tvandijck tvandijck merged commit 6907f67 into premake:master Jun 13, 2017
@tvandijck tvandijck deleted the add-notset-characterset branch June 13, 2017 21:25
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.

4 participants