GNU bug report logs - #53255
highlight-regexp should show faces with their properties applied when selecting a face

Previous Next

Package: emacs;

Reported by: ndame <laszlomail <at> protonmail.com>

Date: Fri, 14 Jan 2022 13:38:01 UTC

Severity: normal

Tags: moreinfo

Fixed in version 29.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 53255 in the body.
You can then email your comments to 53255 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Fri, 14 Jan 2022 13:38:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to ndame <laszlomail <at> protonmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 14 Jan 2022 13:38:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: ndame <laszlomail <at> protonmail.com>
To: "Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
Subject: highlight-regexp should show faces with their properties applied when
 selecting a face
Date: Fri, 14 Jan 2022 13:37:13 +0000
[Message part 1 (text/plain, inline)]
highlight-regexp asks for a face to highlight the matches. If it's a complicated file structure when one wants to apply multiple faces to demote, emphasise, etc. parts for readability, then the face selection could be much easier if beside listing the names they also had their properties (color,etc.) applied to them, so the user could see instantly which faces are suitable to apply to various parts of the file.

Like read-color shows the color names with their actual colors, the selection of a face should also show the names with their properties applied.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 08:48:01 GMT) Full text and rfc822 format available.

Message #8 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: ndame <laszlomail <at> protonmail.com>
Cc: 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 15 Jan 2022 09:46:54 +0100
[Message part 1 (text/plain, inline)]
ndame <laszlomail <at> protonmail.com> writes:

> Like read-color shows the color names with their actual colors, the
> selection of a face should also show the names with their properties
> applied.

The patch below implements this, but it does lead to some strange
effects:

[Message part 2 (image/png, inline)]
[Message part 3 (text/plain, inline)]
That is, some faces have odd sizes, so the completion display may look a
bit odd.

But it does accurately tell you what the result is doing to be, so
perhaps it's OK?  Anybody have any opinions?

diff --git a/lisp/hi-lock.el b/lisp/hi-lock.el
index b70d4a7569..18e5f73369 100644
--- a/lisp/hi-lock.el
+++ b/lisp/hi-lock.el
@@ -727,11 +727,16 @@ hi-lock-read-face-name
 			   (cdr (member last-used-face hi-lock-face-defaults))
 			   hi-lock-face-defaults))
 	 face)
-          (if (and hi-lock-auto-select-face (not current-prefix-arg))
+    (if (and hi-lock-auto-select-face (not current-prefix-arg))
 	(setq face (or (pop hi-lock--unused-faces) (car defaults)))
-      (setq face (completing-read
-		  (format-prompt "Highlight using face" (car defaults))
-		  obarray 'facep t nil 'face-name-history defaults))
+      (let ((completion-extra-properties
+             (list :annotation-function
+                   (lambda (s)
+                     (add-face-text-property
+                      0 (length s) (intern s) t s)))))
+        (setq face (completing-read
+		    (format-prompt "Highlight using face" (car defaults))
+		    obarray 'facep t nil 'face-name-history defaults)))
       ;; Update list of un-used faces.
       (setq hi-lock--unused-faces (remove face hi-lock--unused-faces))
       ;; Grow the list of defaults.


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Added tag(s) moreinfo. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Sat, 15 Jan 2022 08:48:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 10:06:01 GMT) Full text and rfc822 format available.

Message #13 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 15 Jan 2022 11:05:20 +0100
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> The patch below implements this, but it does lead to some strange
> effects:
>
> That is, some faces have odd sizes, so the completion display may look a
> bit odd.

In addition to the odd sizes, some face names become completely
unreadable (e.g. the ansi-color-* ones).

> But it does accurately tell you what the result is doing to be, so
> perhaps it's OK?  Anybody have any opinions?

Maybe using an affixation function (like read-char-by-name) would make
things less jarring?  E.g. fontifying the same string for every face
(say "x" or "example") and using that as suffix (rather than prefix, to
keep the face names aligned)?

At any rate, I do think that showing the actual sizes (and colors,
however unreadable) is a good feature to keep.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 10:12:01 GMT) Full text and rfc822 format available.

Message #16 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 15 Jan 2022 11:11:33 +0100
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> In addition to the odd sizes, some face names become completely
> unreadable (e.g. the ansi-color-* ones).

Yes.  But perhaps that's OK?  You don't want to choose those faces here
anyway.

