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

SerializeAsHTML() does not work with xterm-headless - TerminalCore lacks _colorManager #4148

Closed
gpickell opened this issue Sep 26, 2022 · 9 comments · Fixed by #4196 or #4348
Closed
Assignees
Labels
Milestone

Comments

@gpickell
Copy link

I tried to use the Terminal from xterm-headless with the xterm-addon-serialize addon. I managed to get it to work after I hacked in a property. See the following code fragment:

import xterm from "xterm-headless";
import { SerializeAddon } from "xterm-addon-serialize";

const term = new xterm.Terminal({
    allowProposedApi: true,
});

const ser = new SerializeAddon();

// HACK: Make it work (omit to demonstrate error)
term._core._colorManager = {
    colors: {
        ansi: {
            1: {
                css: "blue",
            }
        }
    }
};

term.loadAddon(ser);
term.write("Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ");
term.write("", () => {
    // this will throw without the hack
    console.log(ser.serializeAsHTML({
        includeGlobalBackground: true
    }));
});

The ColorManager should get moved to common or this addon package should bundle a copy of it and use it only if there is no _colorManager.

@Tyriar
Copy link
Member

Tyriar commented Sep 26, 2022

I'm not surprised this doesn't work, the reason ColorManager lives in browser is because color doesn't need to be displayed to the user in the headless version. What's your use case for this?

@gpickell
Copy link
Author

Pre-rendered CLI tool output presentation as simple HTML. There may be other ways to accomplish this, but xtermjs is the best for handling problems in this area it seems?

@Tyriar
Copy link
Member

Tyriar commented Sep 26, 2022

I don't think we should move ColorManager to common such that it would be included in xterm-headless since nothing in that package uses it. However we could adapt the serialize addon to support not having a color manager. So changing this:

// https://github.com/xtermjs/xterm.js/issues/3601
this._colors = (_terminal as any)._core._colorManager.colors;

To include a list of colors like this generates:

// An IIFE to generate DEFAULT_ANSI_COLORS.
export const DEFAULT_ANSI_COLORS = Object.freeze((() => {
const colors = [
// dark:
css.toColor('#2e3436'),
css.toColor('#cc0000'),
css.toColor('#4e9a06'),
css.toColor('#c4a000'),
css.toColor('#3465a4'),
css.toColor('#75507b'),
css.toColor('#06989a'),
css.toColor('#d3d7cf'),
// bright:
css.toColor('#555753'),
css.toColor('#ef2929'),
css.toColor('#8ae234'),
css.toColor('#fce94f'),
css.toColor('#729fcf'),
css.toColor('#ad7fa8'),
css.toColor('#34e2e2'),
css.toColor('#eeeeec')
];
// Fill in the remaining 240 ANSI colors.
// Generate colors (16-231)
const v = [0x00, 0x5f, 0x87, 0xaf, 0xd7, 0xff];
for (let i = 0; i < 216; i++) {
const r = v[(i / 36) % 6 | 0];
const g = v[(i / 6) % 6 | 0];
const b = v[i % 6];
colors.push({
css: channels.toCss(r, g, b),
rgba: channels.toRgba(r, g, b)
});
}
// Generate greys (232-255)
for (let i = 0; i < 24; i++) {
const c = 8 + i * 10;
colors.push({
css: channels.toCss(c, c, c),
rgba: channels.toRgba(c, c, c)
});
}
return colors;
})());

@silamon
Copy link
Contributor

silamon commented Sep 26, 2022

If terminal.options.theme is filled in in headless mode, we can use that.

@Tyriar
Copy link
Member

Tyriar commented Sep 26, 2022

@silamon I think that's initialized to {} by default?

But even if it had more of the colors, I don't think it would have extendedAnsi which is important for serializing to HTML.

@jerch
Copy link
Member

jerch commented Sep 26, 2022

Maybe the html output of the serialize addon should emit color slots as css classes? This way a theme can be applied to it from a css file defining those colors (except RGB, which can always be derived from CellData).

Note - I have not looked at the serialize code, thus idk what it needs the color manager for.

@Tyriar
Copy link
Member

Tyriar commented Sep 26, 2022

@jerch i think a common use case would be to put the html into the clipboard and allow pasting into a word doc or something that supports rich text (vscode does this). Going class based wouldn't work as then it's not self contained.

@jerch
Copy link
Member

jerch commented Sep 26, 2022

Well if thats an issue, how about a style/theme argument to serailizeAsHtml containing the colors? Then the code can inline them.

Edit: To me it seems pretty obvious not to move the color manager to headless, as there is no notion of colors in headless needed anywhere else.

@Tyriar
Copy link
Member

Tyriar commented Dec 19, 2022

This had to get reverted for v5.1 as we can't package implementation from core in the serialize addon yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment