GNU bug report logs - #6227
Color isearch regexp submatches differently

Previous Next

Package: emacs;

Reported by: Lennart Borgman <lennart.borgman <at> gmail.com>

Date: Thu, 20 May 2010 11:13:01 UTC

Severity: wishlist

Tags: fixed, patch

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6227 in the body.
You can then email your comments to 6227 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 20 May 2010 11:13:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Lennart Borgman <lennart.borgman <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 20 May 2010 11:13:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Emacs Bugs <bug-gnu-emacs <at> gnu.org>
Subject: Color isearch regexp submatches differently
Date: Thu, 20 May 2010 13:01:37 +0200
Just a suggestion, of course.




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 20 May 2010 13:23:01 GMT) Full text and rfc822 format available.

Message #8 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Lennart Borgman'" <lennart.borgman <at> gmail.com>, <6227 <at> debbugs.gnu.org>
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Thu, 20 May 2010 06:21:32 -0700
[Message part 1 (text/plain, inline)]
Did you mean something like this (attached)?
This is how I highlight submatches in Icicles search.

(The top part of the image, with light blue background, shows the highlighting.
The bottom part of the image, with white background, shows the regexp used and
is just an explanation of the subgroup highlighting.)

[drew-emacs-icicle-search-context-colors.png (image/png, attachment)]

Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 20 May 2010 13:29:01 GMT) Full text and rfc822 format available.

Message #11 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Thu, 20 May 2010 15:28:07 +0200
On Thu, May 20, 2010 at 3:21 PM, Drew Adams <drew.adams <at> oracle.com> wrote:
> Did you mean something like this (attached)?
> This is how I highlight submatches in Icicles search.
>
> (The top part of the image, with light blue background, shows the highlighting.
> The bottom part of the image, with white background, shows the regexp used and
> is just an explanation of the subgroup highlighting.)

Yes, exactly.




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 20 May 2010 13:35:02 GMT) Full text and rfc822 format available.

Message #14 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Lennart Borgman'" <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Thu, 20 May 2010 06:33:50 -0700
> > Did you mean something like this (attached)?
> > This is how I highlight submatches in Icicles search.
> >
> > (The top part of the image, with light blue background, 
> > shows the highlighting. The bottom part of the image,
> > with white background, shows the regexp used and
> > is just an explanation of the subgroup highlighting.)
> 
> Yes, exactly.

IMO, this can be very helpful when searching with regexps.
And it can help users learn about using regexps more generally.

Of course, such highlighting should be optional, and preferably via a toggle
during Isearch.





Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 20 May 2010 15:04:01 GMT) Full text and rfc822 format available.

Message #17 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Lennart Borgman'" <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Thu, 20 May 2010 08:02:53 -0700
> > Yes, exactly.
> 
> IMO, this can be very helpful when searching with regexps.
> And it can help users learn about using regexps more generally.
> 
> Of course, such highlighting should be optional, and 
> preferably via a toggle
> during Isearch.

Need I add that this need not be shown for all search hits simultaneously
(costly and distracting to the user, in general). Just the current hit is
sufficient (what is shown now using face `isearch', not `lazy-highlight').





Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Fri, 21 May 2010 00:13:02 GMT) Full text and rfc822 format available.

Message #20 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Fri, 21 May 2010 03:07:21 +0300
> Just a suggestion, of course.

We already have highlighting like that: lisp/emacs-lisp/re-builder.el
uses faces `reb-match-1', `reb-match-2', `reb-match-3' to highlight
regexp subexpressions.  I think this should be used by isearch.

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Fri, 21 May 2010 01:21:02 GMT) Full text and rfc822 format available.

Message #23 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Fri, 21 May 2010 03:19:48 +0200
On Fri, May 21, 2010 at 2:07 AM, Juri Linkov <juri <at> jurta.org> wrote:
>> Just a suggestion, of course.
>
> We already have highlighting like that: lisp/emacs-lisp/re-builder.el
> uses faces `reb-match-1', `reb-match-2', `reb-match-3' to highlight
> regexp subexpressions.  I think this should be used by isearch.

That sounds right to me.

Also Drew suggestion to not color submatches in lazy marking seems right.




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 23 May 2010 00:40:04 GMT) Full text and rfc822 format available.

Message #26 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 23 May 2010 02:44:04 +0300
>> We already have highlighting like that: lisp/emacs-lisp/re-builder.el
>> uses faces `reb-match-1', `reb-match-2', `reb-match-3' to highlight
>> regexp subexpressions.  I think this should be used by isearch.
>
> That sounds right to me.
>
> Also Drew suggestion to not color submatches in lazy marking seems right.

(add-hook 'isearch-update-post-hook
          (lambda ()
	    (require 're-builder)
	    (when isearch-regexp
	      (let ((reb-regexp isearch-string)
		    (reb-target-buffer (current-buffer))
		    (reb-target-window (selected-window)))
		(reb-update-overlays)))))

(add-hook 'isearch-mode-end-hook
          (lambda ()
	    (let ((reb-target-buffer (current-buffer)))
	      (reb-delete-overlays))))

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 23 May 2010 00:53:02 GMT) Full text and rfc822 format available.

