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.

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

Implement Console.SetWindowSize() for linux #75824

Merged
merged 12 commits into from
Nov 8, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@ internal struct WinSize

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

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetWindowSize", SetLastError = true)]
internal static partial int SetWindowSize(in WinSize winSize);
}
}
7 changes: 5 additions & 2 deletions src/libraries/System.Console/ref/System.Console.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +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 { } }
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 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"), System.Runtime.Versioning.SupportedOSPlatformAttribute("linux"), System.Runtime.Versioning.SupportedOSPlatformAttribute("freebsd"), System.Runtime.Versioning.SupportedOSPlatformAttribute("osx")] set { } }
Copy link
Member

@adamsitnik adamsitnik Nov 2, 2022

Choose a reason for hiding this comment

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

@buyaa-n @stephentoub So far, getting windows height and width was working on all OSes except browser, ios, adnroid and tvos.
Setting the value was working only on Windows. So the attributes applied were like this:

public static int WindowHeight
{ 
    [UnsupportedOSPlatformAttribute("android")]
    [UnsupportedOSPlatformAttribute("browser")]
    [UnsupportedOSPlatformAttribute("ios")]
    [UnsupportedOSPlatformAttribute("tvos")]
    get { throw null; } 
    [SupportedOSPlatformAttribute("windows")]
    set { } }
}

With this PR, both sets of supported/unsupported OSes are the same:

  • supported: Windows, Linux, macOS, FreeBSD
  • unsupported: Android, iOS, tvOS, Broweser

The question: what is the recommended way to annotate the APIs in such case:
a) marking getter and setter of WindowHeight as unsupported on the mentioned platforms (a removal of [SupportedOSPlatformAttribute("windows")] from the setter)

[UnsupportedOSPlatformAttribute("android")]
[UnsupportedOSPlatformAttribute("browser")]
[UnsupportedOSPlatformAttribute("ios")]
[UnsupportedOSPlatformAttribute("tvos")]
public static int WindowHeight
{ 
    get { throw null; } 
    set { } }
}

b) Extending the list of supported OSes for the setter (no breaking changes, but using two different ways of expressing the same thing for one public property which can be confusing)

public static int WindowHeight
{ 
    [UnsupportedOSPlatformAttribute("android")]
    [UnsupportedOSPlatformAttribute("browser")]
    [UnsupportedOSPlatformAttribute("ios")]
    [UnsupportedOSPlatformAttribute("tvos")]
    get { throw null; } 
    [SupportedOSPlatformAttribute("windows")]
    [SupportedOSPlatformAttribute("linux")]
    [SupportedOSPlatformAttribute("freebsd")]
    [SupportedOSPlatformAttribute("osx")]
    set { } }
}

Copy link
Member

Choose a reason for hiding this comment

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

[UnsupportedOSPlatformAttribute("android")]
[UnsupportedOSPlatformAttribute("browser")]
[UnsupportedOSPlatformAttribute("ios")]
[UnsupportedOSPlatformAttribute("tvos")]

Looks cleaner though it means the API is unsupported on android, browser, ios and tvOS, but supported all other, not only windows, linux, freebsd, osx. Not sure if that is OK.

For the PCA analyzer it doesn't really matter where the attribute applied, but not sure if it's moving the attributes is OK for API compat side

Copy link
Member

Choose a reason for hiding this comment

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

