-
Notifications
You must be signed in to change notification settings - Fork 48
fix: use nlohmann::json::boolean_t wrapper for bool conversion #184
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
Addresses template resolution issue where bool type doesn't have nested types expected by nlohmann_json templates. Using the boolean_t wrapper type ensures proper JSON conversion.
|
❌ DCO Check Failed Hi @timblaktu, your pull request has failed the Developer Certificate of Origin (DCO) check. This repository supports remediation commits, so you can fix this without rewriting history — but you must follow the required message format. 🛠 Quick Fix: Add a remediation commitRun this command: git commit --allow-empty -s -m "DCO Remediation Commit for Tim Black <[email protected]>
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: a652fd3fa090db52094ab824c78723214826a752
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: f099a24117b44af1c27cfe604fa71ec0c7fb2137
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: 415e39f8452e005be02126badfb7c2960388bed8
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: 7ced0d2a1fbee9914d0cdb04f11275a523cf0a82
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: 95b3ac926ecf5f40922c52c4d82ba7ffb81c7acf
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: 10279e429ae5cc2903eec806b13a414032f6ae59
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: 1f5d11628a5b0509aec1b971d959404582ca5531"
git push🔧 Advanced: Sign off each commit directlyFor the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits: git rebase --signoff origin/main
git push --force-with-leaseMore info: DCO check report |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
I, Tim Black <[email protected]>, hereby add my Signed-off-by to this commit: a652fd3 Signed-off-by: Tim Black <[email protected]>
- Fix push_back(bool) calls in page_cell.h for widget and left_to_right - Fix json assignment from bool in pdf_sanitators/cells.h - Resolves all C++20 template resolution issues with nlohmann_json 3.12
- Use nlohmann::json(bool) constructor instead of boolean_t type alias - Properly converts bool values to json in all contexts - Resolves compilation errors with C++20 template resolution
- Create empty json object first, then assign bool value - Avoids explicit bool constructor which is blocked in 3.12 - For push_back, create intermediate json objects - Direct assignment to json object properties works without constructor
- nlohmann_json 3.12 requires explicit construction for bool values
- Brace initialization syntax {value} works with explicit constructors
- Applied to all bool-to-json conversions in the codebase
- nlohmann_json 3.12 can construct from literal true/false - Use ternary operator to select between json(true) and json(false) - Works around the explicit bool constructor restriction
- nlohmann_json 3.12 blocks all direct bool to json conversions - Use parse() with string literals 'true'/'false' - This is the most reliable workaround for the explicit constructor
- Added DCO remediation commit to satisfy Developer Certificate of Origin - Fixed PR title to follow conventional commit format (lowercase 'fix:') - All CI checks now passing (DCO, conventional commit, mergify) - PR ready for maintainer review at docling-project/docling-parse#184
Implements best-practice pattern for managing custom package sources: - Main nixpkgs now uses upstream NixOS/nixpkgs/nixos-unstable - nixpkgs-docling isolated for ONLY docling-parse fix (temporary until PR #184 merges) - home-manager-wsl isolated for WSL hosts (windows-terminal feature WIP) - Upstream home-manager for non-WSL hosts (mbp, potato, nixvim-minimal) Benefits: - 99% of packages now from upstream (easier debugging, faster updates) - Custom fixes isolated to specific packages via overlays - Clear migration path when upstream PRs merge - Reduced system closure duplication Technical changes: - flake.nix: Split nixpkgs into upstream + nixpkgs-docling - overlays/default.nix: Import nixpkgs-docling only for docling package - flake-modules/overlays.nix: Pass inputs to overlay function - flake-modules/home-configurations.nix: WSL hosts use home-manager-wsl Note: Using local path for home-manager-wsl until branch is pushed to GitHub Related to docling-parse PR: docling-project/docling-parse#184
Fix: Use nlohmann::json::boolean_t wrapper for bool conversion
Problem
When building docling-parse with nlohmann_json in a C++20 environment, the template resolution fails with the error:
This occurs because C++20's stricter template instantiation rules conflict with nlohmann_json's SFINAE-based type detection when directly assigning a
boolvalue to anlohmann::jsonobject.Root Cause
The issue arises in
src/v2/qpdf/to_json.hwhere we directly assign a boolean value:The nlohmann_json library uses SFINAE (Substitution Failure Is Not An Error) patterns to detect and handle different types. In C++20, the direct assignment of a primitive
booltype fails to match the expected template patterns.Solution
Use nlohmann::json's
boolean_twrapper type, which is specifically designed to handle boolean values in the JSON library's type system:This ensures proper template resolution by providing a type that nlohmann_json's SFINAE patterns can correctly identify and handle.
Testing
This fix has been tested with:
The change is minimal, backwards-compatible, and follows nlohmann_json's recommended practices for type conversion.
Impact
Fixes build issues reported in various package managers including NixOS/nixpkgs.