search

romgrk / termrk

Terminal for atom, using pty.js & term.js
MIT License
33 stars 7 forks source link

Custom colors #78

Closed olmokramer closed 8 years ago

olmokramer commented 8 years ago

This PR adds the terminalColors setting. It is an array of 8, 10, 16 or 18 CSS color strings that is passed directly to the Terminal constructor as the colors option. The first 8 colors are used for the \x1b[0;XXm escape characters where XX = [ 30 .. 37 ]. If 8 other colors are present, those are used for the \x1b[1;XXm escape characters. In both cases, if 2 more colors are present they will be used as background and foreground color, in that order.

It doesn't validate if the entries of the array are valid color strings. One way around this is to let Atom's Config class do this, but currently Atom doesn't support an array of colors as the config type. (It accepts it and returns the correct array, but it shows nothing in the settings view). That, in turn, could be solved by switching from an array of colors to an object with colors, but then we'd lose the order of the colors.

I upgraded term.js because there was a bug in v0.0.4 when the colors array had 18 entries.

romgrk commented 8 years ago

All good! I'll review later tonight. About term.js, I was thinking moving away from it. The code is a mess, and the repo isn’t maintained enough. Interesting features like cursor shapes and truecolor are missing.

Also https://github.com/chjj/term.js/issues/117 seems to be pointing to https://github.com/sourcelair/xterm.js. (which looks good)

olmokramer commented 8 years ago

Awesome! If you decide to move to xterm.js, you can ignore this PR altogether, because xterm just adds classes to the colored strings, so we can customize them from our ~/.atom/styles.less :) And the code in this PR is quite dependent on the inner workings of term.js, which is something to avoid as much as possible. That said, it looks like xterm.js used term.js as a basis (or the other way around) and that this PR will work with either of them.

romgrk commented 8 years ago

All good! (Sorry I got caught by VS Code; have you checked it out? It's awesome) Just a detail, could you update the changelog? I'll merge and publish to apm as soon as it's done.

For xterm.js migration, it's not on my immediate tasklist, so it might take a few weeks before I do the change.

(Oh and also: nice work with the git branching package, works swiftly!)

olmokramer commented 8 years ago

Nope, never checked it out. I might, though, I hear good things about it! Anyway, changelog's updated and I also added a description to the settings view, because it was just an incomprehensible array of random colors...

Thanks! and also thanks for termrk, I simply loved it from the moment I saw my good ol' zsh configuration inside Atom :)

romgrk commented 8 years ago

Published. :+1:

olmokramer commented 8 years ago

:tada: Thanks!