[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC] Add :invisible face attribute
From: |
Michal Nazarewicz |
Subject: |
Re: [RFC] Add :invisible face attribute |
Date: |
Wed, 18 Dec 2024 19:05:45 +0100 |
On Wed, Dec 18 2024, Eli Zaretskii wrote:
> A new face attribute adds quite a bit of complexity, so we should
> really believe this is a good idea. Given that this can already be
> achieved without a new attribute, I'm not sure. Is this really a
> frequent need/situation?
Yes, agreed. org-hide is the only face I’m aware of and this only came
up as I was working on auto-dim-other-buffers.
To quickly recap, auto-dim-other-buffers adds a face remap to default
face which changes its background. The problem is that if default
background is changed, text rendered with org-hide becomes visible. So
now I also need to remap org-hide.
This by itself I could live with, but it creates friction for users who
need to not only customise auto-dim-other-buffers-face (which specifies
the remap background) but also auto-dim-other-buffers-hide-face.
There are actually two minor bugs in Org mode that this patch fixes.
First is when default face is changed after org-mode is initialised:
(setq org-hide-leading-stars t)
(org-mode)
(insert "\n*** Foo\n") ; the first two stars are invisible
(set-face-background 'default "red") ; the starts become visible
Second is when running with frames on multiple displays, e.g. starting
org-mode on terminal frame with black background makes org-hide use
black foreground. But that’s visible on X11 frame which uses white
background.
This is why I’ve not gone with Org’s way of initialising the hide face
when the mode is first enabled.
>> @@ -2911,6 +2925,13 @@ merge_face_ref (struct window *w,
>> else
>> err = true;
>> }
>> + else if (EQ (keyword, QCinvisible))
>> + {
>> + if (EQ (value, Qt) || NILP (value))
>> + to[LFACE_INVISIBLE_INDEX] = value;
>> + else
>> + err = true;
>> + }
> Does this mean that the result of merging two faces with different
> values for :invisible will depend on the order of the merge? That is,
> does the nil value override the t value? Or am I missing something?
The behaviour is the same as for other boolean attributes, e.g. extend
or inverse-video, so yes, if the attribute is specified multiple times
one overrides the other.
There’s a related difference in how org-hide works now vs with this
patch. Currently, selecting text which uses org-hide makes the text
visible; with this patch it remains invisible unless region face is
changed to include :invisible nil.
> Btw, I think "invisible" is not the best name for this, because that's
> not really what will happen on display. It's more like "illegible" or
> something to that effect.
Not sure I’m a fan of illegible but I’m happy to go with whatever name.
By the way, two other ideas I had were:
a) add another value for :inverse-video (e.g. ‘:inverse-video
'foreground’ makes foreground same as background) or
b) add a pseudo colour so one could write ‘:foreground "background"’.
--
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»