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

Fixes #2634 - Windows Terminal now clears correctly on exit #2764

Merged
merged 11 commits into from
Jul 25, 2023

Conversation

tig
Copy link
Collaborator

@tig tig commented Jul 20, 2023

Fixes #2634

I found that by changing Cleanup() to not call SetConsoleOutputWindow(), everything works fine!

This also fixes the problem where after resizing and closing, the terminal is screwed up.

YdBNKzP 1

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tig tig requested a review from migueldeicaza as a code owner July 20, 2023 15:59
@tig
Copy link
Collaborator Author

tig commented Jul 20, 2023

@BDisp please review.

@tig tig added the v1 For Issues & PRs targetting v1.x label Jul 20, 2023
Copy link
Collaborator

@BDisp BDisp left a comment

Choose a reason for hiding this comment

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

@tig please see my review. Thanks.

Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Outdated Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Outdated Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Outdated Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Outdated Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Outdated Show resolved Hide resolved
Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Outdated Show resolved Hide resolved
@tig
Copy link
Collaborator Author

tig commented Jul 20, 2023

@BDisp Please give the latest a look. I think I addressed everything.

@tig
Copy link
Collaborator Author

tig commented Jul 25, 2023

@BDisp Please give the latest a look. I think I addressed everything.

Any suggestions for improving this now @BDisp? I'd like to merge it so a new ocgv can be released that doesn't leave the status bar...

@BDisp
Copy link
Collaborator

BDisp commented Jul 25, 2023

Any suggestions for improving this now @BDisp? I'd like to merge it so a new ocgv can be released that doesn't leave the status bar...

@tig if you only want to fix the issue of the status bar appearing after exit the app, I'll look into and submit a PR to your branch.

@tig
Copy link
Collaborator Author

tig commented Jul 25, 2023

This also fixes the problem where after resizing and closing, the terminal is screwed up.

I think fixing "This also fixes the problem where after resizing and closing, the terminal is screwed up." is also important (and more important that EnableConsoleScrolling).

@BDisp
Copy link
Collaborator

BDisp commented Jul 25, 2023

I think fixing "This also fixes the problem where after resizing and closing, the terminal is screwed up." is also important (and more important that EnableConsoleScrolling).

Why not remove EnableConsoleScrolling completely if it will not needed anymore? I think that makes more sense after all.

@tig
Copy link
Collaborator Author

tig commented Jul 25, 2023

I think fixing "This also fixes the problem where after resizing and closing, the terminal is screwed up." is also important (and more important that EnableConsoleScrolling).

Why not remove EnableConsoleScrolling completely if it will not needed anymore? I think that makes more sense after all.

Ok. How about I merge this PR and then we do a second PR to remove that since it's technically a breaking change and should be handled carefully?

@BDisp
Copy link
Collaborator

BDisp commented Jul 25, 2023

Ok. How about I merge this PR and then we do a second PR to remove that since it's technically a breaking change and should be handled carefully?

I agree. Assign it to you and open a new one for the status bar issue or already exist an issue with that?

@tig
Copy link
Collaborator Author

tig commented Jul 25, 2023

See #2767

@tig tig merged commit 0d6b0b5 into gui-cs:develop Jul 25, 2023
1 check passed
tig added a commit to tig/Terminal.Gui that referenced this pull request Jul 26, 2023
tig added a commit that referenced this pull request Aug 9, 2023
…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 <bd.bdisp@gmail.com>
Co-authored-by: adstep <stephensonadamj@gmail.com>
@tig tig deleted the v1_fixes_2634_WT_statusbarstays branch April 3, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1 For Issues & PRs targetting v1.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows Terminal - Last line of app is not cleared when exiting
2 participants