bug#43339: 27.1; Hi-Lock mode mishandles face names specified as strings

bug#43339: 27.1; Hi-Lock mode mishandles face names specified as strings

[Sean Devlin]

Hi folks,

Hi-Lock mode seems to get into an erroneous state when you call its
functions with the face name specified as a string rather than a
symbol. For example, open the scratch buffer and evaluate the following
forms:

    (require 'hi-lock)
    (hi-lock-face-buffer "create" "hi-yellow")
    hi-lock-interactive-patterns
    ;; (("create" (0 '"hi-yellow" prepend)))

The comment on the last line shows the value of
hi-lock-interactive-patterns after evaluating the forms. Note the quote
in front of "hi-yellow". Note also that no error has been raised. The
function has apparently worked correctly, and the string "create" is
highlighted in yellow.

Now move point onto some symbol and invoke interactively the command
hi-lock-face-symbol-at-point. There is an error message:

    Wrong type argument: symbolp, "hi-yellow"

Hi-Lock is passing this string to the face-name function, but it expects
a symbol.

I think the core issue is that Hi-Lock fails to canonicalize the face
name as a symbol in its internal data structures. It seems to rely on
the underlying faces API to enforce data consistency, but whereas facep
returns non-nil on string inputs, face-name does not allow them.

You might be able to argue that the above is an issue in the faces
library rather than Hi-Lock. I'm not sure if changing something at that
layer would break things elsewhere.

Either way, I think Hi-Lock should probably canonicalize its own data
structures and convert faces to symbols.

Note that everything works correctly when these functions are called
interactively, because the package does convert faces to symbols in
hi-lock-read-face-name.

Thanks!


In GNU Emacs 27.1 (build 1, x86_64-apple-darwin18.7.0, NS appkit-1671.60 Version 10.14.6 (Build 18G95))
of 2020-08-12 built on builder10-14.porkrind.org
Windowing system distributor 'Apple', version 10.3.2009
System Description:  macOS 11.0

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set
hi-lock
nil
(("create" (0 '"hi-yellow" prepend)))
nil [2 times]
Making completion list...
face-name: Wrong type argument: symbolp, "hi-blue"
Making completion list... [2 times]

Configured using:
'configure --with-ns '--enable-locallisppath=/Library/Application
Support/Emacs/${version}/site-lisp:/Library/Application
Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES
THREADS JSON PDUMPER

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

Major mode: Lisp Interaction

Minor modes in effect:
  hi-lock-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils thingatpt time-date
subr-x cl-loaddefs cl-lib hi-lock tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads kqueue cocoa ns
multi-tty make-network-process emacs)

Memory information:
((conses 16 46803 9943)
(symbols 48 6022 1)
(strings 32 15725 2224)
(string-bytes 1 515994)
(vectors 16 10380)
(vector-slots 8 129643 8794)
(floats 8 21 51)
(intervals 56 262 12)
(buffers 1000 12))

bug#43339: 27.1; Hi-Lock mode mishandles face names specified as strings

[Lars Ingebrigtsen]

Sean Devlin <spd@toadstyle.org> writes:

> Either way, I think Hi-Lock should probably canonicalize its own data
> structures and convert faces to symbols.

Yup.  This has now been fixed in Emacs 28.

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