>> But it does accurately tell you what the result is doing to be, so
>> perhaps it's OK?  Anybody have any opinions?
>
> Maybe using an affixation function (like read-char-by-name) would make
> things less jarring?  E.g. fontifying the same string for every face
> (say "x" or "example") and using that as suffix (rather than prefix, to
> keep the face names aligned)?

Hm, yes, that sounds like a good idea.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 11:37:01 GMT) Full text and rfc822 format available.

Message #19 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 15 Jan 2022 12:36:46 +0100
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>> Maybe using an affixation function (like read-char-by-name) would make
>> things less jarring?  E.g. fontifying the same string for every face
>> (say "x" or "example") and using that as suffix (rather than prefix, to
>> keep the face names aligned)?
>
> Hm, yes, that sounds like a good idea.

Serving suggestion attached, for discussion purposes; there might be
smarter things to do wrt alignment (using :align-to specs?  let-binding
tab-width?).

FWIW it looks pretty good with icomplete-vertical-mode, IMO.

[hi-lock.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 19:17:01 GMT) Full text and rfc822 format available.

Message #22 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 15 Jan 2022 21:12:53 +0200
>> Like read-color shows the color names with their actual colors, the
>> selection of a face should also show the names with their properties
>> applied.
>
> The patch below implements this, but it does lead to some strange
> effects:
>
> That is, some faces have odd sizes, so the completion display may look a
> bit odd.
>
> But it does accurately tell you what the result is doing to be, so
> perhaps it's OK?  Anybody have any opinions?

Whereas highlight-regexp can use all faces, the preferable faces are in
hi-lock-face-defaults, so they need to be displayed separately.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 19:17:02 GMT) Full text and rfc822 format available.

Message #25 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 15 Jan 2022 21:14:37 +0200
>>> Maybe using an affixation function (like read-char-by-name) would make
>>> things less jarring?  E.g. fontifying the same string for every face
>>> (say "x" or "example") and using that as suffix (rather than prefix, to
>>> keep the face names aligned)?
>> Hm, yes, that sounds like a good idea.
>
> Serving suggestion attached, for discussion purposes; there might be
> smarter things to do wrt alignment (using :align-to specs?  let-binding
> tab-width?).
>
> FWIW it looks pretty good with icomplete-vertical-mode, IMO.

So this is like `M-x list-faces-display'?

But OTOH, indeed maybe better to use completion-tab-width
like read-char-by-name does?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 21:47:01 GMT) Full text and rfc822 format available.

Message #28 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, ndame <laszlomail <at> protonmail.com>
Cc: "53255 <at> debbugs.gnu.org" <53255 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#53255: highlight-regexp should show faces with
 their properties applied when selecting a face
Date: Sat, 15 Jan 2022 21:46:37 +0000
> > Like read-color shows the color names with their actual colors, the
> > selection of a face should also show the names with their properties
> > applied.
> 
> The patch below implements this, but it does lead to some strange
> effects:

Icicles has done this since 2007.  See here:

https://www.emacswiki.org/emacs/Icicles_-_Candidates_with_Text_Properties

(FWIW, I see this Note on that page:
   "2007-01-24 - RMS has agreed to fix Emacs in this
    way, so this will become possible in Emacs."
 AFAIK, that never happened.)
___

Whether face, color, etc. completion candidates are
shown as, or accompanied by, such propertized samples is
controlled by option `icicle-WYSIWYG-Completions-flag':

---8<-------

icicle-WYSIWYG-Completions-flag is a variable defined in `icicles-opt.el'.

Its value is "MMMM"

Documentation:
Non-nil means show candidates in `*Completions*' using WYSIWYG.
This has an effect only for completion of things like faces, fonts,
and colors.

For face and color candidates, the particular non-nil value determines
the appearance:

* If t, the candidate displays its meaning: WYSIWYG.
* If a string, the string is propertized and then appended to the
  candidate,  to serve as a color swatch.

Some commands might override a string value with different text.  This
is the case for `icicle-read-color-WYSIWYG', for instance: the color
swatch text is always the color's RGB code.

Note that, starting with Emacs 22, if this option is non-nil, then
command `describe-face' does not use `completing-read-multiple', since
that (non-Icicles) function does not support WYSIWYG candidates.

---8<-------




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 22:33:02 GMT) Full text and rfc822 format available.

Message #31 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>, Lars
 Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>,
 "53255 <at> debbugs.gnu.org" <53255 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#53255: highlight-regexp should show faces with
 their properties applied when selecting a face
Date: Sat, 15 Jan 2022 22:31:53 +0000
> > The patch below implements this, but it does lead to some strange
> > effects:
> >
> > That is, some faces have odd sizes, so the completion display may look a
> > bit odd.
> 
> In addition to the odd sizes, some face names become completely
> unreadable (e.g. the ansi-color-* ones).
>
> > But it does accurately tell you what the result is doing to be, so
> > perhaps it's OK?  Anybody have any opinions?
> 
> Maybe using an affixation function (like read-char-by-name) would make
> things less jarring?  E.g. fontifying the same string for every face
> (say "x" or "example") and using that as suffix (rather than prefix, to
> keep the face names aligned)?
> 
> At any rate, I do think that showing the actual sizes (and colors,
> however unreadable) is a good feature to keep.

Yes.

15 years of practice (with Icicles) says this
enhancement request can be made to work well.

The key is to give users (and their code) the
control.  It's perfectly possible to have the
face etc. name itself be shown without any face
property, but to be accompanied by a swatch
that shows the appearance.

I assume that's what you meant by referring to
`read-char-by-name'.

See the screen shots I linked to in my previous
message.  Give users 3 ways to show candidates
that determine colors, fonts, and faces:

 * No sample - just the plain candidate name
 * Candidate name shown as sample
 * Candidate name shown next to sample (swatch)





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 22:42:02 GMT) Full text and rfc822 format available.

Message #34 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Kévin Le Gouguec
 <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>,
 "53255 <at> debbugs.gnu.org" <53255 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#53255: highlight-regexp should show faces with
 their properties applied when selecting a face
Date: Sat, 15 Jan 2022 22:40:58 +0000
> > In addition to the odd sizes, some face names become completely
> > unreadable (e.g. the ansi-color-* ones).
> 
> Yes.  But perhaps that's OK?  You don't want to choose
> those faces here anyway.

Bad assumptions.

1. People use completion to do more than just make a
   choice.

2. The background of the *Completions* window isn't
   necessarily anything like the place where face
   you do choose is to be used.

> >> But it does accurately tell you what the result is doing to be, so
> >> perhaps it's OK?  Anybody have any opinions?
> >
> > Maybe using an affixation function (like read-char-by-name) would make
> > things less jarring?  E.g. fontifying the same string for every face
> > (say "x" or "example") and using that as suffix (rather than prefix, to
> > keep the face names aligned)?
> 
> Hm, yes, that sounds like a good idea.

It's been helping users since 2007 - as has giving
them the possibility to choose whether to use it or
not.  User control is important to such behavior.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 15 Jan 2022 22:49:02 GMT) Full text and rfc822 format available.

Message #37 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>, Lars
 Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>,
 "53255 <at> debbugs.gnu.org" <53255 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#53255: highlight-regexp should show faces with
 their properties applied when selecting a face
Date: Sat, 15 Jan 2022 22:48:25 +0000
> Serving suggestion attached, for discussion purposes; there might be
> smarter things to do wrt alignment (using :align-to specs?  let-binding
> tab-width?).

It shouldn't be limited to hi-lock.  It should be
for `read-face-name'.

And users should be able to control whether and in
what way it happens.

(And the same enhancement should be made for reading
colors and fonts.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sun, 16 Jan 2022 18:23:02 GMT) Full text and rfc822 format available.

Message #40 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sun, 16 Jan 2022 20:20:19 +0200
>>> Like read-color shows the color names with their actual colors, the
>>> selection of a face should also show the names with their properties
>>> applied.
>>
>> The patch below implements this, but it does lead to some strange
>> effects:
>>
>> That is, some faces have odd sizes, so the completion display may look a
>> bit odd.
>>
>> But it does accurately tell you what the result is doing to be, so
>> perhaps it's OK?  Anybody have any opinions?
>
> Whereas highlight-regexp can use all faces, the preferable faces are in
> hi-lock-face-defaults, so they need to be displayed separately.

Also it would be nice to highlight faces displayed in the minibuffer
after fetching them with M-n M-n M-n ...




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Thu, 20 Jan 2022 13:24:02 GMT) Full text and rfc822 format available.

Message #43 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Thu, 20 Jan 2022 14:23:50 +0100
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Serving suggestion attached, for discussion purposes; there might be
> smarter things to do wrt alignment (using :align-to specs?  let-binding
> tab-width?).

Thanks; I've now pushed a variation on this to Emacs 29...

Juri Linkov <juri <at> linkov.net> writes:

