Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use E3 to completely clear console on Unix when possible #88487

Merged
merged 3 commits into from
Jul 7, 2023
Merged

Use E3 to completely clear console on Unix when possible #88487

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5939770
Use E3 to clear console on Unix when possible
natehitze Jul 6, 2023
832b6c4
Use IsNullOrEmpty instead of null check
natehitze Jul 6, 2023
4bc8eb6
Send E3 before clear and fix tests
natehitze Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libraries/System.Console/src/System/ConsolePal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ public static void Clear()
if (!Console.IsOutputRedirected)
{
WriteStdoutAnsiString(TerminalFormatStringsInstance.Clear);
if (!string.IsNullOrEmpty(TerminalFormatStringsInstance.ClearScrollbackBuffer))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to https://unix.stackexchange.com/a/375784 we should do E3 before clear:

If the terminfo/termcap entry has an E3 capability it, it first writes out that.
It then uses the clear capability to clear the visible screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snippet of clear shared in the issue had E3 being issued after clear, but I'm not sure it matters either way. I'll switch the order.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we manually validated the behavior on at least some system with the E3 and then clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub I had tested before, but unfortunately just assumed the switched order was okay per the linked answer. I just ran my manual test again and having E3 issued before clear does not clear the scrollback buffer.

image
image

That means this order needs reversed back to the original: Clear, then E3. Would you mind making that change on your PR? Or do you want me to submit a new PR with the optimization mentioned below + this fix?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind making that change on your PR?

No problem; I switched the order back.

{
WriteStdoutAnsiString(TerminalFormatStringsInstance.ClearScrollbackBuffer);
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Console/src/System/TerminalFormatStrings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ internal sealed class TerminalFormatStrings
public readonly string? Bell;
/// <summary>The format string to use to clear the terminal.</summary>
public readonly string? Clear;
/// <summary>The format string to use to clear the terminal scrollback buffer, if supported.</summary>
public readonly string? ClearScrollbackBuffer;
/// <summary>The format string to use to set the position of the cursor.</summary>
public readonly string? CursorAddress;
/// <summary>The format string to use to move the cursor to the left.</summary>
Expand Down Expand Up @@ -73,6 +75,7 @@ public TerminalFormatStrings(TermInfo.Database? db)
Reset = db.GetString(TermInfo.WellKnownStrings.OrigPairs) ?? db.GetString(TermInfo.WellKnownStrings.OrigColors);
Bell = db.GetString(TermInfo.WellKnownStrings.Bell);
Clear = db.GetString(TermInfo.WellKnownStrings.Clear);
ClearScrollbackBuffer = db.GetExtendedString("E3");
Columns = db.GetNumber(TermInfo.WellKnownNumbers.Columns);
Lines = db.GetNumber(TermInfo.WellKnownNumbers.Lines);
CursorVisible = db.GetString(TermInfo.WellKnownStrings.CursorVisible);
Expand Down
35 changes: 18 additions & 17 deletions src/libraries/System.Console/tests/TermInfo.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,22 @@ public void VerifyTermInfoSupportsNewAndLegacyNcurses()

[Theory]
[PlatformSpecific(TestPlatforms.AnyUnix)] // Tests TermInfo
[InlineData("xterm-256color", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0)]
[InlineData("xterm-256color", "\u001B\u005B\u00331m", "\u001B\u005B\u00341m", 1)]
[InlineData("xterm-256color", "\u001B\u005B90m", "\u001B\u005B100m", 8)]
[InlineData("screen", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0)]
[InlineData("screen", "\u001B\u005B\u00332m", "\u001B\u005B\u00342m", 2)]
[InlineData("screen", "\u001B\u005B\u00339m", "\u001B\u005B\u00349m", 9)]
[InlineData("Eterm", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0)]
[InlineData("Eterm", "\u001B\u005B\u00333m", "\u001B\u005B\u00343m", 3)]
[InlineData("Eterm", "\u001B\u005B\u003310m", "\u001B\u005B\u003410m", 10)]
[InlineData("wsvt25", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0)]
[InlineData("wsvt25", "\u001B\u005B\u00334m", "\u001B\u005B\u00344m", 4)]
[InlineData("wsvt25", "\u001B\u005B\u003311m", "\u001B\u005B\u003411m", 11)]
[InlineData("mach-color", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0)]
[InlineData("mach-color", "\u001B\u005B\u00335m", "\u001B\u005B\u00345m", 5)]
[InlineData("mach-color", "\u001B\u005B\u003312m", "\u001B\u005B\u003412m", 12)]
public void TermInfoVerification(string termToTest, string expectedForeground, string expectedBackground, int colorValue)
[InlineData("xterm-256color", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0, true)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test failures indicate that E3 is missing on xterm-256color, but only macOS. I wonder why: does macOS simply not provide it or it has a different name?

Please let me know if you need a copy of macOS terminfo file.

/private/tmp/helix/working/AFB20920/w/BFE50A43/e /private/tmp/helix/working/AFB20920/w/BFE50A43/e
  Discovering: System.Console.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Console.Tests (found 113 of 148 test cases)
  Starting:    System.Console.Tests (parallel test collections = on, max threads = 4)
    Color.RedirectedOutput_EnvVarSet_EmitsAnsiCodes [SKIP]
      Condition(s) not met: "TermIsSetAndRemoteExecutorIsSupported"
0
0
    TermInfoTests.TermInfoVerification(termToTest: "xterm-256color", expectedForeground: "\x1b[30m", expectedBackground: "\x1b[40m", colorValue: 0, expectedScrollbackClearSupport: True) [FAIL]
      Assert.Equal() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Console/tests/TermInfo.Unix.cs(102,0): at TermInfoTests.TermInfoVerification(String termToTest, String expectedForeground, String expectedBackground, Int32 colorValue, Boolean expectedScrollbackClearSupport)
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvoker.cs(59,0): at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
    TermInfoTests.TermInfoVerification(termToTest: "xterm-256color", expectedForeground: "\x1b[31m", expectedBackground: "\x1b[41m", colorValue: 1, expectedScrollbackClearSupport: True) [FAIL]
      Assert.Equal() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Console/tests/TermInfo.Unix.cs(102,0): at TermInfoTests.TermInfoVerification(String termToTest, String expectedForeground, String expectedBackground, Int32 colorValue, Boolean expectedScrollbackClearSupport)
           at InvokeStub_TermInfoTests.TermInfoVerification(Object, Object, IntPtr*)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
    TermInfoTests.TermInfoVerification(termToTest: "xterm-256color", expectedForeground: "\x1b[90m", expectedBackground: "\x1b[100m", colorValue: 8, expectedScrollbackClearSupport: True) [FAIL]
      Assert.Equal() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Console/tests/TermInfo.Unix.cs(102,0): at TermInfoTests.TermInfoVerification(String termToTest, String expectedForeground, String expectedBackground, Int32 colorValue, Boolean expectedScrollbackClearSupport)
           at InvokeStub_TermInfoTests.TermInfoVerification(Object, Object, IntPtr*)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
  Finished:    System.Console.Tests

Copy link
Contributor Author

@natehitze natehitze Jul 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://github.com/watchexec/clearscreen/blob/main/TERMINALS.md#platforms macOS doesn't include E3 in terminfo. That seems to be true on the mac I have access to -- and the clear command with xterm-256color does not clear the scrollback buffer but does clear it on my machine (PopOS 22.04) with xterm-256color.

I'll take a look at the Console.ReadKey tests you mentioned below and get the tests fixed up for CI. Thanks for reviewing!

[InlineData("xterm-256color", "\u001B\u005B\u00331m", "\u001B\u005B\u00341m", 1, true)]
[InlineData("xterm-256color", "\u001B\u005B90m", "\u001B\u005B100m", 8, true)]
[InlineData("screen", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0, false)]
[InlineData("screen", "\u001B\u005B\u00332m", "\u001B\u005B\u00342m", 2, false)]
[InlineData("screen", "\u001B\u005B\u00339m", "\u001B\u005B\u00349m", 9, false)]
[InlineData("Eterm", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0, false)]
[InlineData("Eterm", "\u001B\u005B\u00333m", "\u001B\u005B\u00343m", 3, false)]
[InlineData("Eterm", "\u001B\u005B\u003310m", "\u001B\u005B\u003410m", 10, false)]
[InlineData("wsvt25", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0, false)]
[InlineData("wsvt25", "\u001B\u005B\u00334m", "\u001B\u005B\u00344m", 4, false)]
[InlineData("wsvt25", "\u001B\u005B\u003311m", "\u001B\u005B\u003411m", 11, false)]
[InlineData("mach-color", "\u001B\u005B\u00330m", "\u001B\u005B\u00340m", 0, false)]
[InlineData("mach-color", "\u001B\u005B\u00335m", "\u001B\u005B\u00345m", 5, false)]
[InlineData("mach-color", "\u001B\u005B\u003312m", "\u001B\u005B\u003412m", 12, false)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a pity that it's not supported by more terminals. FWIW we have Console.ReadKey tests that have copies of terminfo files for multiple terminals:

runtime/src/libraries/System.Console/tests/KeyParserTests.cs

Line 446 in 104752a

protected override string EncodedTerminalDb => "GgElACYADwCdAQIGeHRlcm0tMjU2Y29sb3J8eHRlcm0gd2l0aCAyNTYgY29sb3JzAAABAAABAAAAAQAAAAABAQAAAAAAAAABAAABAAEBAAAAAAAAAAABAFAACAAYAP//////////////////////////AAH/fwAABAAGAAgAGQAeACYAKgAuAP//OQBKAEwAUABXAP//WQBmAP//agBuAHgAfAD/////gACEAIkAjgD//6AApQCqAP//rwC0ALkAvgDHAMsA0gD//+QA6QDvAPUA////////BwH///////8ZAf//HQH///////8fAf//JAH//////////ygBLAEyATYBOgE+AUQBSgFQAVYBXAFgAf//ZQH//2kBbgFzAXcBfgH//4UBiQGRAf////////////////////////////+ZAaIB/////6sBtAG9AcYBzwHYAeEB6gHzAfwB////////BQIJAg4CEwInAjAC/////0ICRQJQAlMCVQJYArUC//+4Av///////////////7oC//////////++Av//8wL/////9wL9Av////////////////////////////8DAwcD//////////////////////////////////////////////////////////////////8LA/////8SA///////////GQMgAycD/////y4D//81A////////zwD/////////////0MDSQNPA1YDXQNkA2sDcwN7A4MDiwOTA5sDowOrA7IDuQPAA8cDzwPXA98D5wPvA/cD/wMHBA4EFQQcBCMEKwQzBDsEQwRLBFMEWwRjBGoEcQR4BH8EhwSPBJcEnwSnBK8EtwS/BMYEzQTUBP/////////////////////////////////////////////////////////////ZBOQE6QT8BAAFCQUQBf////////////////////////////9uBf///////////////////////3MF////////////////////////////////////////////////////////////////////////////////////////eQX///////99BbwF//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////wF/wUbW1oABwANABtbJWklcDElZDslcDIlZHIAG1szZwAbW0gbWzJKABtbSwAbW0oAG1slaSVwMSVkRwAbWyVpJXAxJWQ7JXAyJWRIAAoAG1tIABtbPzI1bAAIABtbPzEybBtbPzI1aAAbW0MAG1tBABtbPzEyOzI1aAAbW1AAG1tNABsoMAAbWzVtABtbMW0AG1s/MTA0OWgbWzIyOzA7MHQAG1sybQAbWzRoABtbOG0AG1s3bQAbWzdtABtbNG0AG1slcDElZFgAGyhCABsoQhtbbQAbWz8xMDQ5bBtbMjM7MDswdAAbWzRsABtbMjdtABtbMjRtABtbPzVoJDwxMDAvPhtbPzVsABtbIXAbWz8zOzRsG1s0bBs+ABtbTAB/ABtbM34AG09CABtPUAAbWzIxfgAbT1EAG09SABtPUwAbWzE1fgAbWzE3fgAbWzE4fgAbWzE5fgAbWzIwfgAbT0gAG1syfgAbT0QAG1s2fgAbWzV+ABtPQwAbWzE7MkIAG1sxOzJBABtPQQAbWz8xbBs+ABtbPzFoGz0AG1s/MTAzNGwAG1s/MTAzNGgAG1slcDElZFAAG1slcDElZE0AG1slcDElZEIAG1slcDElZEAAG1slcDElZFMAG1slcDElZEwAG1slcDElZEQAG1slcDElZEMAG1slcDElZFQAG1slcDElZEEAG1tpABtbNGkAG1s1aQAlcDElYxtbJXAyJXsxfSUtJWRiABtjG10xMDQHABtbIXAbWz8zOzRsG1s0bBs+ABs4ABtbJWklcDElZGQAGzcACgAbTQAlPyVwOSV0GygwJWUbKEIlOxtbMCU/JXA2JXQ7MSU7JT8lcDUldDsyJTslPyVwMiV0OzQlOyU/JXAxJXAzJXwldDs3JTslPyVwNCV0OzUlOyU/JXA3JXQ7OCU7bQAbSAAJABtPRQBgYGFhZmZnZ2lpampra2xsbW1ubm9vcHBxcXJyc3N0dHV1dnZ3d3h4eXl6ent7fHx9fX5+ABtbWgAbWz83aAAbWz83bAAbT0YAG09NABtbMzsyfgAbWzE7MkYAG1sxOzJIABtbMjsyfgAbWzE7MkQAG1s2OzJ+ABtbNTsyfgAbWzE7MkMAG1syM34AG1syNH4AG1sxOzJQABtbMTsyUQAbWzE7MlIAG1sxOzJTABtbMTU7Mn4AG1sxNzsyfgAbWzE4OzJ+ABtbMTk7Mn4AG1syMDsyfgAbWzIxOzJ+ABtbMjM7Mn4AG1syNDsyfgAbWzE7NVAAG1sxOzVRABtbMTs1UgAbWzE7NVMAG1sxNTs1fgAbWzE3OzV+ABtbMTg7NX4AG1sxOTs1fgAbWzIwOzV+ABtbMjE7NX4AG1syMzs1fgAbWzI0OzV+ABtbMTs2UAAbWzE7NlEAG1sxOzZSABtbMTs2UwAbWzE1OzZ+ABtbMTc7Nn4AG1sxODs2fgAbWzE5OzZ+ABtbMjA7Nn4AG1syMTs2fgAbWzIzOzZ+ABtbMjQ7Nn4AG1sxOzNQABtbMTszUQAbWzE7M1IAG1sxOzNTABtbMTU7M34AG1sxNzszfgAbWzE4OzN+ABtbMTk7M34AG1syMDszfgAbWzIxOzN+ABtbMjM7M34AG1syNDszfgAbWzE7NFAAG1sxOzRRABtbMTs0UgAbWzFLABtbJWklZDslZFIAG1s2bgAbWz8lWzswMTIzNDU2Nzg5XWMAG1tjABtbMzk7NDltABtdMTA0BwAbXTQ7JXAxJWQ7cmdiOiVwMiV7MjU1fSUqJXsxMDAwfSUvJTIuMlgvJXAzJXsyNTV9JSolezEwMDB9JS8lMi4yWC8lcDQlezI1NX0lKiV7MTAwMH0lLyUyLjJYG1wAG1szbQAbWzIzbQAbW00AG1slPyVwMSV7OH0lPCV0MyVwMSVkJWUlcDElezE2fSU8JXQ5JXAxJXs4fSUtJWQlZTM4OzU7JXAxJWQlO20AG1slPyVwMSV7OH0lPCV0NCVwMSVkJWUlcDElezE2fSU8JXQxMCVwMSV7OH0lLSVkJWU0ODs1OyVwMSVkJTttABtsABttAAIAAABAAIIAAwMBAQAABwATABgAKgAwADoAQQBIAE8AVgBdAGQAawByAHkAgACHAI4AlQCcAKMAqgCxALgAvwDGAM0A1ADbAOIA6QDwAPcA/gAFAQwBEwEaASEBKAEvATYBPQFEAUsBUgFZAWABZwFuAXUBfAGDAYoBkQGYAZ8B//////////+mAawBAAADAAYACQAMAA8AEgAVABgAHQAiACcALAAxADUAOgA/AEQASQBOAFQAWgBgAGYAbAByAHgAfgCEAIoAjwCUAJkAngCjAKkArwC1ALsAwQDHAM0A0wDZAN8A5QDrAPEA9wD9AAMBCQEPARUBGwEfASQBKQEuATMBOAE8AUABRAFIAU0BG10xMTIHABtdMTI7JXAxJXMHABtbM0oAG101MjslcDElczslcDIlcwcAG1syIHEAG1slcDElZCBxABtbMzszfgAbWzM7NH4AG1szOzV+ABtbMzs2fgAbWzM7N34AG1sxOzJCABtbMTszQgAbWzE7NEIAG1sxOzVCABtbMTs2QgAbWzE7N0IAG1sxOzNGABtbMTs0RgAbWzE7NUYAG1sxOzZGABtbMTs3RgAbWzE7M0gAG1sxOzRIABtbMTs1SAAbWzE7NkgAG1sxOzdIABtbMjszfgAbWzI7NH4AG1syOzV+ABtbMjs2fgAbWzI7N34AG1sxOzNEABtbMTs0RAAbWzE7NUQAG1sxOzZEABtbMTs3RAAbWzY7M34AG1s2OzR+ABtbNjs1fgAbWzY7Nn4AG1s2Ozd+ABtbNTszfgAbWzU7NH4AG1s1OzV+ABtbNTs2fgAbWzU7N34AG1sxOzNDABtbMTs0QwAbWzE7NUMAG1sxOzZDABtbMTs3QwAbWzE7MkEAG1sxOzNBABtbMTs0QQAbWzE7NUEAG1sxOzZBABtbMTs3QQAbWzI5bQAbWzltAEFYAFhUAENyAENzAEUzAE1zAFNlAFNzAGtEQzMAa0RDNABrREM1AGtEQzYAa0RDNwBrRE4Aa0ROMwBrRE40AGtETjUAa0RONgBrRE43AGtFTkQzAGtFTkQ0AGtFTkQ1AGtFTkQ2AGtFTkQ3AGtIT00zAGtIT000AGtIT001AGtIT002AGtIT003AGtJQzMAa0lDNABrSUM1AGtJQzYAa0lDNwBrTEZUMwBrTEZUNABrTEZUNQBrTEZUNgBrTEZUNwBrTlhUMwBrTlhUNABrTlhUNQBrTlhUNgBrTlhUNwBrUFJWMwBrUFJWNABrUFJWNQBrUFJWNgBrUFJWNwBrUklUMwBrUklUNABrUklUNQBrUklUNgBrUklUNwBrVVAAa1VQMwBrVVA0AGtVUDUAa1VQNgBrVVA3AGthMgBrYjEAa2IzAGtjMgBybXh4AHNteHgA"; // /lib/terminfo/x/xterm-256color

runtime/src/libraries/System.Console/tests/KeyParserTests.cs

Lines 16 to 26 in 104752a

new XTermData(),
new GNOMETerminalData(),
new LinuxConsole(),
new PuTTYData_xterm(),
new PuTTYData_linux(),
new PuTTYData_putty(),
new WindowsTerminalData(),
new TmuxData(),
new Tmux256ColorData(),
new RxvtUnicode(),
new AlacrittyData(),

if you want to check anything related to this terminals you can just hack these tests, the TermInfo.Database is created here:

runtime/src/libraries/System.Console/tests/KeyParserTests.cs

Lines 431 to 432 in 104752a

internal TerminalFormatStrings TerminalDb => _terminalDb ??=
new TerminalFormatStrings(new TermInfo.Database(Term, Convert.FromBase64String(EncodedTerminalDb)));

You can even debug that on Windows, as these tests are platform-independent.

public void TermInfoVerification(string termToTest, string expectedForeground, string expectedBackground, int colorValue, bool expectedScrollbackClearSupport)
{
TermInfo.Database db = TermInfo.DatabaseFactory.ReadDatabase(termToTest);
if (db != null)
Expand All @@ -99,6 +99,7 @@ public void TermInfoVerification(string termToTest, string expectedForeground, s
Assert.Equal(expectedForeground, TermInfo.ParameterizedStrings.Evaluate(info.Foreground, colorValue));
Assert.Equal(expectedBackground, TermInfo.ParameterizedStrings.Evaluate(info.Background, colorValue));
Assert.InRange(info.MaxColors, 1, int.MaxValue);
Assert.Equal(expectedScrollbackClearSupport, !string.IsNullOrEmpty(info.ClearScrollbackBuffer));
}
}

Expand All @@ -108,7 +109,7 @@ public void EmuTermInfoDoesntBreakParser()
{
// This file (available by default on OS X) is called out specifically since it contains a format where it has %i
// but only one variable instead of two. Make sure we don't break in this case
TermInfoVerification("emu", "\u001Br1;", "\u001Bs1;", 0);
TermInfoVerification("emu", "\u001Br1;", "\u001Bs1;", 0, false);
}

[Fact]
Expand Down
Loading