Skip to content

Commit 065380d

Browse files
tigCopilot
andcommitted
Address review feedback: width cap, thread safety, initial height, docs
- Add MAX_RENDER_WIDTH (4096) upper bound to prevent OOM from absurd widths - Wrap env var set/restore in a static lock for thread safety - Use initial height of 1 instead of width for first layout pass - Fix XML docs to clarify ShowCopyButtons is always disabled for ANSI output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent dba6ef9 commit 065380d

1 file changed

Lines changed: 55 additions & 40 deletions

File tree

Terminal.Gui/Views/Markdown/MarkdownView.Ansi.cs

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ namespace Terminal.Gui.Views;
55

66
public partial class Markdown
77
{
8+
private const int MAX_RENDER_WIDTH = 4096;
9+
private static readonly Lock _renderToAnsiLock = new ();
10+
811
/// <summary>
912
/// Renders the current <see cref="Text"/> (or the supplied <paramref name="markdown"/>)
1013
/// to an ANSI escape-sequence string suitable for writing directly to a terminal.
@@ -13,7 +16,8 @@ public partial class Markdown
1316
/// Optional markdown text to render. If <see langword="null"/>, uses the current <see cref="Text"/>.
1417
/// </param>
1518
/// <param name="width">
16-
/// The target column width for word-wrapping. Defaults to 80.
19+
/// The target column width for word-wrapping. Defaults to 80. Clamped to
20+
/// the range [<see cref="MIN_WRAP_WIDTH"/>, 4096].
1721
/// </param>
1822
/// <returns>A string containing ANSI escape sequences that reproduce the styled markdown output.</returns>
1923
/// <remarks>
@@ -24,9 +28,10 @@ public partial class Markdown
2428
/// </para>
2529
/// <para>
2630
/// Configuration properties set on this instance — <see cref="SyntaxHighlighter"/>,
27-
/// <see cref="MarkdownPipeline"/>, <see cref="UseThemeBackground"/>,
28-
/// <see cref="ShowHeadingPrefix"/>, and <see cref="ShowCopyButtons"/> — are copied to the
29-
/// temporary view used for rendering.
31+
/// <see cref="MarkdownPipeline"/>, <see cref="UseThemeBackground"/>, and
32+
/// <see cref="ShowHeadingPrefix"/> — are copied to the temporary view used for rendering.
33+
/// Copy buttons (<see cref="ShowCopyButtons"/>) are always disabled for ANSI output because
34+
/// they are interactive-only controls.
3035
/// </para>
3136
/// </remarks>
3237
public string RenderToAnsi (string? markdown = null, int width = 80)
@@ -35,6 +40,10 @@ public string RenderToAnsi (string? markdown = null, int width = 80)
3540
{
3641
width = MIN_WRAP_WIDTH;
3742
}
43+
else if (width > MAX_RENDER_WIDTH)
44+
{
45+
width = MAX_RENDER_WIDTH;
46+
}
3847

3948
string text = markdown ?? Text;
4049

@@ -43,50 +52,56 @@ public string RenderToAnsi (string? markdown = null, int width = 80)
4352
return string.Empty;
4453
}
4554

46-
// Suppress real terminal I/O for the headless driver
47-
string? previousValue = Environment.GetEnvironmentVariable ("DisableRealDriverIO");
48-
Environment.SetEnvironmentVariable ("DisableRealDriverIO", "1");
49-
50-
try
55+
// A static lock guards the process-wide DisableRealDriverIO environment variable
56+
// so concurrent calls do not race on set/restore.
57+
lock (_renderToAnsiLock)
5158
{
52-
using IApplication app = Application.Create ().Init (DriverRegistry.Names.ANSI);
53-
app.Driver!.SetScreenSize (width, width);
59+
string? previousValue = Environment.GetEnvironmentVariable ("DisableRealDriverIO");
60+
Environment.SetEnvironmentVariable ("DisableRealDriverIO", "1");
5461

55-
using Markdown renderView = new ()
62+
try
5663
{
57-
Text = text,
58-
Width = Dim.Fill (),
59-
Height = Dim.Fill (),
60-
SyntaxHighlighter = SyntaxHighlighter,
61-
MarkdownPipeline = MarkdownPipeline,
62-
UseThemeBackground = UseThemeBackground,
63-
ShowHeadingPrefix = ShowHeadingPrefix,
64-
ShowCopyButtons = false // Copy buttons are interactive-only
65-
};
64+
using IApplication app = Application.Create ().Init (DriverRegistry.Names.ANSI);
6665

67-
renderView.App = app;
68-
renderView.SetRelativeLayout (app.Screen.Size);
69-
renderView.Layout ();
66+
// Use a small initial height for the first layout pass (just enough to compute content height)
67+
app.Driver!.SetScreenSize (width, 1);
7068

71-
int contentHeight = renderView.GetContentHeight ();
69+
using Markdown renderView = new ()
70+
{
71+
Text = text,
72+
Width = Dim.Fill (),
73+
Height = Dim.Fill (),
74+
SyntaxHighlighter = SyntaxHighlighter,
75+
MarkdownPipeline = MarkdownPipeline,
76+
UseThemeBackground = UseThemeBackground,
77+
ShowHeadingPrefix = ShowHeadingPrefix,
78+
ShowCopyButtons = false // Copy buttons are interactive-only
79+
};
7280

73-
if (contentHeight < 1)
74-
{
75-
return string.Empty;
76-
}
81+
renderView.App = app;
82+
renderView.SetRelativeLayout (app.Screen.Size);
83+
renderView.Layout ();
7784

78-
// Resize to the full content height so the entire document is drawn
79-
app.Driver.SetScreenSize (width, contentHeight);
80-
renderView.Frame = app.Screen with { X = 0, Y = 0 };
81-
renderView.Layout ();
82-
app.Driver.ClearContents ();
83-
renderView.Draw ();
85+
int contentHeight = renderView.GetContentHeight ();
8486

85-
return app.Driver.ToAnsi ();
86-
}
87-
finally
88-
{
89-
Environment.SetEnvironmentVariable ("DisableRealDriverIO", previousValue);
87+
if (contentHeight < 1)
88+
{
89+
return string.Empty;
90+
}
91+
92+
// Resize to the full content height so the entire document is drawn
93+
app.Driver.SetScreenSize (width, contentHeight);
94+
renderView.Frame = app.Screen with { X = 0, Y = 0 };
95+
renderView.Layout ();
96+
app.Driver.ClearContents ();
97+
renderView.Draw ();
98+
99+
return app.Driver.ToAnsi ();
100+
}
101+
finally
102+
{
103+
Environment.SetEnvironmentVariable ("DisableRealDriverIO", previousValue);
104+
}
90105
}
91106
}
92107
}

0 commit comments

Comments
 (0)