> So this is like `M-x list-faces-display'?

Right; I knew we had something similar to this somewhere.

> But OTOH, indeed maybe better to use completion-tab-width
> like read-char-by-name does?

Yes.

Drew Adams <drew.adams <at> oracle.com> writes:

> It shouldn't be limited to hi-lock.  It should be
> for `read-face-name'.

So I put the affixion thing into read-file-name and made hi-lock call
read-file-name instead of doing the completion itself.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug marked as fixed in version 29.1, send any further explanations to 53255 <at> debbugs.gnu.org and ndame <laszlomail <at> protonmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Thu, 20 Jan 2022 13:25:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Thu, 20 Jan 2022 22:59:01 GMT) Full text and rfc822 format available.

Message #48 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Thu, 20 Jan 2022 23:58:43 +0100
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> So I put the affixion thing into read-file-name and made hi-lock call
> read-file-name instead of doing the completion itself.

Great!  A couple of nits:

1. Is there a reason why you went with (list "SAMPLE\t" "" face) instead
   of (list face "" \tSAMPLE)?  "(elisp) Completion Variables" says that
   the completion goes first, then the prefix, then the suffix; I don't
   know how hard a rule that is, however I do think the latter option
   fares better when faced with variable heights.

   Cf. patch and screenshots: I like the second option better because
   IIUC the face names (which is what I'll be focusing my eyes on in
   order to know what to type; the face decoration I can see well enough
   in my peripheral vision) stand a better chance of being aligned.

   (Although I don't know if it's the hour getting late or something,
   but now that I actually try out C-u M-s h r I just… can't get any
   highlighting, with or without my patch on top.  The completion works
   fine, but when I exit the minibuffer… nothing gets highlighted.

   Ugh.  Will sleep on it)

2. As you can see on the screenshots, the prompt shows the default face
   twice.  I think fixing this is as simple as removing format-prompt
   (patch also attached), but then again, I'm not thinking straight this
   evening it seems.

Thanks for your time.

[affixation.patch (text/x-patch, attachment)]
[current.png (image/png, attachment)]
[suggested.png (image/png, attachment)]
[defaults.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Fri, 21 Jan 2022 06:33:02 GMT) Full text and rfc822 format available.

Message #51 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Fri, 21 Jan 2022 07:32:19 +0100
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

>    (Although I don't know if it's the hour getting late or something,
>    but now that I actually try out C-u M-s h r I just… can't get any
>    highlighting, with or without my patch on top.  The completion works
>    fine, but when I exit the minibuffer… nothing gets highlighted.
>
>    Ugh.  Will sleep on it)

OK, turns out it really was well past bedtime for me; C-u sets the
regexp *group*, I don't know why, I was certain it was a toggle to
choose the face.

So please disregard this parenthesized tangent 🙃 and let the patches
and screenshots speak for themselves.  Sorry for the noise.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Fri, 21 Jan 2022 09:32:02 GMT) Full text and rfc822 format available.

Message #54 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Fri, 21 Jan 2022 10:30:50 +0100
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Great!  A couple of nits:
>
> 1. Is there a reason why you went with (list "SAMPLE\t" "" face) instead
>    of (list face "" \tSAMPLE)?  "(elisp) Completion Variables" says that
>    the completion goes first, then the prefix, then the suffix; I don't
>    know how hard a rule that is, however I do think the latter option
>    fares better when faced with variable heights.

It looked really bad with the face names first, since they have wildly
differing lengths.

> 2. As you can see on the screenshots, the prompt shows the default face
>    twice.  I think fixing this is as simple as removing format-prompt
>    (patch also attached), but then again, I'm not thinking straight this
>    evening it seems.

(hi-lock-read-face-name) doesn't seem to display the default twice in an
emacs -Q?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Fri, 21 Jan 2022 13:40:02 GMT) Full text and rfc822 format available.

Message #57 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Fri, 21 Jan 2022 14:39:34 +0100
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
>> Great!  A couple of nits:
>>
>> 1. Is there a reason why you went with (list "SAMPLE\t" "" face) instead
>>    of (list face "" \tSAMPLE)?  "(elisp) Completion Variables" says that
>>    the completion goes first, then the prefix, then the suffix; I don't
>>    know how hard a rule that is, however I do think the latter option
>>    fares better when faced with variable heights.
>
> It looked really bad with the face names first, since they have wildly
> differing lengths.

… and so the samples would get misaligned?  (Trying to make sure I
understand the issue)

IMO that's less annoying than the names being misaligned (since that
throws me off slightly when I'm looking them over to figure out what I
need to type), but YMMV.

