Skip to content

/ xterm.js

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

Jump to bottom

xterm-addon-serialize ignores cursor visibility #3364

Open
cyrus-and opened this issue Jun 9, 2021 · 3 comments
Open

xterm-addon-serialize ignores cursor visibility #3364

cyrus-and opened this issue Jun 9, 2021 · 3 comments
Labels
area/addon/serialize good first issue help wanted type/enhancement

Comments

@cyrus-and
Copy link

@cyrus-and cyrus-and commented Jun 9, 2021

ANSI codes to hide (\x1b[?25l) and show (\x1b[?25h) the cursor are not retained during the serialization. I understand that this concerns more the current state of the terminal, but since the cursor position is preserved, I find it natural to also save the cursor visibility.

If this is a wontfix, is there at least a reliable way to detect the cursor visibility? Other than the undocumented:

terminal._core._inputHandler._coreService.isCursorHidden

Also, are there other instances of this scenario that I should be aware of?

Details

  • Browser and browser version: Google Chrome 91.0.4472.77 (Official Build) (x86_64)
  • OS version: macOS Big Sur 11.4
  • xterm version: 4.12.0 (latest)
  • xterm-addon-serialize version: 0.5.0 (latest)

Steps to reproduce

package.json

{
    "dependencies": {
        "xterm": "4.12.0",
        "xterm-addon-serialize": "0.5.0"
    }
}

index.html

<!doctype html>
<html>
    <head>
        <link rel="stylesheet" href="node_modules/xterm/css/xterm.css" />
        <script src="node_modules/xterm/lib/xterm.js"></script>
        <script src="node_modules/xterm-addon-serialize/lib/xterm-addon-serialize.js"></script>
    </head>
    <body>
        <div id="terminal"></div>
        <script>
            const terminal = new Terminal();
            const serializeAddon = new SerializeAddon.SerializeAddon();
            terminal.loadAddon(serializeAddon);
            terminal.open(document.getElementById('terminal'));
            const data = 'hello\x1b[?25l';
            terminal.write(data, () => {
                const serializedData = serializeAddon.serialize();
                alert(JSON.stringify(serializedData));
            });
        </script>
    </body>
</html>

  1. npm install

  2. open index.html

The alert shows "hello" (hence without any ANSI code) but the cursor in the terminal is hidden.
@jerch
Copy link
Member

@jerch jerch commented Jun 9, 2021 •
edited

@cyrus-and Imho the serialize addon does not preserve any terminal state beside the buffer data currently, it simply got not implemented yet. Things that would need some care:

  • all SM/DECSET related flags (cursor hidden belongs here, normal/alternate buffer, mouse protocols and several more)
  • OSC related overrides (custom colors and such)
  • cursor shape
  • several more internal states, that can only be set via ctor args (not sure, if we should make those data-scriptable, some are security related like windowOps, others like the renderer setting should not be data driven). Guess this will need a case to case decision.

Most of the SM/DECSET related flags can be added at the end of the data stream, e.g. \x1b[?25l to hide the cursor. Others need to be added in place, like alternate buffer switch. If you want to look into these things, feel free to do a PR.
@cyrus-and
Copy link
Author

@cyrus-and cyrus-and commented Jun 9, 2021

Thank you @jerch for the quick reply. I ended up implementing this workaround for the time being:

let ansi = serializeAddon.serialize();
const {isCursorHidden} = terminal._core._inputHandler._coreService;
ansi += isCursorHidden ? '\x1b[?25l' : '\x1b[?25h';
@Tyriar
Copy link
Member

@Tyriar Tyriar commented Jun 9, 2021

We accept PRs 😉

This seems like a pretty straightforward API to add to support it in the serialize addon:

interface IBuffer {
  readonly isCursorHidden: boolean;
}
cyrus-and added a commit to cyrus-and/ratty that referenced this issue Jun 11, 2021
@cyrus-and
Keep track of the cursor visibility
443da06 
This has to be performed manually as xterm-addon-serialize does not take
care of that, see: xtermjs/xterm.js#3364
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/addon/serialize good first issue help wanted type/enhancement
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@cyrus-and @Tyriar @jerch