search

tinted-theming / base16-emacs

Base16 themes for Emacs
MIT License
382 stars 76 forks source link

ansi-term-color-vector improperly overridden #109

Closed tadfisher closed 4 years ago

tadfisher commented 4 years ago

Since Emacs 24 at least, ansi-term-color-vector is a vector of faces, not color strings.

Here's the definition of ansi-term-color-vector: https://github.com/emacs-mirror/emacs/blob/emacs-24/lisp/term.el#L774-L783

Here are the definitions of the faces in that vector: https://github.com/emacs-mirror/emacs/blob/emacs-24/lisp/term.el#L774-L783

And here is where M-x ansi-term crashes when I use a base16 theme: https://github.com/emacs-mirror/emacs/blob/emacs-24/lisp/term.el#L3248-L3250

It appears you already set the foreground and background of the term faces, so the code setting ansi-term-color-vector should be entirely unnecessary.

Note that this is essentially a duplicate of this bug: https://github.com/belak/base16-emacs/issues/4.

belak commented 4 years ago

Oh wow, good call. That issue seems to be from before my time. So, you're saying that just removing the code to set ansi-term-color-vector should be enough? Honestly that would be nice because it simplifies the code a bit.

tadfisher commented 4 years ago

Yep! If you want to support older Emacs versions, you can wrap it in a version< check like in my PR.