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

add /WX mscv compiler option for only mscv compiler #1495

Merged
merged 3 commits into from
Mar 28, 2020
Merged

add /WX mscv compiler option for only mscv compiler #1495

merged 3 commits into from
Mar 28, 2020

Conversation

trondhe
Copy link
Contributor

@trondhe trondhe commented Mar 27, 2020

clang on windows support both gcc and mscv style options. Clang.exe on windows
defaults to gcc style, which will result in /WX unknown compiler command.
This will set /WX if and only if the compiler is MSVC and greater than version 1900.

Could perhaps be done in a prettier way, but it solves the issue nicely.

clang on windows support both gcc and mscv style options. Clang.exe on windows
defaults to gcc style, which will result in /WX unknown compiler command.
This will set /WX if and only if the compiler is MSVC and greater than version 1900
@gabime
Copy link
Owner

gabime commented Mar 27, 2020

The msvc 2015 tests fail..

@trondhe
Copy link
Contributor Author

trondhe commented Mar 27, 2020

Ah the joys of CMake string handling. Using the list api is probably more sound regardless.
I'm not sure what the travil ci issue is though.

@@ -28,13 +28,15 @@ endfunction()

# Turn on warnings on the given target
function(spdlog_enable_warnings target_name)
Copy link
Owner

@gabime gabime Mar 28, 2020

Choose a reason for hiding this comment

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

Need to check if we under msvc before creating this list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made some changes. You think this is good?

Copy link
Owner

Choose a reason for hiding this comment

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

why all the whitespace changes all of the sudden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, sorry.
The code in the spdlog_enable_warnings function uses 4 spaces as indentation, but when I added the if statements for msvc, my vscode indented with tabs.
But it seems it did it that way to due mixed usage, didnt notice, so I just did a convert tabs to spaces inside the utils file.

I should probably revert the space/tabs thing and it should probably be done as a single pull request just changing tabs to 4 spaces or the other way around depending on your preference.

@gabime gabime merged commit 1ccdc22 into gabime:v1.x Mar 28, 2020
@gabime
Copy link
Owner

gabime commented Mar 28, 2020

Merged. Thanks.

bachittle pushed a commit to bachittle/spdlog that referenced this pull request Dec 22, 2022
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