Message #29 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 23 May 2010 02:51:48 +0200
On Sun, May 23, 2010 at 1:44 AM, Juri Linkov <juri <at> jurta.org> wrote:
>>> We already have highlighting like that: lisp/emacs-lisp/re-builder.el
>>> uses faces `reb-match-1', `reb-match-2', `reb-match-3' to highlight
>>> regexp subexpressions.  I think this should be used by isearch.
>>
>> That sounds right to me.
>>
>> Also Drew suggestion to not color submatches in lazy marking seems right.
>
> (add-hook 'isearch-update-post-hook
>          (lambda ()
>            (require 're-builder)
>            (when isearch-regexp
>              (let ((reb-regexp isearch-string)
>                    (reb-target-buffer (current-buffer))
>                    (reb-target-window (selected-window)))
>                (reb-update-overlays)))))
>
> (add-hook 'isearch-mode-end-hook
>          (lambda ()
>            (let ((reb-target-buffer (current-buffer)))
>              (reb-delete-overlays))))


Nice. So I suggest moving (and renaming) `reb-count-subexps' to
isearch.el and splitting off the marking of one overlay from
`reb-update-overlays' and moving that too to isearch.el (since
isearch.el) is probably always loaded for a normal Emacs user).




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 23 May 2010 01:46:02 GMT) Full text and rfc822 format available.

Message #32 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 23 May 2010 03:54:34 +0300
> Nice. So I suggest moving (and renaming) `reb-count-subexps' to
> isearch.el and splitting off the marking of one overlay from
> `reb-update-overlays' and moving that too to isearch.el (since
> isearch.el) is probably always loaded for a normal Emacs user).

I think `reb-update-overlays' should be completely rewritten
for isearch.el.

The only thing we need from re-builder.el are faces
reb-match-1, reb-match-2, reb-match-3.  We should try
using the existing faces for the same functionality.

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 23 May 2010 10:06:02 GMT) Full text and rfc822 format available.

Message #35 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 23 May 2010 12:04:50 +0200
On Sun, May 23, 2010 at 2:54 AM, Juri Linkov <juri <at> jurta.org> wrote:
>> Nice. So I suggest moving (and renaming) `reb-count-subexps' to
>> isearch.el and splitting off the marking of one overlay from
>> `reb-update-overlays' and moving that too to isearch.el (since
>> isearch.el) is probably always loaded for a normal Emacs user).
>
> I think `reb-update-overlays' should be completely rewritten
> for isearch.el.


You surely know this things much better than me, but is there any
reason to double the code? If it is rewritten why not let re-builder
share the same code?


> The only thing we need from re-builder.el are faces
> reb-match-1, reb-match-2, reb-match-3.  We should try
> using the existing faces for the same functionality.




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 23 May 2010 16:16:01 GMT) Full text and rfc822 format available.

Message #38 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 23 May 2010 19:12:35 +0300
>> I think `reb-update-overlays' should be completely rewritten
>> for isearch.el.
>
> You surely know this things much better than me, but is there any
> reason to double the code?

`reb-update-overlays' highlights all matches in the buffer.
This is like what lazy-highlighting does.  But we agreed
that it should affect only the current isearch match,
not all lazy-highlighted matches.

> If it is rewritten why not let re-builder share the same code?

Yes, and query-replace highlighting could share it too.

>> The only thing we need from re-builder.el are faces
>> reb-match-1, reb-match-2, reb-match-3.  We should try
>> using the existing faces for the same functionality.

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 23 May 2010 16:42:01 GMT) Full text and rfc822 format available.

Message #41 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 23 May 2010 18:40:58 +0200
On Sun, May 23, 2010 at 6:12 PM, Juri Linkov <juri <at> jurta.org> wrote:
>>> I think `reb-update-overlays' should be completely rewritten
>>> for isearch.el.
>>
>> You surely know this things much better than me, but is there any
>> reason to double the code?
>
> `reb-update-overlays' highlights all matches in the buffer.
> This is like what lazy-highlighting does.  But we agreed
> that it should affect only the current isearch match,
> not all lazy-highlighted matches.
>
>> If it is rewritten why not let re-builder share the same code?
>
> Yes, and query-replace highlighting could share it too.


I see. A misunderstanding, we mean the same. I wrote the
reb-update-overlays should be split and I meant then into one function
that hilights only one match (which is given as a parameter) and one
that loops for reb-update-overlays.


>>> The only thing we need from re-builder.el are faces
>>> reb-match-1, reb-match-2, reb-match-3.  We should try
>>> using the existing faces for the same functionality.
>
> --
> Juri Linkov
> http://www.jurta.org/emacs/
>




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Tue, 08 Jun 2010 13:39:01 GMT) Full text and rfc822 format available.

Message #44 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Tue, 8 Jun 2010 15:37:50 +0200
[Message part 1 (text/plain, inline)]
On Sun, May 23, 2010 at 6:40 PM, Lennart Borgman
<lennart.borgman <at> gmail.com> wrote:
> On Sun, May 23, 2010 at 6:12 PM, Juri Linkov <juri <at> jurta.org> wrote:
>>>> I think `reb-update-overlays' should be completely rewritten
>>>> for isearch.el.
>>>
>>> You surely know this things much better than me, but is there any
>>> reason to double the code?
>>
>> `reb-update-overlays' highlights all matches in the buffer.
>> This is like what lazy-highlighting does.  But we agreed
>> that it should affect only the current isearch match,
>> not all lazy-highlighted matches.
>>
>>> If it is rewritten why not let re-builder share the same code?
>>
>> Yes, and query-replace highlighting could share it too.
>
>>>> The only thing we need from re-builder.el are faces
>>>> reb-match-1, reb-match-2, reb-match-3.  We should try
>>>> using the existing faces for the same functionality.

