protesilaos / modus-themes

Highly accessible themes for GNU Emacs, conforming with the highest standard for colour contrast between background and foreground values (WCAG AAA).
https://protesilaos.com/emacs/modus-themes
GNU General Public License v3.0
526 stars 29 forks source link

default foreground color is the same as background color in vterm #96

Open oxcl opened 6 months ago

oxcl commented 6 months ago

after this commit in vterm the default colors for forground and background are the same. reverting back to a previous commit of vterm fixes this issue since the commit introduced a breaking change i figured it might be a modus-themes problem image image

my pallete overrides in modus-themes-common-overrides:

 (bg-term-black "red")
 (bg-term-red red)
 (bg-term-green green)
 (bg-term-yellow yellow)
 (bg-term-blue blue)
 (bg-term-magenta magenta)
 (bg-term-cyan cyan)
 (bg-term-white "white")
 (bg-term-black-bright "green")
 (bg-term-red-bright red)
 (bg-term-green-bright green)
 (bg-term-yellow-bright yellow-warmer)
 (bg-term-blue-bright blue)
 (bg-term-magenta-bright magenta)
 (bg-term-cyan-bright cyan)
 (bg-term-white-bright "pink")

i have set bg-term-white-bright and bg-term-black-bright to pink and green for debugging.

i also have the same colors for the fg-term-* values:

    (fg-term-red red) ; color 1
    (fg-term-green green) ; color 2
    (fg-term-yellow yellow) ; color 3
    (fg-term-blue blue) ; color 4
    (fg-term-magenta magenta) ; color 5
    (fg-term-cyan cyan) ; color 6
    (fg-term-white "white") ; color 7
    (fg-term-black-bright "green") ; color 8
    (fg-term-red-bright red) ; color 9
    (fg-term-green-bright green) ; color 10
    (fg-term-yellow-bright yellow-warmer) ; color 11
    (fg-term-blue-bright blue) ; color 12
    (fg-term-magenta-bright magenta) ; color 13
    (fg-term-cyan-bright cyan) ; color 14
    (fg-term-white-bright "pink"))) ; color 15

image

protesilaos commented 5 months ago

I am not sure if this is on our end. Maybe @iostapyshyn who sent the pull request to vterm knows what is happening.

iostapyshyn commented 5 months ago

@oxcl

These particular colors should be coming from the default face (see vterm--get-color). Can you show what the following expressions return on your setup (with the latest vterm)?

  1. (vterm--get-color -1)
  2. (vterm--get-color -1 :foreground)
iostapyshyn commented 5 months ago

@oxcl I’m pretty sure I found the issue: vterm doesn't recompile its module automatically after checking out a newer version. The new lisp code in https://github.com/akermu/emacs-libvterm/commit/e96c53f5035c841b20937b65142498bd8e161a40 is not compatible with old vterm C module, resulting in the exact behaviour you’re experiencing.

Can you try recompiling the vterm module? Just do M-x vterm-module-compile RET and restart Emacs.

oxcl commented 5 months ago

@iostapyshyn I'm using nixos so I rely on emacs-libvterm package in nixpkgs to get vterm to work currectly since vterm-module-compile doesn't work in nixos. which I think is the issue here since the nix package has not been updated since April 2023 and I'm not experienced enough with nix to fix it

iostapyshyn commented 5 months ago

Is it possible that you're mixing emacs-libvterm from current melpa with the one provided by nixpkgs? You should definitely keep it consistent and stick to single package. If you're checking out vterm from git (which you seem to, since you were able to identify the violating commit) and using Nix flakes you can just

$ nix develop nixpkgs#emacsPackages.vterm
$ cd /path/to/emacs-libvterm
$ cmake .
$ make

I use NixOS too and that's the way I build emacs-libvterm module.

Otherwise, if you want to have latest features through nixpkgs, I see that the emacsPackages.vterm version in NixOS is coming from https://github.com/NixOS/nixpkgs/blob/nixos-23.11/pkgs/applications/editors/emacs/elisp-packages/recipes-archive-melpa.json (line 123258). Looks like someone regenerates the file from time to time. The comment here looks like the instruction to do so. I bet they will accept a PR with updated melpa, with the last regeneration being 3 months ago it is long overdue