Skip to content

Commit

Permalink
Console.Unix: revert SetWindowSize implementation. (#100272)
Browse files Browse the repository at this point in the history
* Console.Unix: revert SetWindowSize implementation.

SetWindowSize was implemented using using TIOCSWINSZ.
TIOCSWINSZ is meant to inform the kernel of the terminal size.
The window that shows the terminal doesn't change to match that size.

* Delete HAVE_TIOCSWINSZ.

* Update compat supressions.

* Code formatting.

* Apply suggestions from code review

---------

Co-authored-by: David Cantú <dacantu@microsoft.com>
  • Loading branch information
tmds and jozkee committed Aug 14, 2024
1 parent 0fbc04b commit 3eba702
Show file tree
Hide file tree
Showing 14 changed files with 87 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,5 @@ internal struct WinSize

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetWindowSize", SetLastError = true)]
internal static partial int GetWindowSize(SafeFileHandle terminalHandle, out WinSize winSize);

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetWindowSize", SetLastError = true)]
internal static partial int SetWindowSize(in WinSize winSize);
}
}
17 changes: 3 additions & 14 deletions src/libraries/System.Console/ref/System.Console.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,10 @@ public static partial class Console
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
public static bool TreatControlCAsInput { get { throw null; } set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
public static int WindowHeight { get { throw null; } set { } }
public static int WindowHeight { [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
public static int WindowLeft { get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
public static int WindowTop { get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
public static int WindowWidth { get { throw null; } set { } }
public static int WindowWidth { [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
Expand Down Expand Up @@ -155,10 +147,7 @@ public static void SetIn(System.IO.TextReader newIn) { }
public static void SetOut(System.IO.TextWriter newOut) { }
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
public static void SetWindowPosition(int left, int top) { }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
public static void SetWindowSize(int width, int height) { }
public static void Write(bool value) { }
public static void Write(char value) { }
Expand Down
50 changes: 12 additions & 38 deletions src/libraries/System.Console/src/System/Console.cs
Original file line number Diff line number Diff line change
Expand Up @@ -392,42 +392,27 @@ public static int WindowTop
set { ConsolePal.WindowTop = value; }
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
public static int WindowWidth
{
[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
get { return ConsolePal.WindowWidth; }
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowWidth));

ConsolePal.WindowWidth = value;
}
[SupportedOSPlatform("windows")]
set { ConsolePal.WindowWidth = value; }
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
public static int WindowHeight
{
[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
get { return ConsolePal.WindowHeight; }
[SupportedOSPlatform("windows")]
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowHeight));

ConsolePal.WindowHeight = value;
}
}
Expand All @@ -438,20 +423,9 @@ public static void SetWindowPosition(int left, int top)
ConsolePal.SetWindowPosition(left, top);
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[SupportedOSPlatform("windows")]
public static void SetWindowSize(int width, int height)
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width, nameof(width));
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height, nameof(height));

ConsolePal.SetWindowSize(width, height);
}

Expand Down
23 changes: 5 additions & 18 deletions src/libraries/System.Console/src/System/ConsolePal.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,24 +378,11 @@ private static void GetWindowSize(out int width, out int height)

public static void SetWindowSize(int width, int height)
{
lock (Console.Out)
{
Interop.Sys.WinSize winsize = default;
winsize.Row = (ushort)height;
winsize.Col = (ushort)width;
if (Interop.Sys.SetWindowSize(in winsize) == 0)
{
s_windowWidth = winsize.Col;
s_windowHeight = winsize.Row;
}
else
{
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
throw errorInfo.Error == Interop.Error.ENOTSUP ?
new PlatformNotSupportedException() :
Interop.GetIOException(errorInfo);
}
}
// note: We can't implement SetWindowSize using TIOCSWINSZ.
// TIOCSWINSZ is meant to inform the kernel of the terminal size.
// The window that shows the terminal doesn't change to match that size.

throw new PlatformNotSupportedException();
}

public static bool CursorVisible
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/System.Console/src/System/ConsolePal.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,14 @@ public static unsafe void SetWindowPosition(int left, int top)

public static unsafe void SetWindowSize(int width, int height)
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width, nameof(width));
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height, nameof(height));

// Get the position of the current console window
Interop.Kernel32.CONSOLE_SCREEN_BUFFER_INFO csbi = GetBufferInfo();

Expand Down
33 changes: 0 additions & 33 deletions src/libraries/System.Console/tests/ManualTests/ManualTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
using System.Diagnostics;
using System.Threading.Tasks;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using Xunit;

namespace System
Expand Down Expand Up @@ -334,37 +332,6 @@ public static void CursorLeftFromLastColumn()
AssertUserExpectedResults("single line with '1' at the start and '2' at the end.");
}

