Just came across some slightly confusing LineRewriter behavior when looking into PR feedback #5771 (comment)
I don't think it's high priority, but would be nice to clarify at some point.
Summary
LineRewriter.IsRewriteConsoleLineSupported probes console capability by calling SetCursorPosition(0, LineToClear), but caches the result in a static field. Since LineToClear is a mutable, instance-specific property, the cached result reflects whether a specific row is valid — not whether the console supports cursor repositioning in general.
Current behavior
// LineRewriter.cs
public int LineToClear { get; set; } // mutable, instance-specific
private static bool? _isSetCursorPositionSupported; // cached globally
public bool IsRewriteConsoleLineSupported
{
get => _isSetCursorPositionSupported ?? EnsureInitialized();
bool EnsureInitialized()
{
try
{
Console.SetCursorPosition(0, LineToClear); // probes with instance value
_isSetCursorPositionSupported = true; // caches globally
}
catch
{
_isSetCursorPositionSupported = false; // caches globally
}
}
}
If LineToClear is -1 (which happens when CursorTop is 0 in no-TTY environments), SetCursorPosition throws, and the static cache permanently records false — even if the console genuinely supports cursor repositioning and a subsequent LineRewriter instance has a valid LineToClear.
Why this doesn't cause issues today
dotnet-trace runs one command per process invocation. collect and collect-linux can't run in the same process. Each invocation creates a single LineRewriter instance with one LineToClear value. The static cache is effectively instance-scoped by accident.
Additionally, PR #5771 added a lineToClear < 0 guard in collect-linux's ProgressWriter that prevents probing with an invalid value.
Design questions
-
Should _isSetCursorPositionSupported be instance-scoped? The cached result depends on LineToClear, which is instance state. A static cache is only correct if all instances share the same LineToClear — which happens to be true today but isn't guaranteed by the API.
-
Should the probe test console capability independently of LineToClear? For example, probing with the current cursor position (SetCursorPosition(left, top)) would test "can this console reposition at all?" without conflating it with "is this specific row valid?" But this changes the semantics — collect relies on the probe to validate its specific LineToClear value.
-
Should LineToClear be immutable? It's set once after construction in both callers (collect and collect-linux), with one exception: collect does LineToClear-- as a one-time adjustment when the cursor is at the buffer bottom. Making it a constructor parameter would make the API harder to misuse.
-
Should RewriteConsoleLine() validate LineToClear internally? Currently callers must check IsRewriteConsoleLineSupported before calling RewriteConsoleLine(). If they don't, and LineToClear is invalid, it crashes. A self-guarding RewriteConsoleLine would be safer.
References
Just came across some slightly confusing LineRewriter behavior when looking into PR feedback #5771 (comment)
I don't think it's high priority, but would be nice to clarify at some point.
Summary
LineRewriter.IsRewriteConsoleLineSupportedprobes console capability by callingSetCursorPosition(0, LineToClear), but caches the result in astaticfield. SinceLineToClearis a mutable, instance-specific property, the cached result reflects whether a specific row is valid — not whether the console supports cursor repositioning in general.Current behavior
If
LineToClearis -1 (which happens whenCursorTopis 0 in no-TTY environments),SetCursorPositionthrows, and the static cache permanently recordsfalse— even if the console genuinely supports cursor repositioning and a subsequentLineRewriterinstance has a validLineToClear.Why this doesn't cause issues today
dotnet-traceruns one command per process invocation.collectandcollect-linuxcan't run in the same process. Each invocation creates a singleLineRewriterinstance with oneLineToClearvalue. The static cache is effectively instance-scoped by accident.Additionally, PR #5771 added a
lineToClear < 0guard incollect-linux'sProgressWriterthat prevents probing with an invalid value.Design questions
Should
_isSetCursorPositionSupportedbe instance-scoped? The cached result depends onLineToClear, which is instance state. A static cache is only correct if all instances share the sameLineToClear— which happens to be true today but isn't guaranteed by the API.Should the probe test console capability independently of
LineToClear? For example, probing with the current cursor position (SetCursorPosition(left, top)) would test "can this console reposition at all?" without conflating it with "is this specific row valid?" But this changes the semantics —collectrelies on the probe to validate its specificLineToClearvalue.Should
LineToClearbe immutable? It's set once after construction in both callers (collectandcollect-linux), with one exception:collectdoesLineToClear--as a one-time adjustment when the cursor is at the buffer bottom. Making it a constructor parameter would make the API harder to misuse.Should
RewriteConsoleLine()validateLineToClearinternally? Currently callers must checkIsRewriteConsoleLineSupportedbefore callingRewriteConsoleLine(). If they don't, andLineToClearis invalid, it crashes. A self-guardingRewriteConsoleLinewould be safer.References
IsRewriteConsoleLineSupported(static cache, probes withLineToClear)collect-linuxcrash whenCursorTop=0, addedlineToClear < 0guard inProgressWriter