Is there a downside to option (a)? That seems like the clear winner :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamsitnik @buyaa-n I think I need help on e315d16
How do I fix CP0014 error?

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 { } }
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 { } }
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"), System.Runtime.Versioning.SupportedOSPlatformAttribute("linux"), System.Runtime.Versioning.SupportedOSPlatformAttribute("freebsd"), System.Runtime.Versioning.SupportedOSPlatformAttribute("osx")] set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
Expand Down Expand Up @@ -148,6 +148,9 @@ public static void SetOut(System.IO.TextWriter newOut) { }
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
public static void SetWindowPosition(int left, int top) { }
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
[System.Runtime.Versioning.SupportedOSPlatformAttribute("linux")]
[System.Runtime.Versioning.SupportedOSPlatformAttribute("freebsd")]
[System.Runtime.Versioning.SupportedOSPlatformAttribute("osx")]
public static void SetWindowSize(int width, int height) { }
public static void Write(bool value) { }
public static void Write(char value) { }
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Console/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@
<data name="InvalidOperation_ConsoleReadKeyOnFile" xml:space="preserve">
<value>Cannot read keys when either application does not have a console or when console input has been redirected. Try Console.Read.</value>
</data>
<data name="InvalidOperation_SetWindowSize" xml:space="preserve">
<value>Cannot set window size when console output has been redirected.</value>
</data>
<data name="PersistedFiles_NoHomeDirectory" xml:space="preserve">
<value>The home directory of the current user could not be determined.</value>
</data>
Expand Down
47 changes: 45 additions & 2 deletions src/libraries/System.Console/src/System/Console.cs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,21 @@ public static int WindowWidth
[UnsupportedOSPlatform("tvos")]
get { return ConsolePal.WindowWidth; }
[SupportedOSPlatform("windows")]
set { ConsolePal.WindowWidth = value; }
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]
[SupportedOSPlatform("osx")]
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
if (value <= 0)
{
throw new ArgumentOutOfRangeException(nameof(value), value, SR.ArgumentOutOfRange_NeedPosNum);
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
ConsolePal.WindowWidth = value;
}
}

public static int WindowHeight
Expand All @@ -411,7 +425,21 @@ public static int WindowHeight
[UnsupportedOSPlatform("tvos")]
get { return ConsolePal.WindowHeight; }
[SupportedOSPlatform("windows")]
set { ConsolePal.WindowHeight = value; }
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]
[SupportedOSPlatform("osx")]
set
{
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
if (value <= 0)
{
throw new ArgumentOutOfRangeException(nameof(value), value, SR.ArgumentOutOfRange_NeedPosNum);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
ConsolePal.WindowHeight = value;
}
}

[SupportedOSPlatform("windows")]
Expand All @@ -421,8 +449,23 @@ public static void SetWindowPosition(int left, int top)
}

[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]
[SupportedOSPlatform("osx")]
public static void SetWindowSize(int width, int height)
{
if (Console.IsOutputRedirected)
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
if (width <= 0)
{
throw new ArgumentOutOfRangeException(nameof(width), width, SR.ArgumentOutOfRange_NeedPosNum);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
if (height <= 0)
{
throw new ArgumentOutOfRangeException(nameof(height), height, SR.ArgumentOutOfRange_NeedPosNum);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
ConsolePal.SetWindowSize(width, height);
}

Expand Down
28 changes: 25 additions & 3 deletions src/libraries/System.Console/src/System/ConsolePal.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ public static int WindowWidth
GetWindowSize(out int width, out _);
return width;
}
set { throw new PlatformNotSupportedException(); }
set => SetWindowSize(value, WindowHeight);
}

public static int WindowHeight
Expand All @@ -356,7 +356,7 @@ public static int WindowHeight
GetWindowSize(out _, out int height);
return height;
}
set { throw new PlatformNotSupportedException(); }
set => SetWindowSize(WindowWidth, value);
}

private static void GetWindowSize(out int width, out int height)
Expand Down Expand Up @@ -392,7 +392,29 @@ public static void SetWindowPosition(int left, int top)

public static void SetWindowSize(int width, int height)
{
throw new PlatformNotSupportedException();
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();
if (errorInfo.Error == Interop.Error.ENOTSUP)
{
throw new PlatformNotSupportedException();
}
else
{
throw Interop.GetIOException(errorInfo);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

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

public static unsafe void SetWindowSize(int width, int height)
{
if (width <= 0)
throw new ArgumentOutOfRangeException(nameof(width), width, SR.ArgumentOutOfRange_NeedPosNum);
if (height <= 0)
throw new ArgumentOutOfRangeException(nameof(height), height, SR.ArgumentOutOfRange_NeedPosNum);

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

Expand Down
40 changes: 40 additions & 0 deletions src/libraries/System.Console/tests/ManualTests/ManualTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
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 @@ -321,6 +323,44 @@ public static void EncodingTest()
AssertUserExpectedResults("Pi and Sigma or question marks");
}

[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 );

try
{
AssertUserExpectedResults("terminal window getting two times smaller?");
}
finally
{
Console.SetWindowSize(widthBefore, heightBefore);
}
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
}
}

