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

Please add styling for vtable #110

Closed shipmints closed 1 month ago

shipmints commented 1 month ago

Thank you for your work and effort to put modus-themes together and maintain them.

Now that vtable is "standard" in Emacs and people are starting to use it, for example, the activities-list function in https://github.com/alphapapa/activities.el, it would be great for modus-themes (and other themes) to have basic vtable styling support.

I use this stanza in my configuration to gain fixed-width vtable display:

(use-package modus-themes
...
  (defun my/modus-themes-after-load-theme-hook (&rest _)
...
    (defface vtable `((t :inherit #'header-line)) "")
...

Thank you.

protesilaos commented 1 month ago

From: shipmints @.***> Date: Thu, 20 Jun 2024 14:11:34 -0700

Thank you for your work and effort to put modus-themes together and maintain them.

You are welcome!

Now that vtable is "standard" in Emacs and people are starting to use it, for example, the activities-list function in https://github.com/alphapapa/activities.el, it would be great for modus-themes (and other themes) to have basic vtable styling support.

Is it only about adding fixed-pitch to it? Or is there more? I have not seen a vtable yet.

-- Protesilaos Stavrou https://protesilaos.com

shipmints commented 1 month ago

vtable is the spiritual successor to tabulated-list-mode (which will never die). The irony is the "v" stands for variable pitch despite that fixed fonts look best to people who use them almost exclusively. The only face it supports is vtable and other styling is applied dynamically on top of it as the programmer provides column and row colors.

So it seems that you are right, just an option to style vtable as fixed is needed. The default is as follows and is very simple, hence my simple override:

(defface vtable
  '((t :inherit variable-pitch))
  "Face used (by default) for vtables."
  :version "29.1"
  :group 'faces)
protesilaos commented 1 month ago

From: shipmints @.***> Date: Fri, 21 Jun 2024 04:59:33 -0700

vtable is the spiritual successor to tabulated-list-mode (which will never die). The irony is the "v" stands for variable pitch despite that fixed fonts look best to people who use them almost exclusively. The only face it supports is vtable and other styling is applied dynamically on top of it as the programmer provides column and row colors.

My understanding with vtable was that it had to be variable-pitch. This is why I never touched it. As you note, this is the meaning of "v" in its name. So is it not wrong to make it all fixed-pitch?

A practical problem I have here is that I do not have some easy-to-reproduce examples to make better comparisons. Of course, you mentioned a package, but using that is not trivial as I need to learn what it does, how to use it, etc. If you have a vtable that I can quickly check, then I am happy to test how it looks with and without variable-pitch.

-- Protesilaos Stavrou https://protesilaos.com

shipmints commented 1 month ago

Here are side-by-sides for vtable native variable pitch and fixed pitch (the fixed font is Inconsolata). Nothing "has to be" variable pitch, of course. It's just a default and column width calculations are designed to accommodate variable pitch. People should be free to configure either way. I use modus-themes in fixed pitch only, not mixed. The theme is your modus-vivendi-deuteranopia with several customizations.

image image
shipmints commented 1 month ago

Here's a small example you can run on a standalone basis.

(require 'vtable)
(defun vtable-example()
  (with-current-buffer (get-buffer-create "*vtable Example*")
    (let ((inhibit-read-only t))
      (read-only-mode)
      (erase-buffer)

      (make-vtable
       :columns '((:name "Column 1" :width 10) (:name "Column 2" :width 10))
       :column-colors '("brown" "darkgray")
       :sort-by '((1 . ascend))
       :objects '(("Pie" 3.141)
                  ("Oiler" 2.718))
       :actions `("q" (lambda (&rest _) (bury-buffer)))
       )

      (pop-to-buffer (current-buffer)))))
protesilaos commented 1 month ago

From: shipmints @.***> Date: Sat, 22 Jun 2024 04:00:28 -0700

Here's a small example you can run on a standalone basis.

[... 20 lines elided]

Thank you! I experimented with it. I do not think we need to change anything in the Modus themes because this is only about the font family.

I tried both the fixed-pitch and variable-pitch and they look equally good to me, as the table is always aligned correctly (after you recreate it).

To change the 'vtable' face, I used this:

(set-face-attribute 'vtable nil :inherit fixed-pitch)

-- Protesilaos Stavrou https://protesilaos.com

shipmints commented 1 month ago

I'd assumed that when using modus-themes in a fixed-pitch mode only, not mixed, that as many relevant faces would be set to fixed, including vtable. Apologies if I misunderstood the scope of theming?

protesilaos commented 1 month ago

From: shipmints @.***> Date: Sat, 22 Jun 2024 07:01:36 -0700

I'd assumed that when using modus-themes in a fixed-pitch mode only, not mixed, that as many relevant faces would be set to fixed, including vtable. Apologies if I misunderstood the scope of theming?

By "fixed-pitch mode" you mean when you do not set the user option 'modus-themes-mixed-fonts'? The idea with that is to make space sensitive elements use fixed-pitch, but otherwise allow everything else to be variable-pitch.

In this scenario, the 'vtable' is not space sensitive because it aligns everything properly regardless of the underlying font family (unless my experiments with it are inconclusive).

Do you think we should have 'vtable' be affected by the "mixed fonts" option we have? I am happy to reconsider.

-- Protesilaos Stavrou https://protesilaos.com

shipmints commented 1 month ago

That was my naive assumption. If the general approach is as you say, I'm happy to make my own adjustments or become accustomed to a more "ransom-note" appearance with mixed fonts.

protesilaos commented 1 month ago

From: shipmints @.***> Date: Sat, 22 Jun 2024 08:46:13 -0700

That was my naive assumption. If the general approach is as you say, I'm happy to make my own adjustments or become accustomed to a more "ransom-note" appearance with mixed fonts.

I just pushed an update that adds support for it. I have made it use the 'default' face by default and switch to 'variable-pitch' if the user option 'modus-themes-mixed-fonts' is non-nil.

Please let me know if this works for you.

-- Protesilaos Stavrou https://protesilaos.com

shipmints commented 1 month ago

Sounds like what modus-theme users would expect. Thank you.

protesilaos commented 1 month ago

From: shipmints @.***> Date: Sat, 22 Jun 2024 11:52:29 -0700

Sounds like what modus-theme users would expect. Thank you.

You are welcome!

-- Protesilaos Stavrou https://protesilaos.com