bug#19877: 24.4; highlight-regexp displays nonsense options for faces

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Boruch Baum]

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

WHAT HAPPENS:
=============

The documentation for `highlight-regexp' ( info:emacs#Highlight
Interactively]] ) says, "While being prompted for a face use ‘M-n’ and
‘M-p’ to cycle through them." However, repeatedly keying ‘M-n’ will
toggle through the entries in `hi-lock-face-defaults' *twice*, and then
will start displaying _other items_, presumably from some other
 history ring, but not necessarily faces. In my case, some of the _other
items_ that appear when I ‘M-n’ cycle, were: apropos-variable-button,
org-target, bookmark-menu-bookmark, org-agenda-date-today, and
ido-incomplete-regexp.


Furthermore, if one of those _other items_ is selected,
`highlight-regexp' exits, without indicating an error, and without
highlighting the requested regexp in *any* face.


Furthermore, `highlight-regexp', as an alias for function
`hi-lock-face-buffer', hard codes face `hi-yellow' as a default, but
that default isn't used when one of those non-face _other items_ are
selected.


Finally, it seems inconsistent with the emacs spirit of flexibility to
have `hi-yellow' be hard-coded as a default in function
`hi-lock-face-buffer'.


Post-finally, courtesy of the indentation police: in function
`hi-lock-read-face-name', the `then` and `else` elements of the
function's only `if` form, are out-dented.


WHAT I EXPECTED:
================
1] There are a few ways I expected `M-n` could toggle - either through
only the entries in `hi-lock-face-defaults'; through the entries in
`hi-lock-face-defaults' plus faces the user had previously entered for
hi-lock purposes; or through the entries in `hi-lock-face-defaults' plus
faces the user had previously entered for any purpose. It should never
*offer* the user an invalid option. Also, it should never accept an
entry that isn't a face.

2] If, somehow, something other than a valid face is entered, the user
should be notified, and sent back to the prompt, with the same option of
`M-n` cycling.

3] If, somehow, something other than a valid face is accepted, the
default should be used.

4] The default should not be hard-coded as `hi-yellow`, or as anything
else. It should be set to user-definable variable, say
`hi-lock-default-face', which could have a default value of `hi-yellow'.

5] Indentation readability / inconsistency quibble.



PARTIAL SOLUTION
================
1] Why `M-n` cycled twice through legitimate defaults before offering up
garbage.

For the following, paste function `hi-lock-read-face-name' into a
scratch buffer in order to follow my line number references


The value of `defaults',  when `hi-lock-interactive-patterns' is *nil*,
because of line 7, hi-lock--unused-faces = hi-lock-face-defaults
because of line 9, last-used-face = nil
so at line 12,
         (defaults (append hi-lock-face-defaults
                           (cdr (member nil hi-lock-face-defaults))
                           hi-lock-face-defaults))
which simplifies to
         (defaults (append hi-lock-face-defaults
                           nil
                           hi-lock-face-defaults))

OTOH, when `hi-lock-interactive-patterns' is not *nil*,
because of line 7, hi-lock--unused-faces = nil
so at line 12,
         (defaults
           (append nil
                   (cdr (member last-used-face hi-lock-face-defaults))
                   hi-lock-face-defaults))


CONSEQUENTLY, line 12 should be
         (defaults
           (if hi-lock-interactive-patterns
             (let ((index (position last-used-face hi-lock-face-defaults)))
               (append (last       hi-lock-face-defaults (1+ index))
                       (butlast    hi-lock-face-defaults (1+ index))))
            hi-lock-face-defaults))


2] on the issue of the strange entries in the history, I've only traced
the lisp code as far as `read-from-minibuffer' in minibuf.c




In GNU Emacs 24.4.1 (x86_64-pc-linux-gnu)
 of 2014-12-09 on gaia, modified by Debian
System Description:     Debian 7.0 GNU/Linux wheezy/testing

