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

Don't include headers in CMakeLists.txt after all #1008

Merged

Conversation

lmoureaux
Copy link
Contributor

The headers were added because Microsoft Visual Studio's "Solution" view
doesn't show the headers. However, VS' built-in CMake handling doesn't use the
solution view and is based on folders (much like CMake itself). There is no
recommendation from the CMake side on how to handle this problem with CMake's
built-in generators for Visual Studio.

Following the VS documentation and the instructions added in #999, one can get
(at least) decent VS support with IntelliSense working. This doesn't require
keeping the headers in the CMakeLists, so remove them for more idiomatic CMake
code.

This reverts part of commit 6e38a15.
See #1005.

The headers were added because Microsoft Visual Studio's "Solution" view
doesn't show the headers.  However, VS' built-in CMake handling doesn't use the
solution view and is based on folders (much like CMake itself).  There is no
recommendation from the CMake side on how to handle this problem with CMake's
built-in generators for Visual Studio.

Following the VS documentation and the instructions added in longturn#999, one can get
(at least) decent VS support with IntelliSense working.  This doesn't require
keeping the headers in the CMakeLists, so remove them for more idiomatic CMake
code.

This reverts part of commit 6e38a15.
See longturn#1005.
@jwrober jwrober self-requested a review May 10, 2022 20:58
Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

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

I see not change in behavior with or without the .h files included. I'm glad they are being removed.

@lmoureaux lmoureaux merged commit 4202588 into longturn:master May 10, 2022
@lmoureaux lmoureaux deleted the bugfix/remove-headers-from-cmake branch May 10, 2022 21:30
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Aug 22, 2022
When cancelling a network connection then going back to PAGE_NETWORK, the old
connection was still around and any attempt to connect would fail with
"Connection in progress". Cancel the connection when switching back to
PAGE_MAIN so we can create a new one when we go back to PAGE_NETWORK.

Might be related to longturn#785 or longturn#1008.
jwrober pushed a commit that referenced this pull request Aug 23, 2022
When cancelling a network connection then going back to PAGE_NETWORK, the old
connection was still around and any attempt to connect would fail with
"Connection in progress". Cancel the connection when switching back to
PAGE_MAIN so we can create a new one when we go back to PAGE_NETWORK.

Might be related to #785 or #1008.
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