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

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

[ndame via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 870 bytes --]

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

[Lars Ingebrigtsen]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

ndame <laszlomail@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:


[-- Attachment #2: Type: image/png, Size: 95517 bytes --]

[-- Attachment #3: Type: text/plain, Size: 1444 bytes --]


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

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

[Kévin Le Gouguec]

Lars Ingebrigtsen <larsi@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.

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

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@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

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

[Kévin Le Gouguec]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

Lars Ingebrigtsen <larsi@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: hi-lock.patch --]
[-- Type: text/x-patch, Size: 1005 bytes --]

diff --git a/lisp/hi-lock.el b/lisp/hi-lock.el
index b70d4a7569..05584b8dc3 100644
--- a/lisp/hi-lock.el
+++ b/lisp/hi-lock.el
@@ -712,6 +712,13 @@ hi-lock-regexp-okay
     (error "Regexp cannot match an empty string"))
    (t regexp)))
 
+(defun hi-lock-decorate-face-names (faces)
+  (mapcar
+   (lambda (face)
+     (list face "" (concat "\t"
+                    (propertize "EXAMPLE" 'face (intern-soft face)))))
+   faces))
+
 (defun hi-lock-read-face-name ()
   "Return face for interactive highlighting.
 When `hi-lock-auto-select-face' is non-nil, just return the next face.
@@ -726,6 +733,8 @@ hi-lock-read-face-name
 	 (defaults (append hi-lock--unused-faces
 			   (cdr (member last-used-face hi-lock-face-defaults))
 			   hi-lock-face-defaults))
+         (completion-extra-properties
+          '(:affixation-function hi-lock-decorate-face-names))
 	 face)
           (if (and hi-lock-auto-select-face (not current-prefix-arg))
 	(setq face (or (pop hi-lock--unused-faces) (car defaults)))

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

[Juri Linkov]

>> 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.

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

[Juri Linkov]

>>> 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?

bug#53255: [External] : bug#53255: highlight-regexp should show faces with their properties applied when selecting a face

[Drew Adams]

> > 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<-------

bug#53255: [External] : bug#53255: highlight-regexp should show faces with their properties applied when selecting a face

[Drew Adams]

> > 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)

bug#53255: [External] : bug#53255: highlight-regexp should show faces with their properties applied when selecting a face

[Drew Adams]

> > 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.

bug#53255: [External] : bug#53255: highlight-regexp should show faces with their properties applied when selecting a face

[Drew Adams]

> 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.)

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

[Juri Linkov]

>>> 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 ...

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

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@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@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@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#53255: highlight-regexp should show faces with their properties applied when selecting a face

[Kévin Le Gouguec]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Lars Ingebrigtsen <larsi@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: affixation.patch --]
[-- Type: text/x-patch, Size: 632 bytes --]

diff --git a/lisp/faces.el b/lisp/faces.el
index bb9b1e979f..cd6145cf68 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1112,10 +1112,10 @@ read-face-name
            (lambda (faces)
              (mapcar
               (lambda (face)
-                (list (concat (propertize "SAMPLE" 'face face)
-                              "\t")
+                (list face
                       ""
-                      face))
+                      (concat "\t"
+                              (propertize "SAMPLE" 'face face))))
               faces))))
         aliasfaces nonaliasfaces faces)
     ;; Build up the completion tables.

[-- Attachment #3: current.png --]
[-- Type: image/png, Size: 99434 bytes --]

[-- Attachment #4: suggested.png --]
[-- Type: image/png, Size: 99488 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: defaults.patch --]
[-- Type: text/x-patch, Size: 688 bytes --]

diff --git a/lisp/hi-lock.el b/lisp/hi-lock.el
index 53e6f779b3..c748bdebfb 100644
--- a/lisp/hi-lock.el
+++ b/lisp/hi-lock.el
@@ -730,9 +730,7 @@ hi-lock-read-face-name
     (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
-                   (format-prompt "Highlight using face" (car defaults))
-                   defaults)))
+                  (read-face-name "Highlight using face" defaults)))
       ;; Update list of un-used faces.
       (setq hi-lock--unused-faces (remove face hi-lock--unused-faces))
       ;; Grow the list of defaults.

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

[Kévin Le Gouguec]

Kévin Le Gouguec <kevin.legouguec@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.

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

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@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

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

[Kévin Le Gouguec]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Kévin Le Gouguec <kevin.legouguec@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):

bug#53255: [External] : bug#53255: highlight-regexp should show faces with their properties applied when selecting a face

[Drew Adams]

> 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.

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

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@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

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

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

>> 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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: read-face-name-defaults.patch --]
[-- Type: text/x-diff, Size: 1618 bytes --]

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.

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

[Lars Ingebrigtsen]

Juri Linkov <juri@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

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

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

>> 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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: read-face-name-defaults.patch --]
[-- Type: text/x-diff, Size: 1331 bytes --]

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)

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

[Lars Ingebrigtsen]

Juri Linkov <juri@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