re-builder highlighting incorrect for more than 3 groupings

re-builder highlighting incorrect for more than 3 groupings

[Christian Plate]

Hello,

I encountered strange behavior with re-builder:

I made a testcase to show it. Consider the following buffer:

t-t-t-t
t-t-t-t
t-t-t-t
t-t-t-t

re-builder with the regex "\\(.\\)-\\(.\\)-\\(.\\)" correctly highlights
all first groupings. But if i make another group,
"\\(.\\)-\\(.\\)-\\(.\\)-\\(.\\)", it only highlights the first
match. Perhaps this has something to do with missing faces for more than
three groupings.

 Christian Plate

Re: re-builder highlighting incorrect for more than 3 groupings

[martin rudalics]

 > I encountered strange behavior with re-builder:
 >
 > I made a testcase to show it. Consider the following buffer:
 >
 > t-t-t-t
 > t-t-t-t
 > t-t-t-t
 > t-t-t-t
 >
 > re-builder with the regex "\\(.\\)-\\(.\\)-\\(.\\)" correctly highlights
 > all first groupings. But if i make another group,
 > "\\(.\\)-\\(.\\)-\\(.\\)-\\(.\\)", it only highlights the first
 > match. Perhaps this has something to do with missing faces for more than
 > three groupings.

In fact you should have noted the message:

reb-update-overlays: Too many subexpressions - face `reb-match-4' not defined

You can add as many deffaces

(defface reb-match-4 ...

(defface reb-match-5 ...

...

you want to your .emacs.

Re: re-builder highlighting incorrect for more than 3 groupings

[Richard Stallman]

    You can add as many deffaces

    (defface reb-match-4 ...

    (defface reb-match-5 ...

Can we do something so that it cycles thru the provided faces
once it has used them all?

Re: re-builder highlighting incorrect for more than 3 groupings

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

>     You can add as many deffaces
> 
>     (defface reb-match-4 ...
> 
>     (defface reb-match-5 ...
> 
> Can we do something so that it cycles thru the provided faces
> once it has used them all?
> 
> 

The attached patch tries to do that.


[-- Attachment #2: re-builder.patch --]
[-- Type: text/plain, Size: 3312 bytes --]

2006-04-27  Martin Rudalics  <rudalics@gmx.at>

	* emacs-lisp/re-builder.el (reb-update-overlays):
	Cycle thru provided faces once they have been used all.

*** re-builder.el	Sat Nov  5 19:44:46 2005
--- re-builder.el	Thu Apr 27 10:35:20 2006
***************
*** 112,118 ****
  (if (not (fboundp 'make-overlay))
      (require 'overlay))

! ;; User costomizable variables
  (defgroup re-builder nil
    "Options for the RE Builder."
    :group 'lisp
--- 112,118 ----
  (if (not (fboundp 'make-overlay))
      (require 'overlay))

! ;; User customizable variables
  (defgroup re-builder nil
    "Options for the RE Builder."
    :group 'lisp
***************
*** 627,637 ****
  	    beg (match-end 0)))
      i))

- 
  (defun reb-update-overlays (&optional subexp)
    "Switch to `reb-target-buffer' and mark all matches of `reb-regexp'.
  If SUBEXP is non-nil mark only the corresponding sub-expressions."
