Here's a proposed change to your codebase: #6
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
Owner
JacobBorden
commented
May 28, 2025
JacobBorden
commented
``` docs: Update CHANGELOG and README for new BmpTool API Updates documentation to reflect the addition of the new span-based BmpTool API. - CHANGELOG.md: Added an entry for version 0.3.0 detailing the new API layer, its functions (`BmpTool::load`, `BmpTool::save`), the `BmpTool::Bitmap` struct, the roundtrip test, and related build system changes. - README.md: Added a new section describing the `BmpTool` API, its components, and a usage example for `BmpTool::load` and `BmpTool::save` using memory spans. Clarified the location of the public header (`include/bitmap.hpp`). ```
Addresses build errors you encountered after setting C++ standard to 20. The errors indicated issues with templated constructor declarations in `src/matrix/matrix.h`. - Changed `Matrix<T>() = default;` to `Matrix() = default;`. - Changed `explicit Matrix<T>(...)` to `explicit Matrix(...)`. - Changed `Matrix<T>(...)` to `Matrix(...)`. This uses the injected-class-name for constructors within the class template definition, which is standard C++ and resolves the parsing issues encountered by the compiler under C++20.
Addresses build errors identified in your feedback:
1. Matrix method calls:
- Updated `BmpTool::load` and `BmpTool::save` in `src/format/bitmap.cpp`
to use the correct method names for the `Matrix` class (e.g.,
`cols()` instead of `Width()`, `rows()` instead of `Height()`,
`at(r,c)` instead of `Get(c,r)` or `Set(c,r,v)`).
- Adjusted `Matrix` constructor call in `BmpTool::save` to
`Matrix(rows, cols)`.
2. Result<void, E> handling:
- Introduced `BmpTool::Success` struct in `include/bitmap.hpp`.
- Added a template specialization for `BmpTool::Result<void, E>`
that internally uses `std::variant<BmpTool::Success, E>` to avoid
issues with `std::variant<void, E>`.
- Modified `BmpTool::save` in `src/format/bitmap.cpp` to return
`BmpTool::Success{}` upon successful completion.
- Updated `tests/api_roundtrip.cpp` to correctly check for success
from `BmpTool::save` calls (using `isSuccess()` rather than checking
the error value).
These changes should resolve the compilation errors related to incorrect
Matrix method names and the use of `void` with `std::variant` in the
Result class.
Refactors `tests/api_roundtrip.cpp` to use the Google Test framework, removing its standalone `main` function. This resolves linker errors caused by multiple `main` function definitions when linking with `tests_main.cpp` (which provides the main for GTest via `gtest_main`). - `tests/api_roundtrip.cpp` now uses `TEST()` macro and GTest assertions. - `tests/CMakeLists.txt` was previously confirmed to correctly include `api_roundtrip.cpp` and link against `gtest_main`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.