(I'm still intrigued by the documentation of :affixation-function
though: it seems to me that it asks for (list COMPLETION PREFIX SUFFIX),
but obviously your code works…)

>> 2. As you can see on the screenshots, the prompt shows the default face
>>    twice.  I think fixing this is as simple as removing format-prompt
>>    (patch also attached), but then again, I'm not thinking straight this
>>    evening it seems.
>
> (hi-lock-read-face-name) doesn't seem to display the default twice in an
> emacs -Q?

It does here 😕

./src/emacs -Q
M-: emacs-repository-version
"1228ec3e1d7657c9eb50184719410f37ed0eb750"
M-: (hi-lock-read-face-name)
Highlight using face (default hi-yellow) (default hi-yellow):




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Fri, 21 Jan 2022 17:07:01 GMT) Full text and rfc822 format available.

Message #60 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Kévin Le Gouguec
 <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>,
 "53255 <at> debbugs.gnu.org" <53255 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#53255: highlight-regexp should show faces with
 their properties applied when selecting a face
Date: Fri, 21 Jan 2022 17:06:19 +0000
> It looked really bad with the face names first, since they have wildly
> differing lengths.

I disagree that it looks really bad.

https://www.emacswiki.org/emacs/Icicles_-_Candidates_with_Text_Properties#FaceCandidatesWithSwatches

And it's definitely better for usability,
including scanning names when candidates
are sorted alphabetically. 

But if you think it looks really bad then
file a separate bug report (enhancement
request) to be able to align parts of
completion candidates.  Surely this isn't
limited to face names and color names.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 22 Jan 2022 11:37:02 GMT) Full text and rfc822 format available.

Message #63 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 22 Jan 2022 12:36:41 +0100
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> … and so the samples would get misaligned?  (Trying to make sure I
> understand the issue)

Yes.

> IMO that's less annoying than the names being misaligned (since that
> throws me off slightly when I'm looking them over to figure out what I
> need to type), but YMMV.

It's what we do in `C-x 8 RET TAB', for much the same rasons.

>> (hi-lock-read-face-name) doesn't seem to display the default twice in an
>> emacs -Q?
>
> It does here 😕
>
> ./src/emacs -Q
> M-: emacs-repository-version
> "1228ec3e1d7657c9eb50184719410f37ed0eb750"
> M-: (hi-lock-read-face-name)
> Highlight using face (default hi-yellow) (default hi-yellow):

Oh, in the prompt -- I thought you meant in the TAB completions.  I must
be going blind.  Fixed now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sat, 22 Jan 2022 18:58:01 GMT) Full text and rfc822 format available.