Configured using:
 `configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/s\
ite-lisp
 --build x86_64-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
 --libexecdir=/usr/lib --localstatedir=/var/lib
 --infodir=/usr/share/info --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/s\
ite-lisp
 --with-x=no --without-gconf --without-gsettings 'CFLAGS=-g -O2
 -fstack-protector-strong -Wformat -Werror=format-security -Wall'
 CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-z,relro'

Important settings:
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: IBuffer

Minor modes in effect:
  highlight-current-line-minor-mode: t
  desktop-save-mode: t
  savehist-mode: t
  winner-mode: t
  global-hi-lock-mode: t
  hi-lock-mode: t
  delete-selection-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-favicon hides
/usr/share/emacs24/site-lisp/w3m/w3m-favicon
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-symbol hides
/usr/share/emacs24/site-lisp/w3m/w3m-symbol
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/bookmark-w3m hides
/usr/share/emacs24/site-lisp/w3m/bookmark-w3m
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/mime-w3m hides
/usr/share/emacs24/site-lisp/w3m/mime-w3m
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-filter hides
/usr/share/emacs24/site-lisp/w3m/w3m-filter
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-ccl hides
/usr/share/emacs24/site-lisp/w3m/w3m-ccl
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-namazu hides
/usr/share/emacs24/site-lisp/w3m/w3m-namazu
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-ems hides
/usr/share/emacs24/site-lisp/w3m/w3m-ems
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/octet hides
/usr/share/emacs24/site-lisp/w3m/octet
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-bookmark hides
/usr/share/emacs24/site-lisp/w3m/w3m-bookmark
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-perldoc hides
/usr/share/emacs24/site-lisp/w3m/w3m-perldoc
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-dtree hides
/usr/share/emacs24/site-lisp/w3m/w3m-dtree
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-session hides
/usr/share/emacs24/site-lisp/w3m/w3m-session
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-mail hides
/usr/share/emacs24/site-lisp/w3m/w3m-mail
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-search hides
/usr/share/emacs24/site-lisp/w3m/w3m-search
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3mhack hides
/usr/share/emacs24/site-lisp/w3m/w3mhack
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-proc hides
/usr/share/emacs24/site-lisp/w3m/w3m-proc
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-tabmenu hides
/usr/share/emacs24/site-lisp/w3m/w3m-tabmenu
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-rss hides
/usr/share/emacs24/site-lisp/w3m/w3m-rss
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/mew-w3m hides
/usr/share/emacs24/site-lisp/w3m/mew-w3m
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m hides
/usr/share/emacs24/site-lisp/w3m/w3m
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m hides
/usr/share/emacs24/site-lisp/w3m/w3m
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-image hides
/usr/share/emacs24/site-lisp/w3m/w3m-image
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-hist hides
/usr/share/emacs24/site-lisp/w3m/w3m-hist
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-lnum hides
/usr/share/emacs24/site-lisp/w3m/w3m-lnum
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-form hides
/usr/share/emacs24/site-lisp/w3m/w3m-form
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-cookie hides
/usr/share/emacs24/site-lisp/w3m/w3m-cookie
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-weather hides
/usr/share/emacs24/site-lisp/w3m/w3m-weather
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-bug hides
/usr/share/emacs24/site-lisp/w3m/w3m-bug
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-fb hides
/usr/share/emacs24/site-lisp/w3m/w3m-fb
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-antenna hides
/usr/share/emacs24/site-lisp/w3m/w3m-antenna
/home/boruch/.emacs.d/elpa/w3m-20140107.1708/w3m-util hides
/usr/share/emacs24/site-lisp/w3m/w3m-util
/home/boruch/.emacs.d/elpa/csv-mode-1.2/csv-mode hides
/usr/share/emacs24/site-lisp/emacs-goodies-el/csv-mode
/usr/share/emacs/24.4/site-lisp/cdargs hides
/usr/share/emacs/site-lisp/cdargs
/usr/share/emacs/24.4/site-lisp/debian-startup hides
/usr/share/emacs/site-lisp/debian-startup
/usr/share/emacs24/site-lisp/flim/md4 hides /usr/share/emacs/24.4/lisp/md4
/usr/share/emacs24/site-lisp/flim/hex-util hides
/usr/share/emacs/24.4/lisp/hex-util
/usr/share/emacs24/site-lisp/dictionaries-common/flyspell hides
/usr/share/emacs/24.4/lisp/textmodes/flyspell
/usr/share/emacs24/site-lisp/dictionaries-common/ispell hides
/usr/share/emacs/24.4/lisp/textmodes/ispell
/usr/share/emacs/site-lisp/rst hides
/usr/share/emacs/24.4/lisp/textmodes/rst
/usr/share/emacs24/site-lisp/flim/sasl-ntlm hides
/usr/share/emacs/24.4/lisp/net/sasl-ntlm
/usr/share/emacs24/site-lisp/flim/hmac-def hides
/usr/share/emacs/24.4/lisp/net/hmac-def
/usr/share/emacs24/site-lisp/flim/hmac-md5 hides
/usr/share/emacs/24.4/lisp/net/hmac-md5
/usr/share/emacs24/site-lisp/flim/sasl-cram hides
/usr/share/emacs/24.4/lisp/net/sasl-cram
/usr/share/emacs24/site-lisp/flim/ntlm hides
/usr/share/emacs/24.4/lisp/net/ntlm
/usr/share/emacs24/site-lisp/flim/sasl hides
/usr/share/emacs/24.4/lisp/net/sasl
/usr/share/emacs24/site-lisp/flim/sasl-digest hides
/usr/share/emacs/24.4/lisp/net/sasl-digest
/usr/share/emacs/24.4/lisp/emacs-lisp/ert-x hides
/usr/share/emacs/24.4/site-lisp/slime/lib/ert-x
/usr/share/emacs/24.4/lisp/emacs-lisp/cl-lib hides
/usr/share/emacs/24.4/site-lisp/slime/lib/cl-lib
/usr/share/emacs/24.4/lisp/emacs-lisp/ert hides
/usr/share/emacs/24.4/site-lisp/slime/lib/ert

Features:
(shadow sort mail-extr emacsbug message rfc822 mml mml-sec mm-decode
mm-bodies mm-encode mailabbrev ...)

Memory information:
((conses 16 459100 69911)
 (symbols 48 42873 0)
 (miscs 40 2286 3515)
 (strings 32 125530 18155)
 (string-bytes 1 3397524)
 (vectors 16 43381)
 (vector-slots 8 1545011 166125)
 (floats 8 374 644)
 (intervals 56 10688 1329)
 (buffers 960 72)
 (heap 1024 42235 1854))


-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0





[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Glenn Morris]

This report is a literal duplicate of http://debbugs.gnu.org/19876
and will be removed.
Please direct any future mails on this matter to 19876 at debbugs.gnu.org.

bug#19876: bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Boruch Baum]

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

Bad idea, Glenn.

I had mistakenly posted two emails, and it was the second one, which
became bug#19877, that has a proposed patch that solves part of the problem.

Better to close or remove 19876 and keep 19877.



On 02/16/2015 02:11 PM, Glenn Morris wrote:
> 
> This report is a literal duplicate of http://debbugs.gnu.org/19876
> and will be removed.
> Please direct any future mails on this matter to 19876 at debbugs.gnu.org.
> 


-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Glenn Morris]

Boruch Baum wrote:

> I had mistakenly posted two emails, and it was the second one, which
> became bug#19877, that has a proposed patch that solves part of the problem.
>
> Better to close or remove 19876 and keep 19877.

done

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Juri Linkov]

> 2] on the issue of the strange entries in the history, I've only traced
> the lisp code as far as `read-from-minibuffer' in minibuf.c

