Fix(windows): Resolve header conflicts and update tests #123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit replaces the direct inclusion of `<windows.h>` with a minimal set of manual forward declarations. This is a deliberate architectural change to: - **Improve Compilation Speed:** Avoids parsing the notoriously large Windows header. - **Eliminate Naming Pollution:** Prevents name clashes with common names like `min`/`max` which conflict with the C++ standard library. - **Enhance Encapsulation:** Makes the library more self-contained by not exposing the entire Windows API.
Previously, including both `subprocess.hpp` and `<Windows.h>` in the same file would cause compilation errors due to redefinitions of Windows API types and functions. This commit resolves these conflicts by wrapping the manual API declarations in `subprocess.hpp` with an `#ifndef _WINDEF_` guard. This ensures the library's lightweight declarations are only used if the official Windows headers haven't been included, making it safe to use both. The `test_ret_code.cc` unit test has been updated to include `<Windows.h>` directly for the `Sleep` function, which also serves to validate this fix.
Wraps the Windows.h include in a platform guard to prevent breaking non-Windows builds.
arun11299
approved these changes
Sep 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, including both
subprocess.hppand<Windows.h>in the same file would cause compilation errors due to redefinitions of Windows API types and functions.This commit resolves these conflicts by wrapping the manual API declarations in
subprocess.hppwith an#ifndef _WINDEF_guard. This ensures the library's lightweight declarations are only used if the official Windows headers haven't been included, making it safe to use both.The
test_ret_code.ccunit test has been updated to include<Windows.h>directly for theSleepfunction, which also serves to validate this fix.