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

Alternate screen buffer #244

Draft
wants to merge 2 commits into
base: xterm-emu
Choose a base branch
from
Draft

Alternate screen buffer #244

wants to merge 2 commits into from

Conversation

davidrg
Copy link
Owner

@davidrg davidrg commented Feb 16, 2024

This is an option for implementing xterms Alternate Screen Buffer (#231). The solution here is to effectively add a second vscrn, VTERM_B, and rename the existing VTERM to VTERM_A.

Both Vscrns share the same event queue, and settings that are applied to one should for the most part also be applied to the other, but they have their own videobuffers allowing us to switch cleanly between them just as we switch between VTERM and VCMD. The alternate buffer, VTERM_B, has no scrollback.

These changes also introduce a new command for controlling the alternate screen buffer - it can be enabled or disabled, and there are hidden commands to force the terminal on to or of off the alternate buffer.

I'm not entirely happy with the scale of the changes here however. Adding a new Vscrn involves adding an additional member to a lot of arrays and its not always obvious which ones should be updated. Having the two VTERMs share some settings and not others, and also share their event queues (even though VTERM_B does technically have its own event queue allocated) is messy and seems a likely source for bugs.

But I don't see a simpler option. We could perhaps instead try just swapping out the contents of VTERMs videobuffer but thats going to involve similar effort keeping the two videobuffers settings in sync and adds some difficulties around disabling scrollback on the alternate buffer, but it does remove the mess around input queues. It is however probably worth investigating this approach in a separate branch just to validate whether this path is indeed the best option.

Before merging:

  • Investigate swapping out the contents of VTERMs videobuffer as an alternative to having an entirely separate Vscrn
  • Fix the bug related to resizing the terminal when on an alternate buffer
  • Carefully review all changes to ensure:
    • All VTERM references have been checked for if they should apply to VTERM_B
    • All arrays that should contain one element per Vscrn have had the VTERM_B element added (not all of these arrays are sized by VNUM - some just have magic numbers)

Which works by adding an additional vscrn called VTERM_B which is semi-independent from VTERM (now called VTERM_A) - it has its own video buffer, but it shares its event queues with VTERM_A and any settings that are applied to VTERM_A also get applied to VTERM_B.

I don't particularly like the scale of these changes but at this stage it seems like the simplest way to achieve this. Before merging its probably worthwhile exploring some other options just in case.

There is also at least one outstanding bug related to the background colour not being applied properly to the main buffer when the window is resized on the alternate buffer.
@davidrg davidrg marked this pull request as draft February 16, 2024 09:02
@davidrg davidrg changed the base branch from master to xterm-emu February 16, 2024 09:05
@davidrg davidrg linked an issue Feb 16, 2024 that may be closed by this pull request
@davidrg davidrg self-assigned this Feb 16, 2024
@davidrg
Copy link
Owner Author

davidrg commented Sep 22, 2024

Having thought about it for a few months, I think a simpler solution will be to do a bit of refactoring. At some point I'll have a go at:

  • Create a new Vscrn struct that holds primary and alternate video buffers (videobuffer*), a pointer to the current buffer (also videobuffer*), and the vscrn type (VCMD, VTERM, etc).
  • videobuffer vscrn[VNUM] in ckoco2.c would become Vscrn vscrn[VNUM]
  • Everything that is currently expecting vscrn[x] to be a videobuffer is refactored to refer to vscrn[x]->currentBuffer
  • Alternate screen buffer switching is achieved by changing the value of currentBuffer to point to one of the two videobuffers
  • perhaps move tt_scrsize into videobuffer, or add a flag to videobuffer to indicate if its scrollable

Ideally we'd eventually move all the other values that currently live in arrays dimensioned by VNUM to values in the Vscrn struct.

Main advantages of this approach:

  • Fewer changes to fewer modules (I hope)
  • Fewer arrays enlarged unnecessarily compared to adding the alternate buffer as another Vscrn
  • Reduction in the number of global variables
  • Provides a pathway to further reduction of terminal emulation-related globals
  • Provides a pathway to terminal multiplexing
  • Generally tidier implementation (I hope)

If we eventually moved all of the terminal emulation-related globals into Vscrn, then we could support things like ANSI emulation in the command screen as well as multiplexed terminal sessions (TD/SMP over serial, multiple pty channels over one SSH connection, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alternate screen buffer
1 participant