Skip to content

feat: add toStyledString overloads for custom builder and precision - #1617

Open
YackerYan wants to merge 2 commits into
open-source-parsers:masterfrom
YackerYan:master
Open

YackerYan wants to merge 2 commits into
open-source-parsers:masterfrom
YackerYan:master

Conversation

@YackerYan

@YackerYan YackerYan commented Aug 5, 2025 •

Copy link
Copy Markdown

Fixes #1616

Adds two Json::Value::toStyledString overloads:

  • toStyledString(const StreamWriterBuilder& builder): serialize using any builder settings (indentation, comment style, precision, etc.).
  • toStyledString(unsigned int precision, PrecisionType precisionType): shorthand for controlling floating-point output, in either decimal places or significant digits. Precision above 17 is capped, same as StreamWriterBuilder.

Notes:

  • The existing no-argument toStyledString() is unchanged, so the exported symbol and ABI are preserved.
  • When the builder sets commentStyle to "None", the leading newline normally added for a commentBefore comment is omitted.
  • Added unit tests ValueTest/toStyledStringWithBuilder and ValueTest/toStyledStringWithPrecision.

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds precision control overloads to JSON serialization.

The PR appears safe to merge; no new actionable defect or outstanding previous finding was identified.

Summary

The PR adds builder-configured and precision-configured Value::toStyledString overloads while retaining the no-argument method.

  • The latest change defaults a one-argument precision call to decimal-place mode.
  • Tests cover builder settings and both explicit precision modes.

Reviews (4) · Last reviewed commit: "Update include/json/value.h"

Comment thread include/json/value.h Outdated
Comment thread include/json/value.h Outdated
Comment thread src/lib_json/json_value.cpp Outdated
Comment thread src/lib_json/json_value.cpp
Comment thread include/json/value.h Outdated
Add Value::toStyledString(const StreamWriterBuilder&) and Value::toStyledString(unsigned int, PrecisionType) while keeping the existing no-argument overload for ABI compatibility. Skip the leading newline when the builder suppresses comments.

Fixes open-source-parsers#1616

Co-authored-by: Cursor <cursoragent@cursor.com>
@YackerYan

Copy link
Copy Markdown
Author

Thanks for the review! I've rebased onto the latest master and squashed everything into a single commit (4c61985). All five points are addressed:

  1. Incomplete builder breaks compilation: Removed the default argument. toStyledString(const StreamWriterBuilder&) is now a separate overload, so the forward declaration in forwards.h is enough and value.h compiles on its own.
  2. Existing library symbol disappears: The original String toStyledString() const; is kept unchanged, so the exported symbol is preserved. It now delegates to the builder overload with a default-constructed StreamWriterBuilder, so behavior is identical.
  3. Negative precision aborts writing: The precision parameter is now unsigned int, matching what StreamWriterBuilder reads via asUInt(). Values above 17 are already capped by newStreamWriter(), so the extra std::min was removed.
  4. Suppressed comments leave blank line: The leading newline is now only added when the root has a commentBefore and the builder's commentStyle is not "None".
  5. New overloads lack tests: Added ValueTest/toStyledStringWithBuilder (default output with comment, commentStyle = "None", compact indentation = "") and ValueTest/toStyledStringWithPrecision (decimal places, significant digits, and capping above 17).

All 134 unit tests and the reader/writer tests pass locally (MSVC), and the changed files are clang-format clean.

@YackerYan
YackerYan marked this pull request as draft September 29, 2026 07:22
@YackerYan
YackerYan marked this pull request as ready for review September 29, 2026 07:24
Comment thread include/json/value.h Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@YackerYan YackerYan changed the title feat(JSON): Add precision control parameters to toStyledString feat: add toStyledString overloads for custom builder and precision Sep 29, 2026

This branch has not been deployed

No deployments
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.

Enhance toStyledString to Support Custom Formatting, Decimal Precision, and Indentation Control

1 participant