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

Refactor ConsoleDrivers to remove duplicate code #666

Closed
tig opened this issue Jun 10, 2020 · 6 comments
Closed

Refactor ConsoleDrivers to remove duplicate code #666

tig opened this issue Jun 10, 2020 · 6 comments
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...)

Comments

@tig
Copy link
Collaborator

tig commented Jun 10, 2020

There is a LOT of common code across the three (now four with Fake) Drivers.

I was just thinking about how to fix bugs related to non-printable chars causing rendering issues on some terminals. For example, in Windows Terminal and Fluent if AddRune gets called with \n it does this:
image

The fix is to modify AddRune to look for non-printable chars like this, that may cause the underlying console to take action and replace them with the right glyph. E.g:

image

However, it's stupid to do this 4 times. Instead the base should implement the least-common-denominator functionality.

Right?

@tig tig added the design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) label Jun 10, 2020
@BDisp
Copy link
Collaborator

BDisp commented Jun 10, 2020

Right. @migueldeicaza want to do all this stuff in the View, so drivers doesn't have to be magic. The View must send the right format to the AddStr and AddRune.

@tig
Copy link
Collaborator Author

tig commented Jun 10, 2020

What if we did this:

  • Gave View a Text property. Default Redraw would draw Text using TextAlignment
  • Refactored Label such that it was nothing more than a renaming of View
  • Refactored Button, etc... to leverage all the text handling in View.
  • Window would need to leverage this too.

???

@BDisp
Copy link
Collaborator

BDisp commented Jun 10, 2020

I agree. We also have to redirect the Driver property to the View itself in order to deviate all the call to the Driver.AddRune and Driver.AddStr to the View and avoiding the views derived from View do call Application.Driver.
I think that this have to change to another way:
public static ConsoleDriver Driver { get { return Application.Driver; } }
to like:
public static ConsoleDriver Driver { get { return View.Driver; } }
But I'm still studing. More thoughts???

@migueldeicaza
Copy link
Collaborator

I think this makes sense.

The abstract capability is more of an indicator that some holes need to be plugged by concrete implementations, but we could move common code here.

@BDisp
Copy link
Collaborator

BDisp commented Aug 3, 2021

I think is already fixed with your creating of the public static Rune MakePrintable (Rune c) method and the TextFormatter class.
So, besides the ConsoleDriver can and must have common code, it must be abstract.

@tig tig added this to the v2.0 milestone Aug 4, 2022
@tig tig changed the title ConsoleDriver: Should it really be abstract? Refactor ConsoleDrivers to remove duplicate code Aug 4, 2022
@tig tig removed this from the v2.0 milestone Feb 28, 2023
tig added a commit that referenced this issue 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
Copy link
Collaborator Author

tig commented Aug 9, 2023

Woo-hoo! Finally!

It ain't perfect, but this radically simplifies and improves the ability to prove quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...)
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

3 participants