private static void AssertUserExpectedResults(string expected)
{
Console.Write($"Did you see {expected}? [y/n] ");
Expand Down
24 changes: 12 additions & 12 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)]
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[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.AnyUnix & ~TestPlatforms.Browser)] // Expected behavior specific to Unix
[PlatformSpecific(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)] // Expected behavior specific to Unix
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
public static void WindowWidth_SetUnix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowWidth = 100);
}

[Theory]
[PlatformSpecific(TestPlatforms.Windows)] // Expected behavior specific to Windows
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[InlineData(0)]
[InlineData(-1)]
public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int value)
Expand All @@ -102,13 +102,6 @@ public static void WindowHeight_GetUnix_Success()
Helpers.RunInRedirectedOutput((data) => Console.WriteLine(Console.WindowHeight));
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix)] // Expected behavior specific to Unix
public static void WindowHeight_SetUnix_ThrowsPlatformNotSupportedException()
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowHeight = 100);
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS)] // Expected behavior specific to Unix
public static void LargestWindowWidth_UnixGet_ReturnsExpected()
Expand All @@ -117,6 +110,13 @@ public static void LargestWindowWidth_UnixGet_ReturnsExpected()
Helpers.RunInRedirectedOutput((data) => Assert.Equal(Console.WindowWidth, Console.LargestWindowWidth));
}

[Fact]
[PlatformSpecific(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)] // Expected behavior specific to Unix
HJLeee marked this conversation as resolved.
Show resolved Hide resolved
public static void WindowHeight_SetUnix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowHeight = 100);
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS)] // Expected behavior specific to Unix
public static void LargestWindowHeight_UnixGet_ReturnsExpected()
Expand Down Expand Up @@ -559,7 +559,7 @@ public void SetWindowPosition_Unix_ThrowsPlatformNotSupportedException()
Assert.Throws<PlatformNotSupportedException>(() => Console.SetWindowPosition(50, 50));
}

[PlatformSpecific(TestPlatforms.Windows)]
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))]
public void SetWindowSize_GetWindowSize_ReturnsExpected()
{
Expand Down Expand Up @@ -587,7 +587,7 @@ public void SetWindowSize_GetWindowSize_ReturnsExpected()
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix)]
[PlatformSpecific(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)]
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
public void SetWindowSize_Unix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.SetWindowSize(50, 50));
Expand Down
1 change: 1 addition & 0 deletions src/native/libs/Common/pal_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#cmakedefine01 KEVENT_REQUIRES_INT_PARAMS
#cmakedefine01 HAVE_IOCTL
#cmakedefine01 HAVE_TIOCGWINSZ
#cmakedefine01 HAVE_TIOCSWINSZ
#cmakedefine01 HAVE_SCHED_GETAFFINITY
#cmakedefine01 HAVE_SCHED_SETAFFINITY
#cmakedefine01 HAVE_SCHED_GETCPU
Expand Down
1 change: 1 addition & 0 deletions src/native/libs/System.Native/entrypoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ 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
12 changes: 12 additions & 0 deletions src/native/libs/System.Native/pal_console.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ int32_t SystemNative_GetWindowSize(WinSize* windowSize)
#endif
}

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

#if HAVE_IOCTL && HAVE_TIOCSWINSZ
return ioctl(STDOUT_FILENO, TIOCSWINSZ, windowSize);
#else
errno = ENOTSUP;
return -1;
#endif
}

int32_t SystemNative_IsATty(intptr_t fd)
{
return isatty(ToFileDescriptor(fd));
Expand Down
9 changes: 8 additions & 1 deletion src/native/libs/System.Native/pal_console.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,17 @@ typedef struct
/**
* Gets the windows size of the terminal
*
* Returns 0 on success; otherwise, returns errorNo.
* Returns 0 on success; otherwise, returns -1 and sets errno.
*/
PALEXPORT int32_t SystemNative_GetWindowSize(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
5 changes: 5 additions & 0 deletions src/native/libs/configure.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ check_symbol_exists(
"sys/ioctl.h"
HAVE_TIOCGWINSZ)

check_symbol_exists(
TIOCSWINSZ
"sys/ioctl.h"
HAVE_TIOCSWINSZ)

check_symbol_exists(
tcgetattr
termios.h
Expand Down