You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Review PR #196: restore original console writer after benchmarks #197
Review findings: PR #196 (restore original console writer after benchmarks)
Verdict: the fix is correct. I read the PR diff (head 6cbac64) and the current LeetCode/BenchmarkRunner.cs on main, which has the same content as the PR head (blob 0f2820e).
Captures the writer before muting.var consoleOut = Console.Out; runs before Console.SetOut(TextWriter.Null);, so the real writer is saved before it is replaced. This fixes the original bug, where Console.SetOut(Console.Out) ran after Console.Out had already become the null writer.
Restores in finally.Console.SetOut(consoleOut) runs even if AnsiConsole.Status()...Start(...) or RunBenchmarks throws, so a failing benchmark does not leave the console muted.
Scope is limited. Only the console writer is touched. Spectre.Console keeps its own writer, so the spinner and markup output are unaffected. The control flow around MoveCursor and the return value is unchanged.
Minor, non-blocking notes
summaries is declared before the try, so an exception leaves it partly filled. The exception still propagates, so this is harmless.
There is no automated test for the restore behaviour. The PR's test plan is manual (benchmark FizzBuzz --csharp). A unit test that checks Console.Out is the original writer after a throwing run would guard against regression, but it is optional.
Note on status: PR #196 was already merged by si618 at 2026-10-18T02:44:55Z. This review did not merge, approve, or change the PR.
Findings are based on reading the diff and the file. I did not build or run the benchmarks.
Activity
cbethin commentedon Oct 18, 2026
Review findings: PR #196 (restore original console writer after benchmarks)
Verdict: the fix is correct. I read the PR diff (head
6cbac64) and the currentLeetCode/BenchmarkRunner.csonmain, which has the same content as the PR head (blob0f2820e).var consoleOut = Console.Out;runs beforeConsole.SetOut(TextWriter.Null);, so the real writer is saved before it is replaced. This fixes the original bug, whereConsole.SetOut(Console.Out)ran afterConsole.Outhad already become the null writer.finally.Console.SetOut(consoleOut)runs even ifAnsiConsole.Status()...Start(...)orRunBenchmarksthrows, so a failing benchmark does not leave the console muted.MoveCursorand the return value is unchanged.Minor, non-blocking notes
summariesis declared before thetry, so an exception leaves it partly filled. The exception still propagates, so this is harmless.benchmark FizzBuzz --csharp). A unit test that checksConsole.Outis the original writer after a throwing run would guard against regression, but it is optional.Note on status: PR #196 was already merged by si618 at 2026-10-18T02:44:55Z. This review did not merge, approve, or change the PR.
Findings are based on reading the diff and the file. I did not build or run the benchmarks.