-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support truecolor #756
Support truecolor #756
Conversation
@Tyriar Not sure about typescript (have tried, but didn't get it to work), but with standard javascript es6 you can extend the Array prototype and setup getters like this: class CharData extends Array {
constructor(char, width, flags, fg, bg) {
super(char, width, flags, fg, bg)
}
get char() {
return this[0]
}
get width() {
return this[1]
}
get flags() {
return this[2]
}
get fg() {
return this[3]
}
get bg() {
return this[4]
}
}
let char = new CharData('a', 1, 0, 1 << 24, 1 << 24);
// will log 'a 1'
console.log(char.char, char.width); You can actually copy & paste the code snippet into your chrome console and execute it, should work. Not sure about the performance of this though. |
@mofux I think the export const CHAR_DATA_FLAGS_INDEX = 3; export function getFlags(charData: CharData) {
return charData[3];
} |
With issue, do you mean that the object instantiation will be slow, or that I could write a jsperf test to see if it actually differs if we create an Array through the Imo using the getters would be the most elegant solution. |
@mofux I mean slow instantiation. I did a small jsperf that tests {} vs [] with a few values/properties each and the array was 20x faster. Checking extends Array would be useful though 👍 |
I have created a test with jsbench. Looks like using a native array is the way to go 😔 |
@mofux results are quite dramatic on my PC 😛 |
Yep 😅. Test number 5 uses a function that gets the array in and returns an object with the mapped values (like a view). let toCharData = (arr) => ({ char: arr[0], width: arr[1], flags: arr[2], fg: arr[3], bg: arr[4] }); It performs surprisingly well when compared to the direct array read method. But I'm afraid it will increase memory usage, although that might be negligible if the generated object can get garbage collected quickly (because it should not get referenced directly somewhere) |
Eww, I think V8 does some nasty optimization here and you are not measuring the "create an array with data xy" thing. Microbenchmarks are not that useful anymore in a complex environment with a JIT compiler and such. |
Very fragile right now, only works for basic IO Part of xtermjs#791
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linting fix and it's good to go!
This one still needs a bunch of testing and fixing edge cases (removing/adding lines, splitting styles if cursor is not at the bottom, etc.) |
Nah, I got too excited 😅. |
@Tyriar Short question about theming support for the base colors (0-15) when introducing true-color support: I've read the comment here https://github.com/sourcelair/xterm.js/blob/v3/src/Terminal.ts#L61 |
@mofux support for 0-255 colors is remaining, a lot of the color code will be removed though. I think the only reason for the colors in JS is for |
Closing, it's always available for reference. |
Fixes #484