Support 256color

[jixiuf]

#58

[suonlight]

It works fine for me. Thank you @jixiuf

[Sbozzolo]

I tried this patch. I don't see any changes in the visual appearance (which is still broken with solarized, the theme I am using).

[suonlight]

@Sbozzolo: What is your term currently? echo $TERM to know that.

[Sbozzolo]

xterm-256colors

[suonlight]

My term is export TERM=xterm-256color. Can you remove s at the end of your term?

[Sbozzolo]

Actually, I miscopied, it is without the "s".

[suonlight]

You use curl -s https://gist.githubusercontent.com/HaleTom/89ffe32783f89f403bba96bd7bcd1263/raw/ | bash to test 256 colors.

[Sbozzolo]

I see 256 colors, but some of them look different if a run the script with ansi-term.

[suonlight]

Do you set ansi colors like (setq ansi-color-names-vector ["#292b2d" "#ce527a" "#2d9474" "#bfa325" "#4e97d6" "#bb6dc3" "#299ba2" "#e4e4e4"])

[suonlight]

I have set the same ansi colors for both emacs and iterm

[Sbozzolo]

I don't set anything. I run everything inside emacs. I use eterm-256color for providing ansi-term with the support for 256 colors. Anyhow, I was hoping that this PR addressed the issue discussed in #56, but it seems that it is not the case. Thanks for the support!

…

On Mon, Apr 22, 2019, 21:15 Minh Nguyen-Hue ***@***.***> wrote: I have set the same ansi colors for both emacs and iterm — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#82 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACF6E7ITKCC6CALPI7CH7D3PR2EMZANCNFSM4HHQRPBA> .

[jixiuf]

I think the 8 color in ansi-color-names-vector is not enough
we need define two face for default background and default foreground

https://github.com/akermu/emacs-libvterm/blob/master/vterm-module.c#L460
https://github.com/akermu/emacs-libvterm/blob/master/vterm-module.c#L464
is not correct

maybe we need revert 39d4c44 and 976c3f2
maybe we can do something like ansi-term-color-vector in term.el

;;; Faces
(defvar ansi-term-color-vector
  [term
   term-color-black
   term-color-red
   term-color-green
   term-color-yellow
   term-color-blue
   term-color-magenta
   term-color-cyan
   term-color-white])

EDIT:
actually ansi-term use ansi-term-color-vector in term.el
and vterm use ansi-color-names-vector in ansi-color.el.gz
the default value of them is different
now I add vterm-color-palette and vterm-color-default ,you can custom these faces

the default value of faces in vterm-color-palette is inherited from ansi-color-names-vector

@Sbozzolo you can custom vterm faces after load your theme like this

(set-face-attribute 'vterm-color-black nil :background nil :foreground nil :inherit 'term-color-black)
(set-face-attribute 'vterm-color-red nil :background nil :foreground nil :inherit 'term-color-red)
(set-face-attribute 'vterm-color-green nil :background nil :foreground nil :inherit 'term-color-green)
(set-face-attribute 'vterm-color-yellow nil :background nil :foreground nil :inherit 'term-color-yellow)
(set-face-attribute 'vterm-color-blue nil :background nil :foreground nil :inherit 'term-color-blue)
(set-face-attribute 'vterm-color-magenta nil :background nil :foreground nil :inherit 'term-color-magenta)
(set-face-attribute 'vterm-color-cyan nil :background nil :foreground nil :inherit 'term-color-cyan)
(set-face-attribute 'vterm-color-white nil :background nil :foreground nil :inherit 'term-color-white)

