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

From: Jambunathan K <[email-masked]>
Date: Fri, 12 Oct 2012 21:47:47 +0530

I am attaching a patch that addresses Part-II/Item-1 below.

This patch is 4-th in the series and applies on top of the third patch
seen here at http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11095#14.

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2012-10-11 20:39:05 +0000
+++ lisp/ChangeLog	2012-10-12 16:08:17 +0000
@@ -1,3 +1,10 @@
+2012-10-12  Jambunathan K  <[email-masked]>
+
+	* hi-lock.el (hi-lock-unface-buffer): Prompt user with useful
+	defaults.  Specifically, if cursor is on an highlighted text,
+	offer a regexp that matches that text as the default.  Update
+	docstring.
+
 2012-10-11  Jambunathan K  <[email-masked]>
 
 	* replace.el (read-regexp): Tighten the regexp that matches tag.

=== modified file 'lisp/hi-lock.el'
--- lisp/hi-lock.el	2012-10-11 20:39:05 +0000
+++ lisp/hi-lock.el	2012-10-12 15:55:49 +0000
@@ -493,8 +493,15 @@ updated as you type."
 ;;;###autoload
 (defun hi-lock-unface-buffer (regexp)
   "Remove highlighting of each match to REGEXP set by hi-lock.
-Interactively, prompt for REGEXP, accepting only regexps
-previously inserted by hi-lock interactive functions."
+Interactively, prompt for REGEXP.  If the cursor is on a
+previously highlighted text and if the associated regexp can be
+inferred via simple heuristics, offer that regexp as default.
+Otherwise, prompt for REGEXP with completion and limit the
+choices to only those regexps used previously with hi-lock
+commands.
+
+If this command is invoked via menu, make no attempt to infer
+REGEXP from text at point."
   (interactive
    (if (and (display-popup-menus-p)
 	    (listp last-nonmenu-event)
@@ -522,15 +529,51 @@ previously inserted by hi-lock interacti
 	  ;; To prevent that, we return an empty string, which will
 	  ;; effectively disable the rest of the function.
 	  (throw 'snafu '(""))))
-     (let ((history-list (mapcar (lambda (p) (car p))
-                                 hi-lock-interactive-patterns)))
+     ;; Un-highlighting triggered via keyboard action.
        (unless hi-lock-interactive-patterns
          (error "No highlighting to remove"))
+     ;; Infer the regexp to un-highlight based on cursor position.
+     (let* (candidate-hi-lock-patterns
+	    (default-regexp
+	      (or
+	       ;; When using overlays, there is no ambiguity on the best
+	       ;; choice of regexp.
+	       (let ((desired-serial (get-char-property
+				      (point) 'hi-lock-overlay-regexp)))
+		 (when desired-serial
+		   (catch 'regexp
+		     (maphash
+		      (lambda (regexp serial)
+			(when (= serial desired-serial)
+			  (throw 'regexp regexp)))
+		      hi-lock-string-serialize-hash))))
+	       ;; With font-locking on, check if the cursor is on an
+	       ;; highlighted text.  Checking for hi-lock face is a
+	       ;; good heuristic.
+	       (and (string-match "\\`hi-lock-" (face-name (face-at-point)))
+		    (let* ((hi-text
+			    (buffer-substring-no-properties
+			     (previous-single-property-change (point) 'face)
+			     (next-single-property-change (point) 'face))))
+		      ;; Compute hi-lock patterns that match the
+		      ;; highlighted text at point.  Use this later in
+		      ;; during completing-read.
+		      (setq candidate-hi-lock-patterns
+			    (delq nil
+				  (mapcar
+				   (lambda (hi-lock-pattern)
+				     (let ((regexp (car hi-lock-pattern)))
+				       (and (string-match regexp hi-text)
+					    hi-lock-pattern)))
+				   hi-lock-interactive-patterns)))
+		      ;; Use regexp from the first matching pattern as
+		      ;; a reasonable default.
+		      (caar candidate-hi-lock-patterns))))))
        (list
         (completing-read "Regexp to unhighlight: "
-                         hi-lock-interactive-patterns nil t
-                         (car (car hi-lock-interactive-patterns))
-                         (cons 'history-list 1))))))
+			 (or candidate-hi-lock-patterns
+			     hi-lock-interactive-patterns)
+			 nil t default-regexp)))))
   (let ((keyword (assoc regexp hi-lock-interactive-patterns)))
     (when keyword
       (font-lock-remove-keywords nil (list keyword))

> Proposal is in two parts.  Part-I deals with `hi-lock-face-buffer'.
> Part-II deals with `unhighlight-regexp'.  Part-III has a dump of the
> current customization I have in my .emacs.
>
> I believe that my proposal is useful in general.  So I request that it
> be folded in to Emacs-24.1.
>
> Part-I: `hi-lock-face-buffer' & Co.
> ----------------------------------
>
> 1) Review the face names used in `hi-lock-face-defaults' and make the
>    faces customizable.  The defaults may not look good on a user's his
>    own font-lock configuration.
>
> 2) Make `hi-lock-face-buffer' use a different face on each invocation.  
>
>    Here is a real-world usecase for the above request.
>
>    As a programmer, I use highlighting to trace variable dependencies
>    within a function.  For example, in the example below, after
>    highlighting the variables in __different__ faces, I will come to the
>    conclusion that "a" depends on "d" and "tmp".
>
>      c = d;
>      b = c + tmp;
>      a = b;
>
>    And I use this very often to track variables and how they get their
>    values from.
>
>    If I were to use the default Emacs provided behaviour then I would
>    have to press M-n multiple times as I highlight more and more
>    symbols. (Typically I have 3-5 symbols highlighted before I turn off
>    highlighting.)
>
> See elisp snippet at the end of the mail.
>
>
> Part-II: `unhighlight-regexp'
> ------------------------------
>
> See usecase in Part-I/Item-2 
>
> 1) I want to selectively turn-off highlighting for certain regexps
>    (arguably) that _specific_ highlighted regexp cursor is stationed on.
>    This would happen when I decide that I don't want to factor a
>    particular variable for my current refactoring exercise.
>
>    I find the current behaviour of `unhighlight-regexp' slightly
>    tedious.  
>
>    1. There is no completion for which regexp I want to unhighlight and
>       I have to circle through `hi-lock-interactive-patterns'.
>
>    2. Browsing the `hi-lock-interactive-patterns' is tedious for the
>       following reasons:
>
>       - The order in which unhighlighting happens could totally be
>         unrelated to the order in which highlighting happens.  When I am
>         analyzing the variable flow, I don't want to do a "context
>         switch" trying to find out what item to choose from the
>         `unhighlight-regexp' menu.
>
> 2) I want to unhighlight all regexps.  This happens when I am done with
>    variable analysis.
>
>
> ps: I am not questioning the current defaults.  I am only saying that it
> the current behaviour is too generic to be immediately useful (atleast
> for my usecase) and so needs some sort of extra augmentation.
>
> Part-III: Elisp snippet
> -----------------------
>
> ;; Why should the color of the faces be encoded in the variable name?
> ;; Seems counter-intutitive to me.  I cannot relate to a hi-yellow
> ;; face customized to render in red.
>
> ;; (defvar 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.")
>
> ;; So roll out my own face for highlighting.  Make them
> ;; __customizable__.  Note that the name of the face doesn't say what
> ;; the color of the highlight will be.  Works for the color theme that
> ;; I have.
> (custom-set-faces
>  '(highlight1 ((t (:background "yellow" :foreground "black"))))
>  '(highlight2 ((t (:background "OrangeRed" :foreground "black"))))
>  '(highlight3 ((((class color) (background dark)) (:background "AntiqueWhite" :foreground "black"))))
>  '(highlight4 ((t (:background "SystemHotTrackingColor"))))
>  '(highlight5 ((t (:background "VioletRed" :foreground "black")))))
>
> ;; override the Emacs default
> (setq hi-lock-face-defaults
>       '("highlight1" "highlight2" "highlight3" "highlight4" "highlight5"))
>
> (defvar hi-lock-faces
>   (let ((l (copy-list hi-lock-face-defaults)))
>     (setcdr (last l) l))
>   "Circular list of faces in `hi-lock-face-defaults'.")
>
> ;; make `hi-lock-faces' buffer local
> (make-variable-buffer-local 'hi-lock-faces)
>
> (defun highlight-symbol ()
>   "Highlight symbol at point.  
> For illustartion only.  Note the use of `hi-lock-face-buffer'.
> Choose a new face from `hi-lock-faces' on each invocation.
> Overrides the default behaviour in vanilla Emacs which is to use
> the face at the head of the list."
>   (interactive)
>   (hi-lock-face-buffer
>    (concat "\\_<" (regexp-quote (thing-at-point 'symbol)) "\\_>")
>    ;; rotate the face list
>    (prog1 (car hi-lock-faces)
>      (setq hi-lock-faces (cdr hi-lock-faces)))))
>
> (defun my-unhighlight-regexp (arg)
>   "Wrapper around `unhighlight-regexp'.
> With a prefix argument, turn off all highlighting.
>
> TODO: Check if the symbol is right now on a highlighted regexp.
> If yes, unhighlight only that regexp."
>   (interactive "P")
>   (if arg
>       (mapc (lambda (p)
> 	      (unhighlight-regexp (car p)))
> 	    hi-lock-interactive-patterns)
>     (call-interactively 'unhighlight-regexp)))
>
>
>
>

--