Here is a patch for the submatches highlighting. (It includes a bug
fix for the prompt face too and a help window scrolling I think is
useful.)

The current faces does not look very well together so that must be fixed.
[isearch-hisub-1.diff (text/x-patch, attachment)]

Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Wed, 09 Jun 2010 08:56:02 GMT) Full text and rfc822 format available.

Message #47 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Wed, 09 Jun 2010 11:36:38 +0300
> Here is a patch for the submatches highlighting.
> (It includes a bug fix for the prompt face too

What's a bug in the prompt face?

> and a help window scrolling I think is useful.)

Please provide an example of the scrolling bug too.

> The current faces does not look very well together so that must be fixed.

If current faces does not look well, then maybe we should completely
get rid of using re-builder.el in isearch, its faces and messy functions
like count-subexps, and to write this functionality for isearch from scratch.

Do you think something more complicated is necessary for this
functionality than the following simple code:

(defvar isearch-sub-overlays nil)
(add-hook 'isearch-update-post-hook
          (lambda ()
            ;; This code could be added to `isearch-highlight'.
            (mapc 'delete-overlay isearch-sub-overlays)
            (setq isearch-sub-overlays nil)
            (when isearch-regexp
              (dolist (i '(1 2 3 4))
                (when (match-beginning i)
                  (let ((ov (make-overlay (match-beginning i) (match-end i))))
                    (overlay-put ov 'face (intern-soft (format "isearch-%d" i)))
                    (overlay-put ov 'priority 1002)
                    (push ov isearch-sub-overlays)))))))

It relies on new faces `isearch-1', `isearch-2', `isearch-3', `isearch-4'.
As for face colors, I tried "magenta1", "magenta2", "magenta3", "magenta4"
for background colors, and they look good.

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Wed, 09 Jun 2010 09:15:03 GMT) Full text and rfc822 format available.

Message #50 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Wed, 9 Jun 2010 11:14:03 +0200
On Wed, Jun 9, 2010 at 10:36 AM, Juri Linkov <juri <at> jurta.org> wrote:
>> Here is a patch for the submatches highlighting.
>> (It includes a bug fix for the prompt face too
>
> What's a bug in the prompt face?


There is a variable, minibuffer-prompt-properties, that holds the name
of the face to use. I think this should be used for consistency. It
makes it much easier for users.


>> and a help window scrolling I think is useful.)
>
> Please provide an example of the scrolling bug too.


I am not sure on that one. Maybe I just forgot to remove it when you
implemented your way of doing it?


>> The current faces does not look very well together so that must be fixed.
>
> If current faces does not look well, then maybe we should completely
> get rid of using re-builder.el in isearch, its faces and messy functions
> like count-subexps, and to write this functionality for isearch from scratch.


I thought maybe something like count-subexps is needed now with the
numbered submatches.


> Do you think something more complicated is necessary for this
> functionality than the following simple code:
>
> (defvar isearch-sub-overlays nil)
> (add-hook 'isearch-update-post-hook
>          (lambda ()
>            ;; This code could be added to `isearch-highlight'.
>            (mapc 'delete-overlay isearch-sub-overlays)
>            (setq isearch-sub-overlays nil)
>            (when isearch-regexp
>              (dolist (i '(1 2 3 4))
>                (when (match-beginning i)
>                  (let ((ov (make-overlay (match-beginning i) (match-end i))))
>                    (overlay-put ov 'face (intern-soft (format "isearch-%d" i)))
>                    (overlay-put ov 'priority 1002)
>                    (push ov isearch-sub-overlays)))))))


It does not take care of numbered matches. But, yes, why not guess? I
agree, your approach is probably better. But check for more
submatches. Maybe upto the value of some variable, say
isearch-max-submatch-num.


> It relies on new faces `isearch-1', `isearch-2', `isearch-3', `isearch-4'.
> As for face colors, I tried "magenta1", "magenta2", "magenta3", "magenta4"
> for background colors, and they look good.


The problem with mixing isearch faces with re-builder dito was the
resulting colors from merging. If it works then just use your
suggestions.

I  have rewritten re-builder.el and got rid of its internal. Just need
to cleanup a bit. Now it is just a front end to isearch with more
editing capabilities, like rx. I think that can be useful.

I plan to keep three "regexp source styles" there and maybe rename
them a bit: regexp, string (or maybe read) and rx. I think those names
are self explanatory. Unfortunately re-builder now uses string/read
instead of regexp/string which is more user-level names.




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 10 Jun 2010 15:40:02 GMT) Full text and rfc822 format available.

Message #53 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Thu, 10 Jun 2010 18:28:57 +0300
>>> (It includes a bug fix for the prompt face too
>>
>> What's a bug in the prompt face?
>
> There is a variable, minibuffer-prompt-properties, that holds the name
> of the face to use. I think this should be used for consistency. It
> makes it much easier for users.

Why do you remove `read-only' with `(propertize m2 'read-only nil)'?
What was a problem with `read-only' in the isearch prompt?

>>> and a help window scrolling I think is useful.)
>>
>> Please provide an example of the scrolling bug too.
>
> I am not sure on that one. Maybe I just forgot to remove it when you
> implemented your way of doing it?

I see no scrolling problems after setting `isearch-allow-scroll' to t.

> I agree, your approach is probably better. But check for more
> submatches. Maybe upto the value of some variable, say
> isearch-max-submatch-num.

Good idea.

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 10 Jun 2010 15:53:03 GMT) Full text and rfc822 format available.

Message #56 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Thu, 10 Jun 2010 17:52:21 +0200
On Thu, Jun 10, 2010 at 5:28 PM, Juri Linkov <juri <at> jurta.org> wrote:
>>>> (It includes a bug fix for the prompt face too
>>>
>>> What's a bug in the prompt face?
>>
>> There is a variable, minibuffer-prompt-properties, that holds the name
>> of the face to use. I think this should be used for consistency. It
>> makes it much easier for users.
>
> Why do you remove `read-only' with `(propertize m2 'read-only nil)'?
> What was a problem with `read-only' in the isearch prompt?

Hm, can't remember. I did this quite a while ago. I just tested and it
seems to work without that. Probably it is something I forgot to
remove after testing.

>>>> and a help window scrolling I think is useful.)
>>>
>>> Please provide an example of the scrolling bug too.
>>
>> I am not sure on that one. Maybe I just forgot to remove it when you
>> implemented your way of doing it?
>
> I see no scrolling problems after setting `isearch-allow-scroll' to t.

Seems that you are right. Fine, just skip that part of the patch.




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 10 Jun 2010 20:55:02 GMT) Full text and rfc822 format available.

Message #59 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Thu, 10 Jun 2010 23:52:39 +0300
>> I agree, your approach is probably better. But check for more
>> submatches. Maybe upto the value of some variable, say
>> isearch-max-submatch-num.
>
> Good idea.

Maybe something like this:

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2010-06-10 14:32:41 +0000
+++ lisp/isearch.el	2010-06-10 20:51:43 +0000
@@ -223,6 +223,15 @@ (defcustom search-highlight t
   :type 'boolean
   :group 'isearch)
 
+(defcustom search-highlight-submatches 0
+  "Highlight regexp subexpressions of the current regexp match.
+An integer means highlight regexp subexpressions up to the
+specified maximal number.
+When 0, do not highlight regexp subexpressions."
+  :type 'integer
+  :version "24.1"
+  :group 'isearch)
+
 (defface isearch
   '((((class color) (min-colors 88) (background light))
      ;; The background must not be too dark, for that means
@@ -2526,6 +2535,23 @@ (defun isearch-unread (&rest char-or-eve
 ;; Highlighting
 
 (defvar isearch-overlay nil)
+(defvar isearch-submatches-overlays nil)
+
+(defface isearch-1
+  '((((class color) (min-colors 88) (background light))
+     :background "magenta2" :foreground "lightskyblue1")
+    (((class color) (min-colors 88) (background dark))
+     :background "palevioletred3" :foreground "brown4"))
+  "Used for displaying the first matching subexpression."
+  :group 'isearch)
+
+(defface isearch-2
+  '((((class color) (min-colors 88) (background light))
+     :background "magenta1" :foreground "lightskyblue1")
+    (((class color) (min-colors 88) (background dark))
+     :background "palevioletred4" :foreground "brown4"))
+  "Used for displaying the second matching subexpression."
+  :group 'isearch)
 
 (defun isearch-highlight (beg end)
   (if search-highlight
@@ -2536,11 +2562,28 @@ (defun isearch-highlight (beg end)
 	(setq isearch-overlay (make-overlay beg end))
 	;; 1001 is higher than lazy's 1000 and ediff's 100+
 	(overlay-put isearch-overlay 'priority 1001)
-	(overlay-put isearch-overlay 'face isearch))))
+	(overlay-put isearch-overlay 'face isearch)))
+  (when (and (integerp search-highlight-submatches)
+	     (> search-highlight-submatches 0)
+	     isearch-regexp)
+    (mapc 'delete-overlay isearch-submatches-overlays)
+    (setq isearch-submatches-overlays nil)
+    (let ((i 0) ov)
+      (while (<= i search-highlight-submatches)
+	(when (match-beginning i)
+	  (setq ov (make-overlay (match-beginning i) (match-end i)))
+	  (overlay-put ov 'face (intern-soft (format "isearch-%d" i)))
+	  (overlay-put ov 'priority 1002)
+	  (push ov isearch-submatches-overlays))
+	(setq i (1+ i))))))
 
 (defun isearch-dehighlight ()
   (when isearch-overlay
-    (delete-overlay isearch-overlay)))
+    (delete-overlay isearch-overlay))
+  (when search-highlight-submatches
+    (mapc 'delete-overlay isearch-submatches-overlays)
+    (setq isearch-submatches-overlays nil)))
+
 
 ;; isearch-lazy-highlight feature
 ;; by Bob Glickstein <http://www.zanshin.com/~bobg/>

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Thu, 10 Jun 2010 21:43:02 GMT) Full text and rfc822 format available.

Message #62 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lennart Borgman <lennart.borgman <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Thu, 10 Jun 2010 23:41:40 +0200
On Thu, Jun 10, 2010 at 10:52 PM, Juri Linkov <juri <at> jurta.org> wrote:
>>> I agree, your approach is probably better. But check for more
>>> submatches. Maybe upto the value of some variable, say
>>> isearch-max-submatch-num.
>>
>> Good idea.
>
> Maybe something like this:

Yes,

> +(defcustom search-highlight-submatches 0

but set I suggest a default of 100. The loop costs essentially nothing
for non-submatches and this is on command level.

> +    (let ((i 0) ov)
> +      (while (<= i search-highlight-submatches)
> +       (when (match-beginning i)
> +         (setq ov (make-overlay (match-beginning i) (match-end i)))
> +         (overlay-put ov 'face (intern-soft (format "isearch-%d" i)))
> +         (overlay-put ov 'priority 1002)
> +         (push ov isearch-submatches-overlays))
> +       (setq i (1+ i))))))




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Fri, 11 Jun 2010 00:45:02 GMT) Full text and rfc822 format available.

Message #65 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lennart Borgman <lennart.borgman <at> gmail.com>
Cc: Juri Linkov <juri <at> jurta.org>, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Thu, 10 Jun 2010 20:44:42 -0400
> but set I suggest a default of 100. The loop costs essentially nothing
> for non-submatches and this is on command level.

(/ (length (match-data)) 2)


        Stefan




Information forwarded to owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Fri, 11 Jun 2010 08:19:02 GMT) Full text and rfc822 format available.

Message #68 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lennart Borgman <lennart.borgman <at> gmail.com>, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Fri, 11 Jun 2010 11:17:19 +0300
>> but set I suggest a default of 100. The loop costs essentially nothing
>> for non-submatches and this is on command level.
>
> (/ (length (match-data)) 2)

Then the loop should iterate from 1 up to the minimum of
`(/ (length (match-data)) 2)' and `search-highlight-submatches'.

As for the faces, perhaps we should dynamically create submatch faces
like in `vc-annotate-lines' that uses `vc-annotate-color-map'.

-- 
Juri Linkov
http://www.jurta.org/emacs/




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sat, 19 Sep 2020 21:31:02 GMT) Full text and rfc822 format available.

Message #71 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> jurta.org>
Cc: Lennart Borgman <lennart.borgman <at> gmail.com>, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sat, 19 Sep 2020 23:29:48 +0200
Juri Linkov <juri <at> jurta.org> writes:

>>> I agree, your approach is probably better. But check for more
>>> submatches. Maybe upto the value of some variable, say
>>> isearch-max-submatch-num.
>>
>> Good idea.
>
> Maybe something like this:

The ten year old patch no longer applied to Emacs 28, so I've respun it.

I think the results are really nice.  I guess we should add a few more
faces before applying?

Anybody else got any comments on this?

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 7fb1d8a3ca..56eb443d31 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -269,6 +269,14 @@ search-highlight
   "Non-nil means incremental search highlights the current match."
   :type 'boolean)
 
+(defcustom search-highlight-submatches 2
+  "Highlight regexp subexpressions of the current regexp match.
+An integer means highlight regexp subexpressions up to the
+specified maximal number.
+When 0, do not highlight regexp subexpressions."
+  :type 'integer
+  :version "28.1")
+
 (defface isearch
   '((((class color) (min-colors 88) (background light))
      ;; The background must not be too dark, for that means
@@ -3654,6 +3662,21 @@ isearch-unread
 ;; Highlighting
 
 (defvar isearch-overlay nil)
+(defvar isearch-submatches-overlays nil)
+
+(defface isearch-1
+  '((((class color) (min-colors 88) (background light))
+     :background "magenta2" :foreground "lightskyblue1")
+    (((class color) (min-colors 88) (background dark))
+     :background "palevioletred3" :foreground "brown4"))
+  "Used for displaying the first matching subexpression.")
+
+(defface isearch-2
+  '((((class color) (min-colors 88) (background light))
+     :background "magenta1" :foreground "lightskyblue1")
+    (((class color) (min-colors 88) (background dark))
+     :background "palevioletred4" :foreground "brown4"))
+  "Used for displaying the second matching subexpression.")
 
 (defun isearch-highlight (beg end)
   (if search-highlight
@@ -3664,11 +3687,28 @@ isearch-highlight
 	(setq isearch-overlay (make-overlay beg end))
 	;; 1001 is higher than lazy's 1000 and ediff's 100+
 	(overlay-put isearch-overlay 'priority 1001)
-	(overlay-put isearch-overlay 'face isearch-face))))
+	(overlay-put isearch-overlay 'face isearch-face)))
+  (when (and (integerp search-highlight-submatches)
+	     (> search-highlight-submatches 0)
+	     isearch-regexp)
+    (mapc 'delete-overlay isearch-submatches-overlays)
+    (setq isearch-submatches-overlays nil)
+    (let ((i 0) ov)
+      (while (<= i search-highlight-submatches)
+	(when (match-beginning i)
+	  (setq ov (make-overlay (match-beginning i) (match-end i)))
+	  (overlay-put ov 'face (intern-soft (format "isearch-%d" i)))
+	  (overlay-put ov 'priority 1002)
+	  (push ov isearch-submatches-overlays))
+	(setq i (1+ i))))))
 
 (defun isearch-dehighlight ()
   (when isearch-overlay
-    (delete-overlay isearch-overlay)))
+    (delete-overlay isearch-overlay))
+  (when search-highlight-submatches
+    (mapc 'delete-overlay isearch-submatches-overlays)
+    (setq isearch-submatches-overlays nil)))
+
 
 ;; isearch-lazy-highlight feature
 ;; by Bob Glickstein <http://www.zanshin.com/~bobg/>

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




Added tag(s) patch. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Sat, 19 Sep 2020 21:31:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sat, 19 Sep 2020 22:20:02 GMT) Full text and rfc822 format available.

Message #76 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Juri Linkov <juri <at> jurta.org>
Cc: Lennart Borgman <lennart.borgman <at> gmail.com>, 6227 <at> debbugs.gnu.org
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Sat, 19 Sep 2020 15:19:30 -0700 (PDT)
> I guess we should add a few more faces before applying?
> 
> Anybody else got any comments on this?

Haven't tried the patch.  But Isearch+ does this.
It uses 8 faces, for the first 8 regexp levels
(groups).

(The code could be go further, repeating those 8
faces if needed for more groups, but I haven't found
that that's needed.)

For lazy-highlighting, the odd groups are highlighted
differently from the even groups.  The odd groups use
face `isearchp-lazy-odd-regexp-groups'.  That simple
lazy highlighting actually helps quite a bit, I find.  

There is also a user option for whether to highlight
groups: `isearchp-highlight-regexp-group-levels-flag'.
And a toggle command for that option: `M-s h R'
(prefix `M-s' is for all Isearch stuff, `h' is for
highlighting, and `R' is for regexp groups).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 05:40:02 GMT) Full text and rfc822 format available.

Message #79 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 08:39:45 +0300
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Sat, 19 Sep 2020 23:29:48 +0200
> Cc: Lennart Borgman <lennart.borgman <at> gmail.com>, 6227 <at> debbugs.gnu.org
> 
> I think the results are really nice.  I guess we should add a few more
> faces before applying?
> 
> Anybody else got any comments on this?

Comment #1: Do we really want to turn this feature on by default?

> +(defface isearch-1
> +  '((((class color) (min-colors 88) (background light))
> +     :background "magenta2" :foreground "lightskyblue1")
> +    (((class color) (min-colors 88) (background dark))
> +     :background "palevioletred3" :foreground "brown4"))
> +  "Used for displaying the first matching subexpression.")
> +
> +(defface isearch-2
> +  '((((class color) (min-colors 88) (background light))
> +     :background "magenta1" :foreground "lightskyblue1")
> +    (((class color) (min-colors 88) (background dark))
> +     :background "palevioletred4" :foreground "brown4"))
> +  "Used for displaying the second matching subexpression.")

Comment #2: This seems to effectively disable the feature on displays
that have fewer than 88 colors.  Is that intentional?  If so, why
doesn't the documentation say so?

Comment #3: What about NEWS and the manual?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 09:42:01 GMT) Full text and rfc822 format available.

Message #82 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 11:41:10 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Anybody else got any comments on this?
>
> Comment #1: Do we really want to turn this feature on by default?

I don't see why not.  It's just a more detailed visualisation.

>> +(defface isearch-2
>> +  '((((class color) (min-colors 88) (background light))
>> +     :background "magenta1" :foreground "lightskyblue1")
>> +    (((class color) (min-colors 88) (background dark))
>> +     :background "palevioletred4" :foreground "brown4"))
>> +  "Used for displaying the second matching subexpression.")
>
> Comment #2: This seems to effectively disable the feature on displays
> that have fewer than 88 colors.  Is that intentional?  If so, why
> doesn't the documentation say so?

I'm guessing it's just cargo-culting off of the isearch face:

(defface isearch
  '((((class color) (min-colors 88) (background light))

> Comment #3: What about NEWS and the manual?

Those would have to be added, of course.

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 09:55:02 GMT) Full text and rfc822 format available.

Message #85 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 12:53:57 +0300
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: juri <at> jurta.org,  lennart.borgman <at> gmail.com,  6227 <at> debbugs.gnu.org
> Date: Sun, 20 Sep 2020 11:41:10 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> Anybody else got any comments on this?
> >
> > Comment #1: Do we really want to turn this feature on by default?
> 
> I don't see why not.  It's just a more detailed visualisation.

But with slightly different colors.  How do we know long-time users of
Isearch will understand what they mean, unless they deliberately turn
on this option?

> > Comment #2: This seems to effectively disable the feature on displays
> > that have fewer than 88 colors.  Is that intentional?  If so, why
> > doesn't the documentation say so?
> 
> I'm guessing it's just cargo-culting off of the isearch face:
> 
> (defface isearch
>   '((((class color) (min-colors 88) (background light))

I don't understand: the 'isearch' face has definitions for all kinds
of displays, even for monochrome ones:

  (defface isearch
    '((((class color) (min-colors 88) (background light))
       ;; The background must not be too dark, for that means
       ;; the character is hard to see when the cursor is there.
       (:background "magenta3" :foreground "lightskyblue1"))
      (((class color) (min-colors 88) (background dark))
       (:background "palevioletred2" :foreground "brown4"))
      (((class color) (min-colors 16))
       (:background "magenta4" :foreground "cyan1"))
      (((class color) (min-colors 8))
       (:background "magenta4" :foreground "cyan1"))
      (t (:inverse-video t)))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 10:05:01 GMT) Full text and rfc822 format available.

Message #88 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 12:04:34 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

>> I don't see why not.  It's just a more detailed visualisation.
>
> But with slightly different colors.  How do we know long-time users of
> Isearch will understand what they mean, unless they deliberately turn
> on this option?

When they're typing "foo\(bar\)" in regexp isearch and see the different
colours, I think they'll catch on pretty quickly.

>> > Comment #2: This seems to effectively disable the feature on displays
>> > that have fewer than 88 colors.  Is that intentional?  If so, why
>> > doesn't the documentation say so?
>> 
>> I'm guessing it's just cargo-culting off of the isearch face:
>> 
>> (defface isearch
>>   '((((class color) (min-colors 88) (background light))
>
> I don't understand: the 'isearch' face has definitions for all kinds
> of displays, even for monochrome ones:
>
>   (defface isearch
>     '((((class color) (min-colors 88) (background light))
>        ;; The background must not be too dark, for that means
>        ;; the character is hard to see when the cursor is there.
>        (:background "magenta3" :foreground "lightskyblue1"))

I meant that these deffaces aren't complete -- they look like examples,
and the examples are taken from the isearch face.

There should be more of them, and they should be complete, and they
should have better names (isearch-group-1 etc perhaps).

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 13:48:01 GMT) Full text and rfc822 format available.

Message #91 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 15:47:13 +0200
Now pushed.  The faces should be legible both on dark and light
backgrounds, but they aren't very...  pretty.  Feel free to tweak them.  :-)

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





Added tag(s) fixed. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Sun, 20 Sep 2020 13:48:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.1, send any further explanations to 6227 <at> debbugs.gnu.org and Lennart Borgman <lennart.borgman <at> gmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Sun, 20 Sep 2020 13:48:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 14:19:02 GMT) Full text and rfc822 format available.

Message #98 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 17:18:40 +0300
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: juri <at> jurta.org,  lennart.borgman <at> gmail.com,  6227 <at> debbugs.gnu.org
> Date: Sun, 20 Sep 2020 15:47:13 +0200
> 
> Now pushed.  The faces should be legible both on dark and light
> backgrounds, but they aren't very...  pretty.  Feel free to tweak them.  :-)

What bothers me is that faces isearch-group-6 to isearch-group-9 are
not defined.  Should we define them?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 16:43:02 GMT) Full text and rfc822 format available.

Message #101 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 09:41:59 -0700 (PDT)
> >   (defface isearch
> >     '((((class color) (min-colors 88) (background light))
> >        ;; The background must not be too dark, for that means
> >        ;; the character is hard to see when the cursor is there.
> >        (:background "magenta3" :foreground "lightskyblue1"))
> 
> I meant that these deffaces aren't complete -- they look like examples,
> and the examples are taken from the isearch face.
> 
> There should be more of them, and they should be complete, and they
> should have better names (isearch-group-1 etc perhaps).

FWIW, I use simple definitions like this one for the
eight regexp-level faces, not bothering with multiple
kinds of displays.  (Not saying that's what isearch.el
should do.)

(defface isearchp-regexp-level-8 
  '((((background dark)) (:background "#12EC00003F0E")) ; a very dark blue
    (t (:background "#F6F5FFFFE1E1")))  ; a light yellow
  "Face used to highlight subgroup level 8 of your search context.
This highlighting is done during regexp searching whenever
`isearchp-highlight-regexp-group-levels-flag' is non-nil."
  :group 'isearch-plus :group 'faces)





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 16:46:01 GMT) Full text and rfc822 format available.

Message #104 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: larsi <at> gnus.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 19:45:43 +0300
> Date: Sun, 20 Sep 2020 09:41:59 -0700 (PDT)
> From: Drew Adams <drew.adams <at> oracle.com>
> Cc: lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
> 
> > >   (defface isearch
> > >     '((((class color) (min-colors 88) (background light))
> > >        ;; The background must not be too dark, for that means
> > >        ;; the character is hard to see when the cursor is there.
> > >        (:background "magenta3" :foreground "lightskyblue1"))
> > 
> > I meant that these deffaces aren't complete -- they look like examples,
> > and the examples are taken from the isearch face.
> > 
> > There should be more of them, and they should be complete, and they
> > should have better names (isearch-group-1 etc perhaps).
> 
> FWIW, I use simple definitions like this one for the
> eight regexp-level faces, not bothering with multiple
> kinds of displays.  (Not saying that's what isearch.el
> should do.)
> 
> (defface isearchp-regexp-level-8 
>   '((((background dark)) (:background "#12EC00003F0E")) ; a very dark blue
>     (t (:background "#F6F5FFFFE1E1")))  ; a light yellow
>   "Face used to highlight subgroup level 8 of your search context.
> This highlighting is done during regexp searching whenever
> `isearchp-highlight-regexp-group-levels-flag' is non-nil."
>   :group 'isearch-plus :group 'faces)

That's not the same thing at all: your face definition will work with
any terminal (through the automatic color translation).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 19:46:02 GMT) Full text and rfc822 format available.

Message #107 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 21:45:38 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi <at> gnus.org>
>> Cc: juri <at> jurta.org,  lennart.borgman <at> gmail.com,  6227 <at> debbugs.gnu.org
>> Date: Sun, 20 Sep 2020 15:47:13 +0200
>> 
>> Now pushed.  The faces should be legible both on dark and light
>> backgrounds, but they aren't very...  pretty.  Feel free to tweak them.  :-)
>
> What bothers me is that faces isearch-group-6 to isearch-group-9 are
> not defined.  Should we define them?

I'd rather go the other way, to be honest -- just leave group-1 to -3,
perhaps.  I think it's rather unusual to have a that many sub-groups
interactively...

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Sun, 20 Sep 2020 20:26:01 GMT) Full text and rfc822 format available.

Message #110 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 13:25:25 -0700 (PDT)
> > What bothers me is that faces isearch-group-6 to isearch-group-9 are
> > not defined.  Should we define them?
> 
> I'd rather go the other way, to be honest -- just leave group-1 to -3,
> perhaps.  I think it's rather unusual to have a that many sub-groups
> interactively...

It is unusual, as in not so common.  But when you do
have a complex regexp with many groups, that's exactly
when such highlighting can be most useful.

I settled on 8 levels, for Isearch+, but that was
without any special research into the question.

It doesn't hurt, in any way that I'm aware of, to
have more rather than less.  In the case where many
would actually be used (which, as you say, is not
usual), a little more time would be needed, but it's
likely that in such a complex case there would not
be many matches to highlight.  I think any time cost
of such highlighting is negligible.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Mon, 21 Sep 2020 02:26:02 GMT) Full text and rfc822 format available.

Message #113 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Mon, 21 Sep 2020 05:25:09 +0300
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: juri <at> jurta.org,  lennart.borgman <at> gmail.com,  6227 <at> debbugs.gnu.org
> Date: Sun, 20 Sep 2020 21:45:38 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> >> Cc: juri <at> jurta.org,  lennart.borgman <at> gmail.com,  6227 <at> debbugs.gnu.org
> >> Date: Sun, 20 Sep 2020 15:47:13 +0200
> >> 
> >> Now pushed.  The faces should be legible both on dark and light
> >> backgrounds, but they aren't very...  pretty.  Feel free to tweak them.  :-)
> >
> > What bothers me is that faces isearch-group-6 to isearch-group-9 are
> > not defined.  Should we define them?
> 
> I'd rather go the other way, to be honest -- just leave group-1 to -3,
> perhaps.  I think it's rather unusual to have a that many sub-groups
> interactively...

That's true, but the problem which bothers me is that customizing
search-highlight-submatches alone to a larger value is not enough.  I
don't think any other customization we have requires users to create
faces or similar objects.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Mon, 21 Sep 2020 04:50:01 GMT) Full text and rfc822 format available.

Message #116 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: RE: bug#6227: Color isearch regexp submatches differently
Date: Sun, 20 Sep 2020 21:49:26 -0700 (PDT)
> > I'd rather go the other way, to be honest -- just leave group-1 to -3,
> > perhaps.  I think it's rather unusual to have a that many sub-groups
> > interactively...
> 
> That's true, but the problem which bothers me is that customizing
> search-highlight-submatches alone to a larger value is not enough.  I
> don't think any other customization we have requires users to create
> faces or similar objects.

FWIW, the option in Isearch+ is just a boolean, not
a max number of levels/groups.  I use it for both
Isearch and `replace-highlight'.  The use cases are
only interactive, AFAICimagine, and I don't imagine
(and haven't seen) any need for lots of levels.  I
use 8 levels, and that's already a lot.

As for the relation between your max # groups and
faces: if you really want to let users change the
max # (not needed, I think), you can just have a
fixed number of faces and recycle them.  You don't
have to imagine that users will need to both (1)
increase `search-highlight-submatches' and (2)
create corresponding new faces for the additional
groups to be matched.  It's pretty clear which
level is involved if the same color is used for,
say, level 2 and level 10.

I mentioned this recycling possibility earlier,
saying that I considered going through the 1-8
faces I have and then going through them again, for
groups 9-18, and again,...  But I've never seen any
need for that.  Your design sounds like overkill, if
the intention is just interactive use.

Just a suggestion.

Also, your option name should have "max" in it, I
think.  And if you use it only for Isearch, then
maybe use "isearch", not "search" in the name.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#6227; Package emacs. (Mon, 21 Sep 2020 13:49:01 GMT) Full text and rfc822 format available.

Message #119 received at 6227 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> jurta.org, lennart.borgman <at> gmail.com, 6227 <at> debbugs.gnu.org
Subject: Re: bug#6227: Color isearch regexp submatches differently
Date: Mon, 21 Sep 2020 15:48:00 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

> That's true, but the problem which bothers me is that customizing
> search-highlight-submatches alone to a larger value is not enough.  I
> don't think any other customization we have requires users to create
> faces or similar objects.

That's true...  OK, I'll just add the remaining four faces, and then
make the variable a boolean toggle, because (as Drew notes) nobody is
going to want to customise the number of sub-expressions, really.

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




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 20 Oct 2020 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 182 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.