;;maybe need after  (require 'term)
(set-face-attribute 'vterm-color-default nil :background nil :foreground nil :inherit 'term)

[Sbozzolo]

I checked out the latest commit, and I was extremely happy to see that it works!
I didn't add any of @jixiuf customizations.

Screenshot to compare XTerm, VTerm and term:

[Sbozzolo]

Small problem with htop:

(Top pane is vterm, some strings are unreadable. Bottom is xterm)

[jixiuf]

can not reproduced with theme color-theme-sanityinc-solarized @Sbozzolo

[Sbozzolo]

This is my setting for the theme. I have nothing relevant in the `custom.el`. ``` (use-package color-theme-sanityinc-solarized :config (load-theme 'sanityinc-solarized-dark t) ;; Flat modeline (set-face-attribute 'mode-line nil :box nil) (set-face-attribute 'mode-line-inactive nil :box nil) ;;(load-theme 'sanityinc-solarized-light t) ) ``` As you see, I don't do anything peculiar. jixiuf writes:

…

can not reproduced with theme color-theme-sanityinc-solarized @Sbozzolo — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.*

[jixiuf]

can you try to run make run in emacs-libvterm/build directory

and install color-theme-sanityinc-solarized

[Sbozzolo]

I performed some tests. If load emacs with the theme and vterm only, then I get the screenshot above. If I load emacs with vterm, and once emacs is running I load the theme, then the colors are slightly different, and the column name in htop are readable.

Do you have suggestions for understanding what is going on?

[jixiuf]

trying change the order of loading vterm and your theme, and see what's going on ,
and trying load your theme in after-init-hook and see what's going on

(add-hook 'after-init-hook (lambda () (load-theme 'solarized-light)))

[Sbozzolo]

Changing order, or using the lambda you posted changes nothing.
However, I noticed that if I remove my .Xresources, both the scenarios (loading in the init or after) yield the same result: wrong colors.

[akermu]

@jixiuf Is this ready to be reviewed or are you still working on this?

[jixiuf]

I rebased the code ,and now is ready to be reviewed

[qhuyduong]

@jixiuf I'm facing this issue with Emacs 26.2. I'm using latest code. Any idea?

Error running timer ‘vterm--delayed-redraw’: (void-function vterm--get-color) [2 times]
Error during redisplay: (vterm--window-size-change-26 #<frame vterm<2> – Doom Emacs 0x10614dcb8>) signaled (void-function vterm--get-color)

[jixiuf]

@jixiuf I'm facing this issue with Emacs 26.2. I'm using latest code. Any idea?

Error running timer ‘vterm--delayed-redraw’: (void-function vterm--get-color) [2 times]
Error during redisplay: (vterm--window-size-change-26 #<frame vterm<2> – Doom Emacs 0x10614dcb8>) signaled (void-function vterm--get-color)

@qhuyduong
are you sure you are using the last version vterm.el in this pull request,
vterm--get-color is defined in vterm.el
make sure you are loading the right vterm.el

[qhuyduong]

@jixiuf my bad. I just build and update vterm-module.so without updating vterm.el. Thank you very much :D

[Sbozzolo]

I don't know if the following comment is needed/wanted/welcomed, but:
I've been using this patch on a daily basis for several weeks now.
While the color rendering is still not perfect, on my system it is a massive improvement compared to the out-of-the-box experience.

[jixiuf]

after last commit ,the bug should be fixed.
because the color are inherited from term.el ,so we should require term.el .
I don't know is this a good idea.

or maybe we can create PR to different theme packages for this package instead of require term.el

[Sbozzolo]

I checked out the latest commit. There are still imperfections in the rendering.
For instance, see the attached screenshot, there is an autosuggestion that is barely visible.
What should happen is that the suggested continuation should be lighter compared to the command already typed in, with the previous commit it was of the same weight, as you can see in the second attached screenshot. What I would like is more akin the last screenshot (where it is clear that there is a suggestion).

Btw: thanks for all your contributions to this package, they are invaluable!

[jixiuf]

I compare vterm and iterm2 ,and found they both are imperfections in the rendering ,like the first screenshot you attached. so I thinks it should be the problem of the theme.
Btw: is the third screenshot using solarized theme, and I found the colors are different with screenshot1 and screenshot2.

[jixiuf]

rebase the code.

[akermu]

Very nice, thanks a lot!