These additional entries in the history are all faces that you can select
if you want instead of the small set of default faces at the beginning
of the list.

Maybe the list of all faces (that come after the default faces)
should be better sorted somehow, e.g. alphabetically or by use frequency.

PS: Regarding the default faces used twice, I don't know the reason
why it was implemented this way, so it seems your patch will improve it.

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Boruch Baum]

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

On 02/18/2015 01:23 PM, Juri Linkov wrote:
> These additional entries in the history are all faces that you can select
> if you want instead of the small set of default faces at the beginning
> of the list.
Since you said so, I double-checked the items that I thought were not
faces, and you're correct: apropos-variable-button,
org-target, bookmark-menu-bookmark, org-agenda-date-today, and
ido-incomplete-regexp are all faces. (would be nice if there were a
standard labeling convention identifying faces, eg. foo-face).

> Maybe the list of all faces (that come after the default faces)
> should be better sorted somehow, e.g. alphabetically or by use frequency.
My vote is to just restrict the options to those specified in
hi-lock-face-defaults. The value can be easily modified, and its a
simple way to keep unusual and unexpected faces from appearing. Also,
how many hi-lock faces is anyone going to ever need?

>  
> PS: Regarding the default faces used twice, I don't know the reason
> why it was implemented this way, so it seems your patch will improve it.
Glad to be of some help.


-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Juri Linkov]

>> Maybe the list of all faces (that come after the default faces)
>> should be better sorted somehow, e.g. alphabetically or by use frequency.
> My vote is to just restrict the options to those specified in
> hi-lock-face-defaults. The value can be easily modified, and its a
> simple way to keep unusual and unexpected faces from appearing.

Currently it's not too easy to modify since hi-lock-face-defaults
is defined with defvar, but not defcustom.

> Also, how many hi-lock faces is anyone going to ever need?

I'd say that in average 8 faces are needed, but the problem is
that hi-lock-face-defaults provides only 4 usable default faces
("hi-yellow" "hi-pink" "hi-green" "hi-blue").  The rest of
default faces with the prefix "-b" are highlighting with bold weight,
so they are less usable.  This is why often is necessary to find
more faces in the list of all faces that come after default faces.

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Boruch Baum]

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

On 02/19/2015 02:14 PM, Juri Linkov wrote:
>>> Maybe the list of all faces (that come after the default faces)
>>> should be better sorted somehow, e.g. alphabetically or by use frequency.
>> My vote is to just restrict the options to those specified in
>> hi-lock-face-defaults. The value can be easily modified, and its a
>> simple way to keep unusual and unexpected faces from appearing.
> 
> Currently it's not too easy to modify since hi-lock-face-defaults
> is defined with defvar, but not defcustom.
> 
>> Also, how many hi-lock faces is anyone going to ever need?
> 
> I'd say that in average 8 faces are needed, but the problem is
> that hi-lock-face-defaults provides only 4 usable default faces
> ("hi-yellow" "hi-pink" "hi-green" "hi-blue").  The rest of
> default faces with the prefix "-b" are highlighting with bold weight,
> so they are less usable.  This is why often is necessary to find
> more faces in the list of all faces that come after default faces.
> 

What works for me to define faces and add them to hi-lock-face-defaults,
are the following two snippets in my .emacs:

(defface hi-test
  '((((background dark)) (:background "white" :foreground "red"))
    (t (:background "white")))
  "Face for hi-lock mode."
  :group 'hi-lock-faces)


(add-hook 'hi-lock-mode-hook
  (lambda ()
    (setq hi-lock-face-defaults '("hi-green" "hi-blue" "hi-red"
"hi-cyan" "hi-magenta" "hi-yellow" "hi-test"))))



-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> The documentation for `highlight-regexp' ( info:emacs#Highlight
> Interactively]] ) says, "While being prompted for a face use ‘M-n’ and
> ‘M-p’ to cycle through them." However, repeatedly keying ‘M-n’ will
> toggle through the entries in `hi-lock-face-defaults' *twice*,

This behaviour is still present in Emacs 29 (after Juri fixed it so that
it works better again) -- the hi-lock defaults appear twice.

I've now fixed this in Emacs 29.

> and then will start displaying _other items_, presumably from some
> other history ring, but not necessarily faces. In my case, some of the
> _other items_ that appear when I ‘M-n’ cycle, were:
> apropos-variable-button, org-target, bookmark-menu-bookmark,
> org-agenda-date-today, and ido-incomplete-regexp.

(Others answered this bit -- it the other faces.)

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

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Juri Linkov]

>> The documentation for `highlight-regexp' ( info:emacs#Highlight
>> Interactively]] ) says, "While being prompted for a face use ‘M-n’ and
>> ‘M-p’ to cycle through them." However, repeatedly keying ‘M-n’ will
>> toggle through the entries in `hi-lock-face-defaults' *twice*,
>
> This behaviour is still present in Emacs 29 (after Juri fixed it so that
> it works better again) -- the hi-lock defaults appear twice.
>
> I've now fixed this in Emacs 29.

I thought it was a feature. 😯  I don't use these duplicate faces, but
it seems someone intentionally created a copy at the end of the face list.
OTOH, we could wait until someone will protest this change.

bug#19877: 24.4; highlight-regexp displays nonsense options for faces

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> I thought it was a feature. 😯  I don't use these duplicate faces, but
> it seems someone intentionally created a copy at the end of the face list.
> OTOH, we could wait until someone will protest this change.

Hm, it was

(append hi-lock--unused-faces
	(cdr (member last-used-face hi-lock-face-defaults))
	hi-lock-face-defaults)

And I guess that makes a weird kind of sense (if you want to avoid all
the other non-hi faces).  But...  It's pretty weird.

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