Subject: Re: bug#32365: Interactive Highlighting: prefix arg as subexp selector

From: Noam Postavsky <[email-masked]>
Date: Sun, 05 Aug 2018 23:07:46 -0400

tags 32365 + patch
forcemerge 32362 32365
quit

Grégory Mounié <[email-masked]> writes:

> PS: Sorry for generating noise with my previous patch (BUG: 32362)

No problem, I'll just merge the bugs.  Next time, you can respond to
your own bug report by sending mail to [email-masked], and the
new patch would be added to the same thread.

>>>From 6b6291ce1974a363080f535b40f06d5772ffa1be Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Gr=C3=A9gory=20Mouni=C3=A9?= <[email-masked]>
> Date: Fri, 3 Aug 2018 23:08:10 +0200
> Subject: [PATCH] Interactive Highlighting: prefix argument to select subexp
>
> Use prefix-argument to highlight only the corresponding
> subexpression of the regexp.
>
> * lisp/hi-lock.el (hi-lock-face-buffer, hi-lock-set-pattern)

The message should go after the ChangeLog entry, as in

* lisp/hi-lock.el (hi-lock-face-buffer, hi-lock-set-pattern): Use
prefix-argument to highlight only the corresponding subexpression of the
regexp.

And I think we should have a NEWS entry mentioning the new feature too.

> -(defun hi-lock-face-buffer (regexp &optional face)
> +(defun hi-lock-face-buffer (regexp &optional face subexp)
>    "Set face of each match of REGEXP to FACE.
>  Interactively, prompt for REGEXP using `read-regexp', then FACE.
>  Use the global history list for FACE.

> -(defun hi-lock-set-pattern (regexp face)
> +(defun hi-lock-set-pattern (regexp face &optional arg)
>    "Highlight REGEXP with face FACE."

You should mention the new arg in the docstrings.
  
> +  (let* ((subexp (if (null arg) 0 arg))

This kind of thing is usually expressesd as (or arg 0).  Though since
`arg' is a bit vague, it might be clearer to rename arg as subexp, and
then inside the function do

    (setq subexp (or subexp 0))

> +                (let ((overlay (make-overlay (match-beginning subexp)
> +                                             (match-end subexp))))
> +                  (overlay-put overlay 'hi-lock-overlay t)
> +                (overlay-put overlay 'hi-lock-orverlay-regexp regexp)
                                                 ^^^^^^^^
Looks like a typo.