- 
    (let* ((re (reb-target-binding reb-regexp))
  	 (subexps (reb-count-subexps re))
  	 (matches 0)
--- 627,635 ----
***************
*** 645,668 ****
  		  (or (not reb-auto-match-limit)
  		      (< matches reb-auto-match-limit)))
  	(if (= 0 (length (match-string 0)))
! 	  (error "Empty regular expression!"))
! 	(let ((i 0))
  	  (setq matches (1+ matches))
  	  (while (<= i subexps)
  	    (if (and (or (not subexp) (= subexp i))
  		     (match-beginning i))
  		(let ((overlay (make-overlay (match-beginning i)
  					     (match-end i)))
! 		      (face-name (format "reb-match-%d" i)))
! 		  (if (not firstmatch)
! 		      (setq firstmatch (match-data)))
  		  (setq reb-overlays (cons overlay reb-overlays)
  			submatches (1+ submatches))
! 		  (overlay-put
! 		   overlay 'face
! 		   (or (intern-soft face-name)
! 		       (error "Too many subexpressions - face `%s' not defined"
! 			      face-name )))
  		  (overlay-put overlay 'priority i)))
  	    (setq i (1+ i))))))
      (let ((count (if subexp submatches matches)))
--- 643,677 ----
  		  (or (not reb-auto-match-limit)
  		      (< matches reb-auto-match-limit)))
  	(if (= 0 (length (match-string 0)))
! 	    (error "Empty regular expression!"))
! 	(let ((i 0)
! 	      suffix max-suffix)
  	  (setq matches (1+ matches))
  	  (while (<= i subexps)
  	    (if (and (or (not subexp) (= subexp i))
  		     (match-beginning i))
  		(let ((overlay (make-overlay (match-beginning i)
  					     (match-end i)))
! 		      ;; When we have exceeded the number of provided faces,
! 		      ;; cycle thru them where `max-suffix' denotes the maximum
! 		      ;; suffix for `reb-match-*' that has been defined and
! 		      ;; `suffix' the suffix calculated for the current match.
! 		      (face
! 		       (cond
! 			(max-suffix
! 			 (if (= suffix max-suffix)
! 			     (setq suffix 1)
! 			   (setq suffix (1+ suffix)))
! 			 (intern-soft (format "reb-match-%d" suffix)))
! 			((intern-soft (format "reb-match-%d" i)))
! 			((setq max-suffix (1- i))
! 			 (setq suffix 1)
! 			 ;; `reb-match-1' must exist.
! 			 'reb-match-1))))
! 		  (unless firstmatch (setq firstmatch (match-data)))
  		  (setq reb-overlays (cons overlay reb-overlays)
  			submatches (1+ submatches))
! 		  (overlay-put overlay 'face face)
  		  (overlay-put overlay 'priority i)))
  	    (setq i (1+ i))))))
      (let ((count (if subexp submatches matches)))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 4/27/06, Richard Stallman <rms@gnu.org> wrote:

>     (defface reb-match-4 ...
>
>     (defface reb-match-5 ...

> Can we do something so that it cycles thru the provided faces
> once it has used them all?

Wouldn't be easier just to define reb-match-[4-9]? Faces aren't
exactly a scarce resource, and nine is the maximum re-builder is going
to use anyway.

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[Adrian Aichner]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 4/27/06, Richard Stallman <rms@gnu.org> wrote:
>
>>     (defface reb-match-4 ...
>>
>>     (defface reb-match-5 ...
>
>> Can we do something so that it cycles thru the provided faces
>> once it has used them all?
>
> Wouldn't be easier just to define reb-match-[4-9]? Faces aren't
> exactly a scarce resource, and nine is the maximum re-builder is going
> to use anyway.

That's what I have done for the XEmacs re-builder package.

-- 
Adrian Aichner
 mailto:adrian@xemacs.org
 http://www.xemacs.org/

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 4/29/06, Adrian Aichner <adrian@xemacs.org> wrote:

> That's what I have done for the XEmacs re-builder package.

I did it on my .emacs.el a long time ago. I use re-builder quite a
lot, and seems a bit silly to limit yourself to only three
subexpressions.

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[martin rudalics]

 > Wouldn't be easier just to define reb-match-[4-9]? Faces aren't
 > exactly a scarce resource, and nine is the maximum re-builder is going
 > to use anyway.

The maximum of nine expressions applies only in "sub-expression mode".
In "normal" mode you would have to settle on an appropriate strategy
anyway when there are more than nine subexpressions.  On the other hand,
in sub-expression mode one face is all you need - you could use say

(face-name (format "reb-match-%d" (if subexp 1 i)))

in `reb-update-overlays'.

Re: re-builder highlighting incorrect for more than 3 groupings

[Richard Stallman]

    > Wouldn't be easier just to define reb-match-[4-9]? Faces aren't
    > exactly a scarce resource, and nine is the maximum re-builder is going
    > to use anyway.

Why does it use no more than 9?  Is that a natural limit
or an arbitrary limit?

Re: re-builder highlighting incorrect for more than 3 groupings

[Adrian Aichner]

Richard Stallman <rms@gnu.org> writes:

>     > Wouldn't be easier just to define reb-match-[4-9]? Faces aren't
>     > exactly a scarce resource, and nine is the maximum re-builder is going
>     > to use anyway.
>
> Why does it use no more than 9?  Is that a natural limit
> or an arbitrary limit?

I think it's an arbitrary limitation, aside from the fact that
reb-subexp-mode uses single digit key mappings to highlight individual
sub-expressions.

-- 
Adrian Aichner
 mailto:adrian@xemacs.org
 http://www.xemacs.org/

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 4/29/06, Richard Stallman <rms@gnu.org> wrote:

> Why does it use no more than 9?  Is that a natural limit
> or an arbitrary limit?

In re-builder's subexpression mode, the user can highlight the text
matched by each one of the first nine subexpressions by typing 1-9, or
0 to highlight the full matched string.

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 4/29/06, martin rudalics <rudalics@gmx.at> wrote:

> In "normal" mode you would have to settle on an appropriate strategy
> anyway when there are more than nine subexpressions.

Yes. By definition any limit is arbitrary. However, in my limited
experience, three subexpressions is way too little. It doesn't take a
very complex regexp to go over three subexps.

> On the other hand,
> in sub-expression mode one face is all you need - you could use say
>
> (face-name (format "reb-match-%d" (if subexp 1 i)))
>
> in `reb-update-overlays'.

I like using different faces in subexpression mode...

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[Richard Stallman]

    The maximum of nine expressions applies only in "sub-expression mode".
    In "normal" mode you would have to settle on an appropriate strategy
    anyway when there are more than nine subexpressions.

Ok, would people please install the face-cycling patch?

Re: re-builder highlighting incorrect for more than 3 groupings

[Kevin Rodgers]

Richard Stallman wrote:
>     The maximum of nine expressions applies only in "sub-expression mode".
>     In "normal" mode you would have to settle on an appropriate strategy
>     anyway when there are more than nine subexpressions.
> 
> Ok, would people please install the face-cycling patch?

And define reb-match-[4-9] as well, to avoid cycling when there are less
than 10 subexpressions.

-- 
Kevin Rodgers

Re: re-builder highlighting incorrect for more than 3 groupings

[Richard Stallman]

    > Ok, would people please install the face-cycling patch?

    And define reb-match-[4-9] as well, to avoid cycling when there are less
    than 10 subexpressions.

I don't see why we need them.  What is wrong with cycling in that case?

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 5/2/06, Richard Stallman <rms@gnu.org> wrote:

> I don't see why we need them.  What is wrong with cycling in that case?

Nothing, probably. But, what *is* wrong with adding the faces? (It'd
be useful to know wheter frequent users of re-builder are defining
them in their .emacs; certainly it's what I would expect.)

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 5/2/06, Richard Stallman <rms@gnu.org> wrote:
>
>> I don't see why we need them.  What is wrong with cycling in that case?
>
> Nothing, probably. But, what *is* wrong with adding the faces?

A complete waste of code for no apparent good whatsoever.

> (It'd be useful to know wheter frequent users of re-builder are
> defining them in their .emacs; certainly it's what I would expect.)

So what?  It is not like the cycling code would keep them from doing
that if they want to.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 5/2/06, David Kastrup <dak@gnu.org> wrote:

> A complete waste of code for no apparent good whatsoever.

Then remove the definitions for faces reb-match-[1-3]. reb-match-0
should suffice. At the very least, reb-match-[2,3] are unnecessary,
from a zen-minimalistic POV.

Not to mention that "no apparent good whatsoever" is highly
subjective, isn't it? I have that very same feeling every time I look
at customize...

> So what?  It is not like the cycling code would keep them from doing
> that if they want to.

No. But I think we could agree that there are at least three criteria
(perhaps more) to define something as a good default for Emacs:

 - Defaulting to that something makes Emacs safer out-of-the-box
 - Defaulting to that something is easier to understand for a newbie
  - If not a default, most people interested in the feature will set
or define that something in their .emacs

I was hypothesizing about the third condition.

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 5/2/06, David Kastrup <dak@gnu.org> wrote:
>
>> A complete waste of code for no apparent good whatsoever.
>
> Then remove the definitions for faces reb-match-[1-3]. reb-match-0
> should suffice. At the very least, reb-match-[2,3] are unnecessary,
> from a zen-minimalistic POV.

The cycling patch was not reducing the number of distinct colors:
there never was a proposal to add more _distinct_ colors to the
standard scheme.

That's what "no apparent good whatsoever" is about: just defining more
faces in a cyclic fashion does not change the appearance in any way.

> Not to mention that "no apparent good whatsoever" is highly
> subjective, isn't it?

No, it isn't.  If the colors are the same anyway, there is no point in
attaining that effect in a more verbose manner.

>> So what?  It is not like the cycling code would keep them from doing
>> that if they want to.
>
> No. But I think we could agree that there are at least three criteria
> (perhaps more) to define something as a good default for Emacs:
>
> - Defaulting to that something makes Emacs safer out-of-the-box
> - Defaulting to that something is easier to understand for a newbie
>  - If not a default, most people interested in the feature will set
> or define that something in their .emacs
>
> I was hypothesizing about the third condition.

So what?  You have not given _any_ kind of rationale where this third
condition would in any way be affected.  Users can define more colors
if they really want to.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: re-builder highlighting incorrect for more than 3 groupings

[Juanma Barranquero]

On 5/2/06, David Kastrup <dak@gnu.org> wrote:

> The cycling patch was not reducing the number of distinct colors:
> there never was a proposal to add more _distinct_ colors to the
> standard scheme.
>
> That's what "no apparent good whatsoever" is about: just defining more
> faces in a cyclic fashion does not change the appearance in any way.

And do you honestly think I was proposing to add more faces and not
make them distinct from the ones already existing?

> If the colors are the same anyway, there is no point in
> attaining that effect in a more verbose manner.

Sure. But to my eyes, it seems like you're fighting a strawman.

> So what?  You have not given _any_ kind of rationale where this third
> condition would in any way be affected.

No. And I've not claimed to. I was just wondering. And from my wording
I think that was quite clear. Perhaps just not to you. I think that
tends to happen a lot every time you and I discuss something. Must be
some kind of failure to communicate on my part.

> Users can define more colors
> if they really want to.

Thanks for pointing that out, I would've never thought of it.

--
                    /L/e/k/t/u

Re: re-builder highlighting incorrect for more than 3 groupings

[Richard Stallman]

    Nothing, probably. But, what *is* wrong with adding the faces? (It'd

It's only useful if they would be different (not the same as cycling).

We want the cycling code in either case--has someone installed it yet?
If not, please do.