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 support ANSI sequences natively (e.g. Virtual Terminal Sequences on Windows) #2610

Open
2 tasks
tig opened this issue May 8, 2023 · 9 comments
Labels
Milestone

Comments

@tig
Copy link
Collaborator

tig commented May 8, 2023

tig added a commit to tig/Terminal.Gui that referenced this issue May 8, 2023
@tig tig added the bug label May 10, 2023
@tig tig changed the title WindowsDriver (because WindowsTerminal) doesn't deal with non-BMP codepoints Rewrite WindowsDriver to use Console Virtual Terminal Sequences (because WriteConsoleOutput doesn't support non-BMP codepoints) May 10, 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 added a commit that referenced this issue Aug 11, 2023
* 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 <tig@users.noreply.github.com>
Co-authored-by: BDisp <bd.bdisp@gmail.com>
Co-authored-by: adstep <stephensonadamj@gmail.com>
@tig tig changed the title Rewrite WindowsDriver to use Console Virtual Terminal Sequences (because WriteConsoleOutput doesn't support non-BMP codepoints) Rewrite WindowsDriver to use Console Virtual Terminal Sequences Oct 26, 2023
@tig tig changed the title Rewrite WindowsDriver to use Console Virtual Terminal Sequences Refactor ConsoleDrivers to support ANSI sequences natively (e.g. Virtual Terminal Sequences on Windows) Jan 19, 2024
@tig tig mentioned this issue Jan 25, 2024
2 tasks
@dodexahedron
Copy link
Collaborator

So I've been doing a deep dive into the win32 console APIs and the frustratingly fractured/incomplete documentation available for virtual terminal sequences...

I think it's a very excellent goal for TG, and helps make things a bit more consistent, cross-platform.

But, a little voice stopped me and gave me a reality check that I just want to relay, for consideration:

Unless significant work that blocks other work has been done on it, perhaps that's something that might make sense for a VNext?

If done right, it's just internal implementation details, so we should be able to handle it in a way that doesn't affect the external API of TG. So, if following SemVer, that could even be acceptable as a point release, if desired.

Just a thought, since it's a pretty significant undertaking and there's value in a carefully thought-out approach to the implementation, so we don't have to iterate on it too much or work ourselves into avoidable pigeonholes. 🤷‍♂️

@Decimation
Copy link

Decimation commented Feb 9, 2024

@dodexahedron Spectre.Console has a robust cross-platform implementation with VTS support. It's similar to Terminal.Gui but is more focused on discrete shell controls. I think its implementation could be used as reference for implementing similar cross-platform VTS support for Terminal.Gui.

@dodexahedron
Copy link
Collaborator

Thanks. :)

I know @tig is aware of that library or may have even looked into it at least on some level, before.

I'm not sure what the current thoughts are on how to take advantage of that kind of thing, be it referencing, using code from parts of it, or drawing inspiration from it. I only came on board recently and have been working on other things, but this stuff is probably going to be at least incidentally relevant to what I'm working on now at some point, so it's good to keep this kind of thing in mind.

There are several other libraries out there on nuget, as well, that have potential application here in whole or in part for various bits of the application, but I'm pretty sure we do at least want to keep hard package dependencies to a minimum, if we can do so without it being too heinous and too much pointless duplication of essentially identical work.

@Decimation
Copy link

You're welcome. I've worked with VTS myself so I understand how it works to an extent.

Here is additional information:

  • I wrote my own shell UI which uses VTS in both native and managed console functions.
  • Pastel is another library to look into: it applies VTS to individual string objects and has a feature to automatically configure the console environment.

I can contribute to this feature if possible/feasible.

@tig
Copy link
Collaborator Author

tig commented Feb 10, 2024

There's no reason I can see that support for vts can't be isolated in just a driver.

And y'all are probably not considering a huge piece of all of this: rendering output is about 20% of the challenge. Spectre does not have mouse support and very limited keyboard support.

@tig
Copy link
Collaborator Author

tig commented Feb 10, 2024

Oh, and curses is Bad.

It is mostly in the way and my vision is we can fix a bunch of Linux/Mac issues by minimizing how much we use it. We'd use curses only for keyboard and mouse input and use the same rendering support we build for "vts" there.

Hence the name of the driver in this VTS branch being "ANSI Driver".

@dodexahedron
Copy link
Collaborator

I have a thought...

Might be a good idea to use a name not including ANSI because of Microsoft's historic decision to use that moniker for when they really meant ASCII in win32, just for sake of avoiding confusion, especially around native method calls, which typically shouldn't be the A versions.

@dodexahedron
Copy link
Collaborator

There's no reason I can see that support for vts can't be isolated in just a driver.

And y'all are probably not considering a huge piece of all of this: rendering output is about 20% of the challenge. Spectre does not have mouse support and very limited keyboard support.

Mostly agreed, here, at a high level, without having delved into these things in depth in this context yet.

My general thinking is just that we do our best to strike a balance between not reinventing as many wheels as possible and keeping dependencies (with all the caveats they can entail, too) to a reasonable minimum.

And remember: Any other project with a compatible license that has something useful but which we don't want or need the whole library for can be utilized via just lifting specific code, so long as the license is complied with (which is pretty simple with MIT). And then we can further modify any such code to fit better here, as well.

@dodexahedron
Copy link
Collaborator

Oh, and curses is Bad.

It is mostly in the way and my vision is we can fix a bunch of Linux/Mac issues by minimizing how much we use it. We'd use curses only for keyboard and mouse input and use the same rendering support we build for "vts" there.

Hence the name of the driver in this VTS branch being "ANSI Driver".

Ha. Yes. Curses has plenty of idiosyncrasies, legacy debt, etc., just like win32 does.

These libraries are old and grew up largely in a vacuum, with a lot of unfortunate results, from hacks to straight-up odd choices, especially for use in the context of a modern project like this one in 2024. It's also a dependency that isn't guaranteed to exist on a target system (though it's very likely to, since it's used by a ton of other software), so that's a small but relevant thing to keep in mind on that side.

@tig tig added this to the V2 Beta milestone May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Status: 🆕 Not Triaged
Development

No branches or pull requests

3 participants