[ConditionalFact(nameof(ManualTestsEnabled))]
[SkipOnPlatform(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Not supported on Browser, iOS, MacCatalyst, or tvOS.")]
public static void ResizeTest()
{
bool wasResized = false;

using (ManualResetEvent manualResetEvent = new(false))
using (PosixSignalRegistration.Create(PosixSignal.SIGWINCH,
ctx =>
{
wasResized = true;
Assert.Equal(PosixSignal.SIGWINCH, ctx.Signal);
manualResetEvent.Set();
}))
{
int widthBefore = Console.WindowWidth;
int heightBefore = Console.WindowHeight;

Assert.False(wasResized);

Console.SetWindowSize(widthBefore / 2, heightBefore / 2);

Assert.True(manualResetEvent.WaitOne(TimeSpan.FromMilliseconds(50)));
Assert.True(wasResized);
Assert.Equal(widthBefore / 2, Console.WindowWidth );
Assert.Equal(heightBefore / 2, Console.WindowHeight );

Console.SetWindowSize(widthBefore, heightBefore);
}
}

private static void AssertUserExpectedResults(string expected)
{
Console.Write($"Did you see {expected}? [y/n] ");
Expand Down
10 changes: 5 additions & 5 deletions src/libraries/System.Console/tests/WindowAndCursorProps.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static void SetBufferSize_Unix_ThrowsPlatformNotSupportedException()
}

[Theory]
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[PlatformSpecific(TestPlatforms.Windows)]
[InlineData(0)]
[InlineData(-1)]
public static void WindowWidth_SetInvalid_ThrowsArgumentOutOfRangeException(int value)
Expand All @@ -71,14 +71,14 @@ public static void WindowWidth_GetUnix_Success()
}

[Fact]
[PlatformSpecific(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)] // Expected behavior specific to Unix
[PlatformSpecific(TestPlatforms.AnyUnix)]
public static void WindowWidth_SetUnix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowWidth = 100);
}

[Theory]
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[PlatformSpecific(TestPlatforms.Windows)]
[InlineData(0)]
[InlineData(-1)]
public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int value)
Expand Down Expand Up @@ -111,7 +111,7 @@ public static void LargestWindowWidth_UnixGet_ReturnsExpected()
}

[Fact]
[PlatformSpecific(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)] // Expected behavior specific to Unix
[PlatformSpecific(TestPlatforms.AnyUnix)]
public static void WindowHeight_SetUnix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowHeight = 100);
Expand Down Expand Up @@ -579,7 +579,7 @@ public void SetWindowSize_GetWindowSize_ReturnsExpected()
}

[Fact]
[PlatformSpecific(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)]
[PlatformSpecific(TestPlatforms.AnyUnix)]
public void SetWindowSize_Unix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.SetWindowSize(50, 50));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Console.SetWindowSize(System.Int32,System.Int32):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/mscorlib.dll</Left>
<Right>net9.0/mscorlib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowHeight:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/mscorlib.dll</Left>
<Right>net9.0/mscorlib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowWidth:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/mscorlib.dll</Left>
<Right>net9.0/mscorlib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down Expand Up @@ -91,6 +109,12 @@
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Console.SetWindowSize(System.Int32,System.Int32):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down Expand Up @@ -121,6 +145,18 @@
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowHeight:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowWidth:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down Expand Up @@ -349,6 +385,24 @@
<Left>net8.0/System.ComponentModel.TypeConverter.dll</Left>
<Right>net9.0/System.ComponentModel.TypeConverter.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Console.SetWindowSize(System.Int32,System.Int32):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/System.Console.dll</Left>
<Right>net9.0/System.Console.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowHeight:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/System.Console.dll</Left>
<Right>net9.0/System.Console.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowWidth:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/System.Console.dll</Left>
<Right>net9.0/System.Console.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down
1 change: 0 additions & 1 deletion src/native/libs/Common/pal_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#cmakedefine01 HAVE_IOCTL
#cmakedefine01 HAVE_IOCTL_WITH_INT_REQUEST
#cmakedefine01 HAVE_TIOCGWINSZ
#cmakedefine01 HAVE_TIOCSWINSZ
#cmakedefine01 HAVE_SCHED_GETAFFINITY
#cmakedefine01 HAVE_SCHED_SETAFFINITY
#cmakedefine01 HAVE_SCHED_GETCPU
Expand Down
1 change: 0 additions & 1 deletion src/native/libs/System.Native/entrypoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ static const Entry s_sysNative[] =
{
DllImportEntry(SystemNative_FStat)
DllImportEntry(SystemNative_GetWindowSize)
DllImportEntry(SystemNative_SetWindowSize)
DllImportEntry(SystemNative_IsATty)
DllImportEntry(SystemNative_InitializeTerminalAndSignalHandling)
DllImportEntry(SystemNative_SetKeypadXmit)
Expand Down
16 changes: 0 additions & 16 deletions src/native/libs/System.Native/pal_console.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,6 @@ int32_t SystemNative_GetWindowSize(intptr_t fd, WinSize* windowSize)
#endif
}

int32_t SystemNative_SetWindowSize(WinSize* windowSize)
{
assert(windowSize != NULL);

#if HAVE_IOCTL_WITH_INT_REQUEST && HAVE_TIOCSWINSZ
return ioctl(STDOUT_FILENO, (int)TIOCSWINSZ, windowSize);
#elif HAVE_IOCTL && HAVE_TIOCSWINSZ
return ioctl(STDOUT_FILENO, TIOCSWINSZ, windowSize);
#else
// Not supported on e.g. Android. Also, prevent a compiler error because windowSize is unused
(void)windowSize;
errno = ENOTSUP;
return -1;
#endif
}

int32_t SystemNative_IsATty(intptr_t fd)
{
return isatty(ToFileDescriptor(fd));
Expand Down
7 changes: 0 additions & 7 deletions src/native/libs/System.Native/pal_console.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,6 @@ typedef struct
*/
PALEXPORT int32_t SystemNative_GetWindowSize(intptr_t fd, WinSize* windowsSize);

/**
* Sets the windows size of the terminal
*
* Returns 0 on success; otherwise, returns -1 and sets errno.
*/
PALEXPORT int32_t SystemNative_SetWindowSize(WinSize* windowsSize);

/**
* Gets whether the specified file descriptor is for a terminal.
*
Expand Down
Loading

0 comments on commit 3eba702

Please sign in to comment.