Skip to content

refactor: use <+ template writing in place of write_string sequences - #4195

Closed
bobzhang wants to merge 2 commits into
mainfrom
hongbo/simplify-write
Closed

refactor: use <+ template writing in place of write_string sequences#4195
bobzhang wants to merge 2 commits into
mainfrom
hongbo/simplify-write

Conversation

@bobzhang

@bobzhang bobzhang commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Replaces literal and sequence StringBuilder/&Logger writes with the <+
template writer across Show implementations and streaming helpers, part of
the ongoing project-wide simplification from write_string sequences to
<+ template writing.

Changes:

  • builtin: BytesView Show and ToJson now share one
    BytesView::escape_to streaming helper; Char/StringView writes of
    non-printable \u{...} escapes collapse into a single template; Iter
    and Map Show stream through <+; a base64 == tail and a
    FixedArray iterator-test builder simplify.
  • diff: Range/HunkHeader Show stream via inline-writer holes.
  • hashmap, set: HashMap/Set Show collapse punctuation writes
    into <+ templates.
  • json: ParseError and the deprecated Json Show impls collapse
    write sequences into templates.
  • json_path: JSON Pointer rendering streams the recursive path through
    one template per step instead of interleaved write_* calls.
  • v128: zero-padded hex formatting streams through a new
    u64_hex_to helper used by the Show template.

\{l => ...} inline-writer holes preserve monomorphic write_object
dispatch, so output is unchanged throughout (full test suite: 7499/7499).

This re-applies the still-relevant content of the closed
#4110 (simplify-template-writing) on top of current main; the JSON
stringify/escape_to part of that PR was superseded by the merged
SIMD need_escape/write_escaped work, so it is intentionally not
re-applied.

Generated with SeekMoon

Replaces literal and sequence StringBuilder writes with the `<+` template
writer across Show implementations and streaming helpers:

- builtin: BytesView Show and ToJson now share one `escape_to` helper;
  Char/StringView non-printable `\u{...}` writes become single templates;
  Iter and Map Show stream through `<+`; a base64 `==` tail and a
  FixedArray test builder simplify
- diff, hashmap, json, set, v128: Show impls write through `<+` templates
  with inline-writer holes (`\{l => ...}`) preserving monomorphic
  write_object dispatch; v128 hex formatting streams via a new `u64_hex_to`

Generated interfaces refreshed for the new pub `BytesView::escape_to`.

Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>
Copilot AI lite review requested due to automatic review settings September 3, 2026 06:25
Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>
@coveralls

coveralls commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 6523

Coverage decreased (-0.02%) to 89.176%

Details

  • Coverage decreased (-0.02%) from the base build.
  • Patch coverage: 5 uncovered changes across 2 files (28 of 33 lines covered, 84.85%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
set/linked_hash_set.mbt 3 0 0.0%
builtin/iterator.mbt 2 0 0.0%
Total (10 files) 33 28 84.85%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 18163
Covered Lines: 16197
Line Coverage: 89.18%
Coverage Strength: 275405.03 hits per line

💛 - Coveralls

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It introduces a new exported builtin API surface (BytesView::escape_to) and related interface drift that appears inconsistent with the stated intent for internal-only helpers, and should be resolved before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Refactors a set of Show/streaming helpers to use the <+ template writer (and inline-writer holes where needed) instead of sequences of write_string/write_char calls, aiming to keep output stable while simplifying streaming code.

Changes:

  • Introduces/uses small streaming helpers (e.g., u64_hex_to, BytesView::escape_to) to write directly into a Logger.
  • Converts several Show implementations (and a few related helpers/tests) to emit punctuation and structured output via <+ templates.
  • Simplifies a few formatting/escaping sequences by collapsing them into single templates.
File summaries
File Description
v128/simd_basic.mbt Adds u64_hex_to and updates V128 Show to stream via <+ with inline-writer holes.
set/linked_hash_set.mbt Switches Set Show punctuation writes to <+.
json/types.mbt Collapses ParseError/deprecated Json Show output into <+ templates.
json/json_path.mbt Streams JSON Pointer token/path rendering using <+ templates.
hashmap/utils.mbt Refactors HashMap Show output to a single <+ template per entry.
diff/hunk.mbt Streams Range and HunkHeader Show output via <+ (with some interpolation changes).
builtin/show.mbt Collapses non-printable \u{...} escaping writes into a single template.
builtin/pkg.generated.mbti Updates generated builtin interface (currently includes BytesView::escape_to).
builtin/linked_hash_map.mbt Switches Map Show punctuation/object writes to <+ templates.
builtin/iterator.mbt Refactors Iter Show output to stream via <+.
builtin/fixedarray.mbt Simplifies an iterator test’s output builder using <+ interpolation.
builtin/console.mbt Collapses base64 padding writes into a single literal <+ "==".
builtin/char.mbt Collapses non-printable \u{...} escaping writes into a single template.
builtin/bytesview.mbt Introduces BytesView::escape_to helper and rewrites BytesView Show/ToJson to share it.
Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread builtin/bytesview.mbt
Comment on lines +349 to 353
/// Writes the bytes with printable ASCII kept as-is and everything else
/// rendered as `\xHH`, without the surrounding `b"`/`"`. Shared by `Show`,
/// which adds the quotes, and `ToJson`, which does not.
pub fn BytesView::escape_to(self : BytesView, logger : &Logger) -> Unit {
for byte in self {
Comment on lines 1171 to 1174
pub fn BytesView::equal(Self, Self) -> Bool
pub fn BytesView::equal_to_bytes(Self, Bytes) -> Bool
pub fn BytesView::escape_to(Self, &Logger) -> Unit
pub fn BytesView::find(Self, Self) -> Int?
Comment thread diff/hunk.mbt
Comment on lines 67 to 69
impl Show for HunkHeader with fn output(self, logger) {
logger.write_string("@@ -\{self.0} +\{self.1} @@")
logger <+ "@@ -\{self.0} +\{self.1} @@"
}
@bobzhang

bobzhang commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Splitting this into reviewable, per-area PRs as requested:

Each PR is independently verified and CI will run per PR. Closing this
umbrella PR in favor of the five above.

@bobzhang bobzhang closed this Sep 3, 2026
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.

3 participants