Message #66 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sat, 22 Jan 2022 20:51:54 +0200
[Message part 1 (text/plain, inline)]
>> It shouldn't be limited to hi-lock.  It should be
>> for `read-face-name'.
>
> So I put the affixion thing into read-file-name and made hi-lock call
> read-file-name instead of doing the completion itself.

This has such regression that before it supported the following workflow:
in the minibuffer 'M-n M-n ...' used to pull hi-lock faces from the ordered
"future history" list.  But read-face-name doesn't support this feature.

If highlight-regexp should use read-face-name, then this feature
could be added to read-face-name.  Here is the simplest patch,
but it would be a tough task to explain the need of an additional
DEFAULTS arg in the documentation.  An alternative would be to
rewrite read-face-name and keep cdr of the existing arg DEFAULT
for the future history of the minibuffer.
[read-face-name-defaults.patch (text/x-diff, inline)]
diff --git a/lisp/faces.el b/lisp/faces.el
index bb9b1e979f..7a1fae3669 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1065,7 +1065,7 @@ invert-face
 
 (defvar crm-separator) ; from crm.el
 
-(defun read-face-name (prompt &optional default multiple)
+(defun read-face-name (prompt &optional default multiple defaults)
   "Read one or more face names, prompting with PROMPT.
 PROMPT should not end in a space or a colon.
 
@@ -1129,7 +1129,7 @@ read-face-name
           (dolist (face (completing-read-multiple
                          prompt
                          (completion-table-in-turn nonaliasfaces aliasfaces)
-                         nil t nil 'face-name-history default))
+                         nil t nil 'face-name-history defaults))
             ;; Ignore elements that are not faces
             ;; (for example, because DEFAULT was "all faces")
             (if (facep face) (push (intern face) faces)))
diff --git a/lisp/hi-lock.el b/lisp/hi-lock.el
index 8df73b1d37..a6760580f8 100644
--- a/lisp/hi-lock.el
+++ b/lisp/hi-lock.el
@@ -729,7 +729,8 @@ hi-lock-read-face-name
 	 face)
     (if (and hi-lock-auto-select-face (not current-prefix-arg))
 	(setq face (or (pop hi-lock--unused-faces) (car defaults)))
-      (setq face (symbol-name (read-face-name "Highlight using face" defaults)))
+      (setq face (symbol-name (read-face-name "Highlight using face"
+                                              (car defaults) t (cdr defaults))))
       ;; Update list of un-used faces.
       (setq hi-lock--unused-faces (remove face hi-lock--unused-faces))
       ;; Grow the list of defaults.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sun, 23 Jan 2022 12:39:01 GMT) Full text and rfc822 format available.

Message #69 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sun, 23 Jan 2022 13:38:36 +0100
Juri Linkov <juri <at> linkov.net> writes:

> This has such regression that before it supported the following workflow:
> in the minibuffer 'M-n M-n ...' used to pull hi-lock faces from the ordered
> "future history" list.  But read-face-name doesn't support this feature.
>
> If highlight-regexp should use read-face-name, then this feature
> could be added to read-face-name.  Here is the simplest patch,
> but it would be a tough task to explain the need of an additional
> DEFAULTS arg in the documentation.  An alternative would be to
> rewrite read-face-name and keep cdr of the existing arg DEFAULT
> for the future history of the minibuffer.

Hm, I assumed that that was what read-face-name did (since it allows
DEFAULT to be a list).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Sun, 23 Jan 2022 18:47:02 GMT) Full text and rfc822 format available.

Message #72 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Sun, 23 Jan 2022 20:45:07 +0200
[Message part 1 (text/plain, inline)]
>> This has such regression that before it supported the following workflow:
>> in the minibuffer 'M-n M-n ...' used to pull hi-lock faces from the ordered
>> "future history" list.  But read-face-name doesn't support this feature.
>>
>> If highlight-regexp should use read-face-name, then this feature
>> could be added to read-face-name.  Here is the simplest patch,
>> but it would be a tough task to explain the need of an additional
>> DEFAULTS arg in the documentation.  An alternative would be to
>> rewrite read-face-name and keep cdr of the existing arg DEFAULT
>> for the future history of the minibuffer.
>
> Hm, I assumed that that was what read-face-name did (since it allows
> DEFAULT to be a list).

Then read-face-name should be improved to support a list of defaults:

[read-face-name-defaults.patch (text/x-diff, inline)]
diff --git a/lisp/faces.el b/lisp/faces.el
index bb9b1e979f..c03a0ff8c1 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1081,6 +1081,7 @@ read-face-name
 element of DEFAULT is returned.  If DEFAULT isn't a list, but
 MULTIPLE is non-nil, a one-element list containing DEFAULT is
 returned.  Otherwise, DEFAULT is returned verbatim."
+  (let (defaults)
     (unless (listp default)
       (setq default (list default)))
     (when default
@@ -1090,7 +1091,7 @@ read-face-name
                            default ", ")
               ;; If we only want one, and the default is more than one,
               ;; discard the unwanted ones.
-            (setq default (car default))
+              (setq defaults default default (car default))
               (if (symbolp default)
                   (symbol-name default)
                 default))))
@@ -1137,8 +1138,8 @@ read-face-name
         (let ((face (completing-read
                      prompt
                      (completion-table-in-turn nonaliasfaces aliasfaces)
-                   nil t nil 'face-name-history default)))
-        (if (facep face) (intern face))))))
+                     nil t nil 'face-name-history defaults)))
+          (if (facep face) (intern face)))))))
 
 ;; Not defined without X, but behind window-system test.
 (defvar x-bitmap-file-path)

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#53255; Package emacs. (Mon, 24 Jan 2022 09:23:01 GMT) Full text and rfc822 format available.

Message #75 received at 53255 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: ndame <laszlomail <at> protonmail.com>, 53255 <at> debbugs.gnu.org,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Subject: Re: bug#53255: highlight-regexp should show faces with their
 properties applied when selecting a face
Date: Mon, 24 Jan 2022 10:22:24 +0100
Juri Linkov <juri <at> linkov.net> writes:

> Then read-face-name should be improved to support a list of defaults:

Makes sense to me; go ahead and push.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 21 Feb 2022 12:24:10 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 57 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.