Skip to content

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Nov 21, 2025

Summary

Fixes the Rust implementation to preserve newlines (\n and \r) within block comments, matching the behavior of the JavaScript sindresorhus/strip-json-comments library.

Changes

Block Comments

  • Modified consume_block_comments() to iterate through characters and only replace non-newline characters with spaces
  • Updated maybe_comment_end() to preserve \n and \r characters

Line Comments

  • Modified consume_line_comments() to preserve \r when it appears immediately before \n (Windows line endings \r\n)

Test Results

All 48 tests pass, including the ported tests from sindresorhus that now correctly expect newlines to be preserved.

Before:

strip_string("{\"a\"/*\n\n\ncomment\r\n*/:\"b\"}")
// Output: "{\"a\"                :\"b\"}"  // newlines replaced with spaces ❌

After:

strip_string("{\"a\"/*\n\n\ncomment\r\n*/:\"b\"}")
// Output: "{\"a\"  \n\n\n       \r\n  :\"b\"}"  // newlines preserved ✅

Related

Builds on #118 which ported the test suite from the JavaScript implementation.

🤖 Generated with Claude Code

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 21, 2025

CodSpeed Performance Report

Merging #120 will degrade performances by 18.87%

Comparing fix/preserve-newlines-in-comments (5d255b2) with main (eab81e0)

Summary

❌ 4 regressions

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
large_with_comments 58.2 µs 68.5 µs -15.08%
minimal_comments 2.4 µs 3 µs -18.87%
no_comments 3.9 µs 4.3 µs -9.41%
tsconfig 19.2 µs 21.4 µs -10.43%

Replace byte-by-byte loops with efficient chunked filling:
- Use memchr::memchr2 to find newline positions
- Fill spaces in chunks between newlines using optimized fill()
- Preserves correctness while restoring performance

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
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.

2 participants