-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
add support for 24-bit RGB color escape sequences #112
Conversation
This is something at least, although you didn't read my previous
On Thu, Sep 17, 2015 at 01:41:31PM -0700, Suraj N. Kurapati wrote:
|
Thanks @sunaku for picking this up! Also thanks @nicm for the helpful review comments!
Interesting idea, would that mean bypassing the problem of the terminfo maintainer? Should other applications check for that Tc entry too? |
We will need some way to detect it, and there is no point in proposing a flag to Thomas Dickey that we might need, or might not, and we aren't even using. Once we have code and know that adding a flag is the best solution for tmux, we can see what he thinks - perhaps he will suggest something else, or perhaps we will have to keep it as an extension, or do something else. There is no problem yet. |
On Thu, 17 Sep 2015 14:20:25 -0700, Nicholas Marriott wrote:
Sorry about that! This patch was indeed a work in progress; I was
Agreed, will do.
How about storing colors in 32-bit unsigned integers as 0xRRGGBBII? The upper bytes will hold the red, green, and blue channel values. The lowest byte will hold either (1) the 256-color index or (2) the
It seems that this scheme allows us to forgo storing the grid_cell In addition, this scheme will eliminate struct copying and simplify
Thanks, I was searching the web for the name of an existing 24-bit
Since we first try emitting RGB and fall back to 256-color anyway,
Thanks, I'll look into it.
Thanks for your feedback and patience. I will push additional |
I don't think it will be better to use 32 bits for the colour. It is a This is a real concern - for example I have 6 MB of history here with I did plan to have a separate array of long cells where if the character As far as 256 and RGB being mutually exclusive - you should never have a On Sat, Sep 19, 2015 at 03:17:45PM -0700, Suraj N. Kurapati wrote:
|
On Sun, 20 Sep 2015 01:36:27 -0700, Nicholas Marriott wrote:
That makes sense; agreed.
Would you like me to pursue this route? Or perhaps you might
In that case, what should happen when a 24-bit color is specified Do we dynamically degrade the 24-bit color down to the next highest |
On Tue, Sep 22, 2015 at 10:20:33PM -0700, Suraj N. Kurapati wrote:
I suggest doing it later but for now keep grid_cell as small as
Yes, except only 256 and 16/8 colours. You probably just need to add |
This patch adds support for 24-bit RGB color escape sequences, requested in issue tmux#34, by adjusting Koichi Shiraishi's patch [1], which derives from Christian Hopps's patch [2], which in turn derives from Arnis Lapsa's patch [3], according to Nicholas Marriott's feedback [4][5]. [1]: https://gist.github.com/zchee/9f6f2ca17acf49e04088 [2]: https://gist.github.com/choppsv1/dd00858d4f7f356ce2cf [3]: https://gist.github.com/ArnisL/6156593 [4]: tmux#34 (comment) [5]: tmux#112
This patch adds support for 24-bit RGB color escape sequences, requested in issue tmux#34, by adjusting Koichi Shiraishi's patch [1], which derives from Christian Hopps's patch [2], which in turn derives from Arnis Lapsa's patch [3], according to Nicholas Marriott's feedback [4][5]. [1]: https://gist.github.com/zchee/9f6f2ca17acf49e04088 [2]: https://gist.github.com/choppsv1/dd00858d4f7f356ce2cf [3]: https://gist.github.com/ArnisL/6156593 [4]: tmux#34 (comment) [5]: tmux#112
On Wed, 23 Sep 2015 00:44:58 -0700, Nicholas Marriott wrote:
Hmm, in my review of tty.c, that doesn't exactly seem to be the case: As a result, we may lose colour information when, say, specifying a Is this correct? P.S. I have force-pushed an updated patch implementing |
No: tty_check_fg, tty_check_bg and tty_colours all operate on a On Sat, Sep 26, 2015 at 03:29:39PM -0700, Suraj N. Kurapati wrote:
|
@nicm on github when you force push a PR the existing PR is updated with the new overwritten commits. |
On Sat, 26 Sep 2015 16:25:12 -0700, Nicholas Marriott wrote:
Thanks for the hint! Will do.
Sure, here is the colourful web version: Or if you like, the traditional version: These links always reflect the current state of the patch / PR. |
Hi Thanks. Some comments: You don't need the changes to the input_transition structs. I think where you have:
It would be better just as gc->flags &= ~(GRID_FLAG_BG256|GRID_FLAG_BGRGB). I don't like defining the struct inside the union, define it outside I think the big check at the start of tty_colours could be broken up to changed = 0; Perhaps this bit could set multiple flags since basically the same Otherwise looks good so far. |
Running Neovim with NVIM_TUI_ENABLE_TRUE_COLOR defined in vanilla tmux <= 2.0 triggers this issue. Without that variable, Neovim will produce poor but legible colouring. Vim works flawlessly either way. This commit sets the relevant environment variable in vimrc unconditionally, where before it was set only for xterm*. Although the variable better belongs elsewhere, this option is most forgiving of my work flow: an alias would not be picked up by third-party applications (e.g. Git), necessitating further changes, and an external environment variable would behave unpredictably in similar ways -- tig, for instance, could not work with an adjusted $VISUAL. The advantage of this approach is that Vim will ignore the setting and Neovim will always enable true-colour support. The downside is that Neovim will always enable true-colour support. That won't play nicely with tmux core until they add support, too, but until then it can be built from source. Progress on adding true-colour support to tmux is being made [1], but compiling that fork did not produce a binary that actually did support true colours -- it behaved similarly to the unpatched version. Until this work is finalised, a functional stand-alone patch and instructions for applying it to tmux 2.0 are available instead [2]. A change in [3] conditionally set the environment variable in vimrc. At the time, that produced as accurate colouring as possible; but it is likely that an alias to Neovim, also setting that variable, was missed, since on a fresh system that change alone turned out to be insufficient. All of this applies to terminals without 24 bit colour support as well. [1] tmux/tmux#112 [2] https://gist.github.com/commonquail/643f64edbe58d5702844 [3] 658568e
I tried this on 2.1 and current master and it did not work. Colors in 256 color mode worked fine but 24bit/truecolor was black/white. There was also a gray color coming in sometimes, like when splitting, one of the splits would get a gray background color. |
Just tried building this branch on ubuntu. Here's what I get: http://d.pr/i/1fI1N right after tmux start (gray scree, already in copy mode). |
@sunaku how're we doin over there? 😋 |
😰 Free time is scarce, I'll try this week. ⏳ |
@sunaku no rush man. We've waited this long. 😀 I know how it is. I'll just ping you every once in a while so you don't forget. 😉 |
@bergman @bsod90 This patch "did not work" for you because it has evolved beyond the previous ones (upon which it builds) to properly require your terminal to advertise a For the time being, you'll need to fake the
Don't give up on progress! 🚀 |
@nicm I've been working through your suggestions but I've hit a roadblock regarding how the
but the Any ideas? 😰 Thanks for your consideration. |
After tmux starts does "show -s terminal-overrides" have your additions? But "tmux info" shows Tc as missing? It looks fine to me, you should try adding some debugging to You should use tty_term_flag to check Tc, not tty_term_has. And the On Thu, Oct 22, 2015 at 08:44:19PM -0700, Suraj N. Kurapati wrote:
|
Confirmed working after adding I do have some weird stuff happening, like below, where colors are messed up after the first colored output and even more messed up after first background colored output: Compare 2.0 with choppsv1's patch For anyone wanting to try it out with homebrew: diff --git a/Library/Formula/tmux.rb b/Library/Formula/tmux.rb
index 44d0dd1..a2e3232 100644
--- a/Library/Formula/tmux.rb
+++ b/Library/Formula/tmux.rb
@@ -22,6 +22,12 @@ class Tmux < Formula
depends_on "pkg-config" => :build
depends_on "libevent"
+ option "with-truecolor", "Build with truecolor patch enabled"
+ patch do
+ url "https://github.com/tmux/tmux/pull/112.diff"
+ sha1 "eac7a6d755df770257267f131bffffa1dfd94f29"
+ end if build.with? "truecolor"
+
def install
system "sh", "autogen.sh" if build.head?
|
@nicm Thanks, this is truly weird. 😨 The overrides are taking effect only after I restart tmux: $ ./tmux -vvv -L test -f /dev/null
# (now I'm inside the tmux session)
$ ./tmux show -s terminal-overrides
terminal-overrides "xterm*:XT:Ms=\E]52;%p1%s;%p2%s\007:Cs=\E]12;%p1%s\007:Cr=\E]112\007:Ss=\E[%p1%d q:Se=\E[2 q,screen*:XT"
$ ./tmux info | grep Tc
201: Tc: [missing]
$ ./tmux set-option -ga terminal-overrides ',xterm*:Tc'
$ ./tmux info | grep Tc
201: Tc: [missing]
$ exit Now I restart tmux and this time it somehow remembers 👻 the overrides I previously set: $ ./tmux -vvv -L test -f /dev/null
# (now I'm inside the tmux session)
$ ./tmux show -s terminal-overrides
terminal-overrides "xterm*:XT:Ms=\E]52;%p1%s;%p2%s\007:Cs=\E]12;%p1%s\007:Cr=\E]112\007:Ss=\E[%p1%d q:Se=\E[2 q,screen*:XT,xterm*:Tc"
$ ./tmux info | grep Tc
201: Tc: (flag) true Is this expected? 😕 |
This patch adds support for 24-bit RGB colour escape sequences in tmux, as requested in issue tmux#34, by adjusting Koichi Shiraishi's patch [1], which derives from Christian Hopps's patch [2], which in turn derives from Arnis Lapsa's patch [3], with Nicholas Marriott's assistance [4]. In order to make use of the functionality this patch provides, users must enable the "Tc" terminal capability for the outer terminal (to which tmux is attached) in tmux through the terminal-overrides option and subsequently detach and reattach tmux, as the following example (wherein $TERM is "st-256color" and % is a shell prompt) illustrates: Outside tmux: % echo $TERM st-256color % tmux attach Inside tmux: % tmux set-option -ga terminal-overrides ",st-256color:Tc" % tmux detach Outside tmux: % tmux attach The following command lets users determine whether the "Tc" terminal capability has been enabled properly for the outer terminal in tmux: tmux info | grep Tc If the command reports "Tc: [missing]", then support for 24-bit colours has not been enabled properly because the terminal-overrides option may have specified the outer terminal's $TERM value incorrectly or tmux may have been reattached to an entirely different outer terminal altogether. [1]: https://gist.github.com/zchee/9f6f2ca17acf49e04088 [2]: https://gist.github.com/choppsv1/dd00858d4f7f356ce2cf [3]: https://gist.github.com/ArnisL/6156593 [4]: tmux#112
I tried this PR in my local machine (MacBook Pro Retina 2015, iTerm2) and it rendered Neovim with 24bit colors perfectly! Thank you so much 😄 |
Applied now. |
@nicm 🎉 |
Thank you so much! 👍 On Fri, Jan 29, 2016, 13:08 Koichi Shiraishi notifications@github.com
|
🎉 |
Does it mean this was merged and tmux will have true colors support in next realease? |
@martin-svk yes, see commit 427b820 |
@XVilka thanks for clarification. That's awesome, finally I will be able to use all those fancy color schemes 👍 🎉 |
Thank you all! Especially @sunaku. I've been waiting for this for a long time. What a fantastic job. 😀 |
Thanks everyone Cheers! 🎈 🎂 🎁 🎉 😸 Let's enjoy our newfound 16 million colors! 🌈 And long live tmux! 👑 P.S. I wrote up a little story about all this (along with usage instructions, of course) on my blog. 📝 |
@sunaku absolutely right! Tmux is a tool that I use every single day and it's work like this (and continued support by @nicm, @ThomasAdam and others) that makes it continue to stay relevant and immune to the temptation for people to create forks or other competitors touting "It's like Tmux but it does x". I'm waiting for the actual stable release to use this, but I'm extremely excited. :) @nicm any idea when the next release is going out? |
On 30 January 2016 at 21:23, Jeff Felchner notifications@github.com wrote:
-- Thomas Adam |
Just a note for anyone that uses Homebrew and wants to install this patch; you can do so by uninstalling Tmux: $ brew uninstall tmux editing the formula for Tmux $ brew edit tmux and comment out references to the Bash completion files on lines $ brew install tmux --HEAD which will grab the latest commit. Of course, only do this if you're OK with using the development version of Tmux. |
@ThomasAdam thank you. |
Will this PR be included in 2.2? Or is it already in 2.1? |
@HeroCC it'll be released in the next release whatever number happens to be. |
While trying 24bit-color support, I noticed there is some issue (when comparing to non-tmux terminal) - see http://ibin.co/2YAmCLcoBNRC. |
@assaf758 You should open a separate issue report for this. This PR covered the initial implementation but if there are bugs now that it has been merged they should be reported separately. When you do please include more information about your setup including the exact commit you compiled TMUX from and what version of the terminals you are using. I am unable to replicate this using those same scripts and Termite as my terminal, so something very specific to your setup is broken and we'll need more details to track it down. |
Screenshots here: #17 Most notable change, at least with the sample colorscheme, is a more strident red, but there are some other subtle differences and this is now pretty damn close to where MacVim is. Note that in order to behave correctly, we have to cajole tmux a little: http://sunaku.github.io/tmux-24bit-color.html tmux/tmux#112
I'm not sure if this is the correct place to ask, so apologies in advance... but does this patch include support for 24-bit colour within tmux.conf as well? I tried using '00ff00' as a colour, for example, but that didn't work. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This patch adds support for 24-bit RGB color escape sequences, requested
in issue #34, by adjusting Koichi Shiraishi's patch 2, which derives
from Christian Hopps's patch 3, which in turn derives from Arnis
Lapsa's patch 4, according to feedback given by Nicholas Marriott 1.