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

Make Clear-Host clear the scrollback buffer too on all platforms #8606

Open
mklement0 opened this Issue Jan 8, 2019 · 13 comments

Comments

Projects
None yet
6 participants
@mklement0
Copy link
Contributor

mklement0 commented Jan 8, 2019

Update:

The decision in #8603 to rely on Console.Clear() amounts to inconsistent behavior across platforms, for no good reason (there's no native behavior that needs to be emulated; native behavior is accessible via the external clear utility).

If there's consensus that the desired behavior is to consistently clear the screen and the scrollback buffer on all platforms, we have two options:

  • Wait for Console.Clear() to be fixed, if ever - see dotnet/corefx#34463

  • Roll our own solution in PowerShell [in the meantime].

The original proposal below was written under the assumption that platform-dependent behavior was intended, and therefore suggested an opt-in mechanism.

Summary of the new feature/enhancement

Implement a switch named -All to direct Clear-Host to clear not only the current screen of terminal (console) hosts but to clear the scrollback buffer (the output history) as well.

Clear-Host now uses [Console]::Clear() (see #8603), which results in platform-specific behavior:

  • Unix-like platforms: clears the current screen (terminal-window content) only.
  • Windows: clears the current screen and the scroll-back buffer.

That is, also clearing the scrollback buffer is invariably implied on Windows, whereas it is currently unavailable on Unix-like platforms.

Implementing -All would allow users to at least opt into a consistent cross-platform experience, e.g. via a user-defined function such as function clsa { Clear-Host -All }.

Note: As before, this means that in the Windows console you do not have the option to selectively clear the current screen only, without erasing the scrollback buffer.

Proposed technical implementation details (optional)

If -All is specified:

  • on Windows: do nothing, because clearing the scrollback buffer is invariably performed anyway.
  • on Unix-like platforms, execute $host.ui.Write("`e[3J") after [Console]::Clear() to also clear the scrollback buffer.
@SteveL-MSFT

This comment has been minimized.

Copy link
Member

SteveL-MSFT commented Jan 8, 2019

The work to do this should be pretty minimal and straight forward. Remove Clear-Host as a function and reimplement as a C# cmdlet.

@dhulwells

This comment has been minimized.

Copy link

dhulwells commented Jan 8, 2019

Can I take this one?

@SteveL-MSFT

This comment has been minimized.

Copy link
Member

SteveL-MSFT commented Jan 8, 2019

@dhulwells consider it yours! If you have something basically working, you can always submit as a Work-In-Progress PR if you need help (just put WIP: as prefix to PR title)

@BrucePay

This comment has been minimized.

Copy link
Member

BrucePay commented Jan 8, 2019

@SteveL-MSFT @mklement0 Since Clear-Host on Windows already clears the scrollback buffer, this new switch -All would be a useless parameter on Windows resulting in either an error or it being silently ignored if used, neither of which seem undesirable.

Note: As before, this means that in the Windows console you do not have the option to selectively clear the current screen only, without erasing the scrollback buffer.

We can certainly implement this on Windows if desired, but I don't know that I've ever wanted it. YMMV.

@JamesWTruher @joeyaiello IIRC, the thrust of what we discussed in the committee meeting is that PowerShell would call Console.Clear() on all 'console' platforms and the CoreCLR team would implement the appropriate behaviour on each platform. If the CoreCLR isn't doing the right thing on a platform, then a bug should be opened in the CoreCLR repo. BUT(!) we do still need to go through the $Host.UI.RawUI APIs in order to support 'non-console' platforms like remoting. In fact, if the RawUI APIs were fully supported on non-Windows console hosts, this whole discussion might be moot.

@SteveL-MSFT

This comment has been minimized.

Copy link
Member

SteveL-MSFT commented Jan 8, 2019

@BrucePay it could be argued (and I expect the corefx team to argue) that [Console]::Clear() has the correct behavior from their point of view today which is terminal specific. Personally, I prefer to have the scrollback buffer cleared as my typical use case is that I clear the screen so I can output a bunch of content and expect the buffer to only have the new content whereas retaining the buffer means I have to visually determine where the new content exists when I use my scroll wheel. Having -All being no-op on Windows doesn't seem bad to me.

@mklement0

This comment has been minimized.

Copy link
Contributor

mklement0 commented Jan 9, 2019

Personally, I prefer to have the scrollback buffer cleared as my typical use case is that I clear the screen so I can output a bunch of content and expect the buffer to only have the new content whereas retaining the buffer means I have to visually determine where the new content exists when I use my scroll wheel.

Amen to that.

So, from that perspective the best user experience would be to consistently clear the scrollback buffer too, on all platforms, as originally suggested in #8554.

Your decision was to rely on [Console]::Clear(), which, I would argue, shouldn't exhibit platform-specific behavior either (and perhaps should offer a parameter for opting into screen-only clearing).

So, yes, a CoreFx issue should be opened, but if the consensus is that we want buffer clearing on all platforms, we can certainly make that happen now without having to wait for [Console]::Clear() to be fixed.

@mklement0

This comment has been minimized.

Copy link
Contributor

mklement0 commented Jan 9, 2019

I've opened a CoreFx issue: dotnet/corefx#34463

@mklement0 mklement0 changed the title Add a switch to Clear-Host to opt into clearing not just the current screen but also the scrollback buffer Make Clear-Host clear the scrollback buffer too on all platforms Jan 9, 2019

@SteveL-MSFT

This comment has been minimized.

Copy link
Member

SteveL-MSFT commented Jan 9, 2019

@mklement0 I had to rollback the change from [Console]::Clear() back to $RawUI since [Console]::Clear() doesn't work remotely when there isn't a physical console (see #8609).

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Jan 10, 2019

From #8609 (comment)

there's interest to convert this to a C# cmdlet and extend it, so let's defer any enhancements to that effort
it's implicitly part of #8606 since the way Clear-Host is currently implemented it's impossible to extend

@SeeminglyScience

This comment has been minimized.

Copy link
Contributor

SeeminglyScience commented Jan 10, 2019

Is there any reason why these changes wouldn't go into ConsoleHost instead of Clear-Host? The host can decide what to do with the current SetBufferContents even if it doesn't really support the API. For example, PSES will invoke Console.Clear if the args look like it came from Clear-Host but will no-op if it's passed anything else.

I realize that #8609 already changes back to using $Host for Windows, but is the expectation that custom hosts should replace Clear-Host for non-Windows?

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Jan 11, 2019

@SeeminglyScience I think this would be in ConsoleHost. Fill free to push PR if you want.

@SeeminglyScience

This comment has been minimized.

Copy link
Contributor

SeeminglyScience commented Jan 11, 2019

@iSazonov All of the changes so far have made within the function Clear-Host itself instead of ConsoleHost. There may be a very good reason for this that I'm not aware of, I'd just like to ensure that a custom host can still implement Clear-Host support on non-Windows platforms.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Jan 11, 2019

My understanding from PR discussion is that intention is (1) convert Clear-Host to binary cmdlet, (2) enhance rawui apis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment