bug#11095: [PATCH] Re: bug#11095: 24.0.94; hi-lock-face-buffer/unhighlight-regexp': Augment?

[Jambunathan K]

There are three issues that I see with your commmit:

Issue-1:  face-at-point broken?
===============================

M-x toggle-debug-on-error RET
M-x find-function RET face-at-point RET
C-x w h
C-x w r

    Debugger entered--Lisp error: (error "Not a face: nil")
      signal(error ("Not a face: nil"))
      error("Not a face: %s" nil)
      check-face(nil)
      face-name(nil)
      hi-lock--regexps-at-point()
      byte-code("\203\305C\207\306 \203.      <\203.\n\203.\307\310\215\207
\204!\311\312!\210\313 \314\f\204-\315\2022\316\317\f@\"
\320\305\320\211\f&)C\207" [current-prefix-arg last-nonmenu-event 
use-dialog-box hi-lock-interactive-patterns defaults t display-popup-menus-p 
snafu (byte-code "\301\302\303\304\305\306\"BB\"\206.\307\310\311\"\207" 
[hi-lock-interactive-patterns x-popup-menu t keymap "Select Pattern to 
Unhighlight" mapcar #[(pattern) "@\301\302@\303A@A@A@!#\304\211B@F\207" 
[pattern format "%s (%s)" symbol-name nil] 6] throw snafu ("")] 7) error "No 
highlighting to remove" hi-lock--regexps-at-point completing-read "Regexp to 
unhighlight: " format "Regexp to unhighlight (default %s): " nil] 8)
      call-interactively(unhighlight-regexp nil nil)

The reason is faceprop happens to be a string

(get-char-property (point) 'face) 
 : "hi-yellow"

Issue-2:  Various issues with unhighlighting 
============================================

Once you fix Issue-1 you will run in to other issues with
un-highlighting.  Try highlighting and UN-highlighting in following 3
ways

1. Buffer with font-lock-mode ON
2. Buffer with font-lock-mode OFF
3. Unhighlight from the menu

Caveat:  Extra testing needed if /type/ of face names are changed 
=================================================================

hi-lock-face-defautls is currently a list of face names (stringp).  If
it is made a defcustom, it will be cast to a list of symbols (symbolp).
In some places, face names are expected and in some other places face as
a symbol is used.  So you need to re-run the tests if move from
string->symbols.

Suggestion: In default faces, don't mix bold and foreground/background
=======================================================================

I am OK with defcustom of faces.  Something like

    (defcustom hi-lock-face-defaults
      '(hi-yellow hi-pink hi-green hi-blue hi-black-b
                  hi-blue-b hi-red-b hi-green-b hi-black-hb)
      "Default faces for hi-lock interactive functions."
      :type '(repeat face)
      :group 'hi-lock-faces)

Bonus points if the default settings of the faces that go in there is
revised as part of this bug.  I want to highlight variables in a buffer.
So consistent policy of highlighting - a changed background of normal
face - will require no additional work.

Here is how my own faces look like.  Note that the first 4 come from
"blue" space and the later 4 or so come from "pink" space, all chosen
using agave.

ps:  I will let you install a change for the above issues.

hi-defaults.png
Description: hi-defaults.pn

my-hi-faces.png
Description: my-hi-faces.png