-
Notifications
You must be signed in to change notification settings - Fork 299
ostream and stringFormat simplifications #3298
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR simplifies various ostream printing routines by replacing legacy formatting code with concise stringFormat invocations while adding an include for image_int.hpp.
- Simplifies prints for rational values and flash compensation formatting.
- Uses new macros (EXV_PRINT_TAG, EXV_PRINT_TAG_BITMASK) to replace custom tag printing helper calls.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/tags_int.cpp | Added image_int.hpp include and refactored several print functions. |
src/olympusmn_int.cpp | Replaced direct tag printing calls with a macro for improved clarity. |
src/nikonmn_int.cpp | Streamlined print functions with early returns and stringFormat usage. |
Comments suppressed due to low confidence (1)
src/tags_int.cpp:3025
- The condition compares 'distance.first' against 0xfffffff; this appears to be a typo, as the original constant was 0xffffffff. Please verify the intended value.
if (static_cast<uint32_t>(distance.first) == 0xfffffff)
wow that is a subtle bug... |
204d787
to
e13602f
Compare
if (value.count() != 1 || value.typeId() != unsignedByte) | ||
return os << "(" << value << ")"; | ||
return EXV_PRINT_TAG(nikonFlashControlMode)(os, (value.toUint32() & 0x0F), metadata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code that saves and restores the flags has been removed. The same thing has happened in many other places in this pull request. Is there some reason why that code is not needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a lot of places, I’m using string format, which doesn’t need this.
Fixed bug with fmt < 8. |
compilation issue was discovered and fixed. Signed-off-by: Rosen Penev <[email protected]>
Signed-off-by: Rosen Penev <[email protected]>
Signed-off-by: Rosen Penev <[email protected]>
Simplifies code slightly. Signed-off-by: Rosen Penev <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3298 +/- ##
==========================================
- Coverage 64.09% 63.98% -0.12%
==========================================
Files 103 103
Lines 21696 21435 -261
Branches 10645 10604 -41
==========================================
- Hits 13907 13715 -192
+ Misses 5572 5507 -65
+ Partials 2217 2213 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.