What's wrong
The constructor that takes a caller-supplied writer overwrites that writer's NewLine and never restores it (CodeBlocker/CodeBlocker.cs:162):
writer.NewLine = newLineString; // default: NewLines.Lf
IndentedTextWriter = new IndentedTextWriter(writer, indentString) { NewLine = newLineString };The docs for the TextWriter constructors (CodeBlocker.cs:115-118, the README and CLAUDE.md) say the writer "stays the caller's", and it is indeed not disposed. Its line terminator, though, is silently changed, and the change outlives the CodeBlocker. Anything else that writes to that writer afterwards gets \n instead of the terminator the caller configured.
Repro (HEAD d04adc9; it reproduces on Linux too, because the writer is set to CRLF explicitly):
var sw = new StringWriter { NewLine = "\r\n" };
using (var cb = new CodeBlocker(sw)) { cb.WriteLine("x"); }
Console.WriteLine(sw.NewLine == "\n"); // True: the caller's CRLF setting is gone
sw.WriteLine("caller line"); // writes "caller line\n", not "\r\n"Observed:
before: \r\n
after dispose: \n
caller output: x\ncaller line\n
Realistic cases:
new CodeBlocker(Console.Out): from then on, every Console.WriteLine in the process writes LF, including on Windows.- A build task passes in a
StreamWriter and writes its own header or footer lines with WriteLine. Those lines now use a different terminator than the one the task chose, so the file ends up with mixed line endings.
- Two
CodeBlockers share one writer but use different newLineStrings. The second one silently changes the terminator the first one writes.
Why it matters
#82 added the TextWriter constructor so generators could stream into writers they don't own. Changing shared state on such a writer is surprising, and it isn't documented. It also works against the reproducibility goal of #81: the terminator on the caller's own lines depends on whether a CodeBlocker was created first.
Suggested fix
Give IndentedTextWriter a private forwarding TextWriter. It would have its own NewLine/CoreNewLine and pass Write(char), Write(string), Flush and the rest through to the caller's writer. The caller's writer is then never changed, and the configured terminator is still used everywhere, netstandard2.0 included (which is why line 162 currently sets both writers). A smaller fallback is to save the original NewLine in the constructor and restore it in Dispose(bool). That fallback doesn't help callers who use the writer while the CodeBlocker is still alive.
Acceptance criteria
- After
new CodeBlocker(writer) and Dispose(), writer.NewLine has the value it had before construction.
- Output written through
CodeBlocker still uses NewLineString for every terminator, including those from Scope and NewLine().
- A test in
TextWriterTests pins both of the above with a writer whose NewLine is "\r\n", so the test also fails on Linux when the behaviour regresses.
What's wrong
The constructor that takes a caller-supplied writer overwrites that writer's
NewLineand never restores it (CodeBlocker/CodeBlocker.cs:162):The docs for the
TextWriterconstructors (CodeBlocker.cs:115-118, the README and CLAUDE.md) say the writer "stays the caller's", and it is indeed not disposed. Its line terminator, though, is silently changed, and the change outlives theCodeBlocker. Anything else that writes to that writer afterwards gets\ninstead of the terminator the caller configured.Repro (HEAD d04adc9; it reproduces on Linux too, because the writer is set to CRLF explicitly):
Observed:
Realistic cases:
new CodeBlocker(Console.Out): from then on, everyConsole.WriteLinein the process writes LF, including on Windows.StreamWriterand writes its own header or footer lines withWriteLine. Those lines now use a different terminator than the one the task chose, so the file ends up with mixed line endings.CodeBlockers share one writer but use differentnewLineStrings. The second one silently changes the terminator the first one writes.Why it matters
#82 added the
TextWriterconstructor so generators could stream into writers they don't own. Changing shared state on such a writer is surprising, and it isn't documented. It also works against the reproducibility goal of #81: the terminator on the caller's own lines depends on whether aCodeBlockerwas created first.Suggested fix
Give
IndentedTextWritera private forwardingTextWriter. It would have its ownNewLine/CoreNewLineand passWrite(char),Write(string),Flushand the rest through to the caller's writer. The caller's writer is then never changed, and the configured terminator is still used everywhere, netstandard2.0 included (which is why line 162 currently sets both writers). A smaller fallback is to save the originalNewLinein the constructor and restore it inDispose(bool). That fallback doesn't help callers who use the writer while theCodeBlockeris still alive.Acceptance criteria
new CodeBlocker(writer)andDispose(),writer.NewLinehas the value it had before construction.CodeBlockerstill usesNewLineStringfor every terminator, including those fromScopeandNewLine().TextWriterTestspins both of the above with a writer whoseNewLineis"\r\n", so the test also fails on Linux when the behaviour regresses.