-
Notifications
You must be signed in to change notification settings - Fork 689
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
Fixes #2261 - Drivers clear the scrollback buffer on resize and exit. #2264
Conversation
@tig this only solves when you exit the app, but if buffer as height is false, when you running the app you can scroll and you shouldn't. Here you needed the 3j. I'll work on it today. |
I don't understand this at all. I don't see the app scrolling. This code is only emitted when the app exits, so it should have no impact while the app is running. |
I think you would want like this: This isn't working well with |
Got it. I now understand. Thank you. For now, until #5094 is fixed by the WT team, I think the behavior shown in your first video is preferrable to clearing the buffer and causing what I (and @kingzawar, apparently) think data loss. Make sure y'all thumbs-up #5094 so the Terminal team takes it seriously. |
The |
I think I understand you. However, what I don't know is how often people want a buffer size that's different than the window size. I suspect it's a very rare use case? |
Not exactly. See the example of the second video where I enabled the |
But WHY would someone want to enable I'm sorry if I am being persistent on this, but I sincerely do not understand (and never have) the use case. This is likely just me being slow. |
It's only an option. Why the author opened issue? Because it was clearing the historic after exit the app. But, why he needs to scroll to see the historic? Well, before I fixed the window resize on the |
If one were to re-write the API docs for Application.HeightAsBuffer currently reads as /// <summary>
/// The current <see cref="ConsoleDriver.HeightAsBuffer"/> used in the terminal.
/// </summary>
public static bool HeightAsBuffer { ConsoleDriver.HeightAsBuffer currently reads as: /// <summary>
/// If false height is measured by the window height and thus no scrolling.
/// If true then height is measured by the buffer height, enabling scrolling.
/// </summary>
public abstract bool HeightAsBuffer { get; set; } "If set to false (the default), the height of the Terminal.Gui application's top-most, TopLevel will be resized to match the visible console size if the console is resized and the console will not scroll. If set to true, the height of the Terminal.Gui application's top-most, TopLevel, will not resize when the console is resized and the console can be scrolled." Is that correct? If not, please re-write it so that it is correct. |
/// <summary>
/// If false height is measured by the window height and thus no scrolling.
/// If true then height is measured by the buffer height, enabling scrolling.
/// </summary>
public abstract bool HeightAsBuffer { get; set; } This definition is perfectly wright. Should be copied to the |
/// <summary>
/// If false height is measured by the window height and thus no scrolling.
/// If true then height is measured by the buffer height, enabling scrolling.
/// </summary>
public abstract bool HeightAsBuffer { get; set; } This definition is perfectly right. Should be copied to the It may be perfectly right, but I find it VERY confusing and unclear.
Would it have made more sense to name this property "EnableConsoleScrolling"? I'm not challenging or disagreeing with you. I seriously do not understand how this is supposed to work. |
Refers to the Rows.
It's the visible console view.
It's the Rows length.
Refers scrolling the console within the app.
Scrolling the console within the app allows view all the available rows.
Perhaps is better. I leave that up to you.
No problem. I hope you now understanding all my concern about this. Thanks. |
I would appreciate it if you used your answers above to recraft the API documentation for |
I will in both, |
Could you draft it here in a comment first? |
Sure and please make the changes needed. In /// <summary>
/// <para>
/// If <see langword="false"/> (the default) the height of the Terminal.Gui application (<see cref="Rows"/>) tracks
/// to the height of the visible console view when the console is resized. In this case
/// scrolling in the console will be disabled and all <see cref="Rows"/> will remain visible.
/// </para>
/// <para>
/// If <see langword="true"/> then height of the Terminal.Gui application <see cref="Rows"/> does not
/// change when the console is resized (it stays the same value it was set to at application startup).
/// In this case console scrolling is enabled and the contents (<see cref="Rows"/> high) will scroll
/// as the console scrolls.
/// </para>
/// </summary>
public abstract bool HeightAsBuffer { get; set; } In /// <summary>
/// The current <see cref="ConsoleDriver.HeightAsBuffer"/> used in the terminal.
/// </summary>
/// <remarks>
/// <para>
/// If <see langword="false"/> (the default) the height of the Terminal.Gui application (<see cref="ConsoleDriver.Rows"/>) tracks
/// to the height of the visible console view when the console is resized. In this case
/// scrolling in the console will be disabled and all <see cref="ConsoleDriver.Rows"/> will remain visible.
/// </para>
/// <para>
/// If <see langword="true"/> then height of the Terminal.Gui application <see cref="Rows"/> does not
/// change when the console is resized (it stays the same value it was set to when <see cref="Application.Begin"/> was called).
/// In this case console scrolling is enabled and the contents (<see cref="Rows"/> high) will scroll
/// as the console scrolls.
/// </para>
/// </remarks>
public static bool HeightAsBuffer { |
I edited your post. Is it still accurate? |
Only this is incorrect: /// If <see langword="true"/> then height of the Terminal.Gui application <see cref="Rows"/> does not
/// change when the console is resized (it stays the same value it was set to at application startup). The height only changes if the current height after resizing is greater than the current |
@tig I sent the PR tig#8 which is currently working on Windows. Also the scroll bar is only hidden when the app is running and |
…f border is null.
So, if I understand you correctly (and this is backed by my tests):
I've now tried 2 different Linux terminals (xterm and terminator) and I have fixed all issues having to with THIS issue (independent of HeightAsBuffer). Please review my code. I also took the liberty of fixing the formatting in all the ConsoleDrivers files and removing extraneous comments since so many of the files were already touched. |
For now yes. My intention is also use a workaround on Unix to add new lines after the windows bottom while the
Negative. The scrolling is only enabled if the
False, it shouldn't. Only must enable the scroll bar if the
For now isn't working but as I said above maybe is possible to make it work. The behavior is intended to make it work with all terminal, but sometimes there are some incompatibilities
You have to be more specific and explain what was bad and now it's working. I know that as I said that scrolling wasn't working on
I agree, it was needed but you know I'm not nothing good with docs :-) PS: When I create the |
@tig but I admit another scenario for the vertical scroll bar behavior, like an public enum VerticalScrollBehavior {
// No scroll at all.
None,
// Only allow scrolling into the app.
AppScrolling,
// Allow scrolling on all scrollback buffer (original buffer more app buffer)
// This allows an app write to the console preserving with all scrollback buffer.
ScrollBackBuffer
} |
We need to retarget this PR at the |
I'm going to get back to working on this asap. I think this may be worth taking into Please debate if you don't agree. |
I think we should consider this for v1. |
I can't remember what we finally decided to do here.
The only other thing in that was in this PR, that does not appear to be in @bdiisp, do you have an opinion on what we should do with this PR? |
If |
Dupe of #2764 |
…ed code (#2612) * Added ClipRegion; cleaned up driver code * clip region unit tests * api docs * Moved color stuff from ConsoleDriver to Color.cs * Removes unused ConsoleDriver APIs * Code cleanup and Removes unused ConsoleDriver APIs * Code cleanup and Removes unused ConsoleDriver APIs * Work around #2610 * adjusted unit tests * initial commit * Made Rows, Cols, Top, Left virtual * Made Clipboard non-virtual * Made EnableConsoleScrolling non-virtual * Made Contents non-virtual * Pulled Row/Col up * Made MoveTo virtual; fixed stupid FakeDriver cursor issue * Made CurrentAttribute non-virtual * Made SetAttribute non-virtual * Moved clipboard code out * Code cleanup * Removes dependecy on NStack from ConsoleDrivers - WIP * Fixed unit tests * Fixed unit tests * Added list of unit tests needed * Did some perf testing; tweaked code and charmap to address * Brough in code from PR #2264 (but commented) * Tons of code cleanup * Fighting with ScrollView * Fixing bugs * Fixed TabView tests * Fixed View.Visible test that was not really working * Fixed unit tests * Cleaned up clipboard APIs in attempt to track down unit test failure * Add Cut_Preserves_Selection test * Removed invalid code * Removed invalid test code; unit tests now pass * EscSeq* - Adjusted naming, added more sequences, made code more consistent, simplified, etc... * Added CSI_SetGraphicsRendition * NetDriver code cleanup * code cleanup * Cleaned up color handling in NetDriver * refixed tabview unit test * WindowsDriver color code cleanup * WindowsDriver color code cleanup * CursesDriver color code cleanup * CursesDriver - Adding _BOLD has no effect. Further up the stack we cast the return of ColorToCursesColor from int to short and the _BOLD values don't fit in a short. * CursesDriver color code - make code more accurate * CursesDriver color code - make code more accurate * Simplified ConsoleDriver.GetColors API * Simplified ConsoleDriver.GetColors API further * Improved encapslation of Attribute; prep for TrueColor & other attributes like blink * Fixes #2249. CharacterMap isn't refreshing well non-BMP code points on scroll. * Use GetRange to take some of the runes before convert to string. * Attempting to fix unit tests not being cleaned up * Fixes #2658 - ConsoleDriver.IsRuneSupported * Fixes #2658 - ConsoleDriver.IsRuneSupported (for WindowsDriver) * Check all the range values and not only the max value. * Reducing code. * Fixes #2674 - Unit test process doesn't exit * Changed Cell to support IsDirty and list of Runes * add support for rendering TrueColor output on Windows merging veeman & tznind code * add colorconverter changes * fixed merged v2_develop * Fixing merge bugs * Fixed merge bugs * Fixed merge bugs - all unit tests pass * Debugging netdriver * More netdriver diag * API docs for escutils * Update unicode scenario to stress more stuff * Contents: Now a 2D array of Cells; WIP * AddRune and ClearContents no longer virtual/abstract * WindowsDriver renders correctly again * Progress on Curses * Progress on Curses * broke windowsdriver * Cleaned up FakeMainLoop * Cleaned up some build warnings * Removed _init from AutoInitShutdown as it's not needed anymore * Removed unused var * Removed unused var * Fixed nullabiltiy warning in LineCanvas * Fixed charmap crash * Fixes #2758 in v2 * Port testonfail fix to v2 * Remove EnableConsoleScrolling * Backport #2764 from develop (clear last line) * Remove uneeded usings * Progress on unicode * Merged in changes from PR #2786, Fixes #2784 * revamp charmap rendering * Charmap option to show glyph widths * Fixed issue with wide glpyhs being overwritten * Fixed charmap startcodepoint change issue * Added abiltiy to see ncurses verison/lib * Fought with CursesDriver; giving up for now. See notes. * Leverage Wcwidth nuget library instaed of our own tables * enhanced charmap Details dialog * Final attempt at fixing curses --------- Co-authored-by: BDisp <[email protected]> Co-authored-by: adstep <[email protected]>
* Added ClipRegion; cleaned up driver code * clip region unit tests * api docs * Moved color stuff from ConsoleDriver to Color.cs * Removes unused ConsoleDriver APIs * Code cleanup and Removes unused ConsoleDriver APIs * Code cleanup and Removes unused ConsoleDriver APIs * Work around #2610 * adjusted unit tests * initial commit * Made Rows, Cols, Top, Left virtual * Made Clipboard non-virtual * Made EnableConsoleScrolling non-virtual * Made Contents non-virtual * Pulled Row/Col up * Made MoveTo virtual; fixed stupid FakeDriver cursor issue * Made CurrentAttribute non-virtual * Made SetAttribute non-virtual * Moved clipboard code out * Code cleanup * Removes dependecy on NStack from ConsoleDrivers - WIP * Fixed unit tests * Fixed unit tests * Added list of unit tests needed * Did some perf testing; tweaked code and charmap to address * Brough in code from PR #2264 (but commented) * Tons of code cleanup * Fighting with ScrollView * Fixing bugs * Fixed TabView tests * Fixed View.Visible test that was not really working * Fixed unit tests * Cleaned up clipboard APIs in attempt to track down unit test failure * Add Cut_Preserves_Selection test * Removed invalid code * Removed invalid test code; unit tests now pass * EscSeq* - Adjusted naming, added more sequences, made code more consistent, simplified, etc... * Added CSI_SetGraphicsRendition * NetDriver code cleanup * code cleanup * Cleaned up color handling in NetDriver * refixed tabview unit test * WindowsDriver color code cleanup * WindowsDriver color code cleanup * CursesDriver color code cleanup * CursesDriver - Adding _BOLD has no effect. Further up the stack we cast the return of ColorToCursesColor from int to short and the _BOLD values don't fit in a short. * CursesDriver color code - make code more accurate * CursesDriver color code - make code more accurate * Simplified ConsoleDriver.GetColors API * Simplified ConsoleDriver.GetColors API further * Improved encapslation of Attribute; prep for TrueColor & other attributes like blink * Fixes #2249. CharacterMap isn't refreshing well non-BMP code points on scroll. * Use GetRange to take some of the runes before convert to string. * Attempting to fix unit tests not being cleaned up * Fixes #2658 - ConsoleDriver.IsRuneSupported * Fixes #2658 - ConsoleDriver.IsRuneSupported (for WindowsDriver) * Check all the range values and not only the max value. * Reducing code. * Fixes #2674 - Unit test process doesn't exit * Changed Cell to support IsDirty and list of Runes * add support for rendering TrueColor output on Windows merging veeman & tznind code * add colorconverter changes * fixed merged v2_develop * Fixing merge bugs * Fixed merge bugs * Add outline for FileSystemColorProvider * Add other known folders * Added remaining cases and test for file colors * Use color provider in FileDialog * Fix default color when UseColors to white * Remove `TestDirectoryContents_Windows_Colors` * Remove unused helper method * Fix formatting * Fixed merge bugs - all unit tests pass * Debugging netdriver * More netdriver diag * API docs for escutils * Update unicode scenario to stress more stuff * Contents: Now a 2D array of Cells; WIP * AddRune and ClearContents no longer virtual/abstract * WindowsDriver renders correctly again * Progress on Curses * Progress on Curses * broke windowsdriver * Cleaned up FakeMainLoop * Cleaned up some build warnings * Removed _init from AutoInitShutdown as it's not needed anymore * Removed unused var * Removed unused var * Fixed nullabiltiy warning in LineCanvas * Fixed charmap crash * Fixes #2758 in v2 * Remove accidentally re-added test * Removed unit test xml file * Remove redundant test --------- Co-authored-by: Tig Kindel <[email protected]> Co-authored-by: BDisp <[email protected]> Co-authored-by: adstep <[email protected]>
Fixes #2261
Fixes #2303
It turns out that all drivers were behaving poorly in this regard.
ESC [3J
on both Resize and End. This always clears the scrollback buffer (even if the alternative screen buffer is activated).ESC [3J
.ESC [3J
instead of callingCurses.refresh()
in resize.We were using
ESC [ ? 1049 h/l
to activate the alternative buffer in both WindowsDriver and NetDriver. Changed this toESC [ ? 1047 h/l
because it is less destructive (doesn't mess with the cursor) and simply works better on Windows Terminal.Turns out
Console.Out.Flush()
is not needed because the underlyingConsole.Out
stream hasAutoFlush
enabled. Removed all calls to simplify code.Removed old comments and ensured formatting was correct (spaces->tabs) in all driver files.