Skip to content

Preserve quoted whitespace when saving with trimming enabled - #226

Open
fhgffy wants to merge 1 commit into
d99kris:masterfrom
fhgffy:fix-trimmed-whitespace-roundtrip
Open

fhgffy wants to merge 1 commit into
d99kris:masterfrom
fhgffy:fix-trimmed-whitespace-roundtrip

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 4, 2026

Copy link
Copy Markdown

Problem

With trimming enabled, CSV fields containing quoted leading or trailing tabs survive the initial parse, but Save emits those values without quotes. Loading that saved CSV with the same parameters strips the tabs and changes the cell value. The same applies to vertical tabs and form feeds.

For example, reading "\tvalue\t"\n with SeparatorParams(',', true, false) produces a seven-byte value; after Save/reload it becomes five bytes on master cbd8a0a.

Change

When trimming is enabled, also quote cells whose first or last byte is whitespace according to the same unsigned-char isspace classification used by Trim. Existing output stays unchanged when trimming is disabled, whitespace is internal, or automatic quoting is disabled.

The regression covers all three whitespace characters, leading/trailing/only/internal positions, trimming on/off, default/custom quote characters, exact saved bytes and reload integrity. Empty, UTF-8 and AutoQuote-disabled cases are included.

Validation

  • New regression fails on the unmodified base and passes with the fix
  • CMake Debug/Ninja: all 113 unit tests pass on Linux, GCC 14.2, C++11
  • CMake Release/Ninja: both performance tests pass
  • Regression passes with AddressSanitizer and UndefinedBehaviorSanitizer; leak detection disabled
  • Additional 36,864-case byte/locale/separator matrix: 144 failures on the baseline, zero failures with the fix under ASan+UBSan
  • Upstream warning flags produce no warnings for the regression
  • git diff --check passes

Uncrustify and other compiler/platform matrices were not run. Leak checking was disabled because LeakSanitizer cannot run under this environment’s ptrace configuration.

@fhgffy
fhgffy marked this pull request as ready for review October 4, 2026 09:51
@d99kris d99kris self-assigned this Oct 4, 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.

2 participants