bug#7604: `loop', PATCH: loop, cl-parse-loop-clause, cl-map-intervals -- being the intervals of

bug#7604: `loop', PATCH: loop, cl-parse-loop-clause, cl-map-intervals -- being the intervals of

[MON KEY]

Current through BZR-102620 there is a bug around `loop',
`cl-parse-loop-clause', and `cl-map-intervals' which put them in
conflict with the manual (which is unclear w/re intended usage):

 ,----
 | `for VAR being the intervals [of BUFFER] ...'
 |  This clause iterates over all intervals of a buffer with constant
 |  text properties.  The variable VAR will be bound to conses of
 |  start and end positions, where one start position is always equal
 |  to the previous end position.  The clause allows `of', `from',
 |  `to', and `property' terms, where the latter term restricts the
 |  search to just the specified property.  The `of' term may specify
 |  either a buffer or a string.
 `----

(loop for <VAR> being the intervals of property <PROP> from <INT> to <INT>
      collecting <VAR>)

The above works (by accident).

What should work (but doesn't) is:

(loop for <VAR> being the intervals in <BUFFER> of property <PROP>
from <INT> to <INT>
      collecting <VAR>)

The `intervals' feature is an extension to the Common Lisp `loop'
macro but is not in keeping with the syntax.

My understanding of the above is following should work (it doesn't):

(loop for <VAR> being the intervals of <BUFFER> of property <PROP>
from <INT> to <INT>
      collecting tp)

Additionally, the source provides that `in' may be used in the same
manner as `of':

(loop for <VAR> being the intervals in <BUFFER> property <PROP> from
<INT> to <INT>
      collecting tp)

So, I would also expect either of these to work as well (they don't):

(loop for <VAR> being the intervals in <BUFFER> of property <PROP>
from <INT> to <INT>
      collecting tp)

(loop for <VAR> being the intervals of <BUFFER> in property <PROP>
from <INT> to <INT>
      collecting tp)

There should be an additional infix filler word preceding `property'
as their is with the hash-table loop clauses, and he right fix IMO
would be to add a `using' syntax instead of and/or in addition to
testing for `property' which is pretty clearly (of itself) NTRT.
Such an addtion would allow for:

(loop for <VAR> being the intervals in <BUFFER> using property <PROP>
from <INT> to <INT>
      collecting tp)

Regardless, the ttached patch is current w/ BZR-102620. It fixes the
problem and remains backwards compatable with the existing behaviour
in Emacs 23.

rgrep says nothing in core uses this loop idiom so it should be a
fairly clean fix.

--- /ediff25130lIS	2010-12-09 20:06:14.727993830 -0500
+++ /lisp/emacs-lisp/cl-macs.el	2010-12-09 19:54:51.992860000 -0500
@@ -916,17 +916,23 @@
 			  (lambda (,var ,(make-symbol "--cl-var--"))
 			    (progn . --cl-map) nil)
 			  ,buf ,from ,to))))
-
+
 	       ((memq word '(interval intervals))
 		(let ((buf nil) (prop nil) (from nil) (to nil)
 		      (var1 (make-symbol "--cl-var1--"))
 		      (var2 (make-symbol "--cl-var2--")))
-		  (while (memq (car loop-args) '(in of property from to))
-		    (cond ((eq (car loop-args) 'from) (setq from (cl-pop2 loop-args)))
-			  ((eq (car loop-args) 'to) (setq to (cl-pop2 loop-args)))
-			  ((eq (car loop-args) 'property)
-			   (setq prop (cl-pop2 loop-args)))
-			  (t (setq buf (cl-pop2 loop-args)))))
+                  (while (memq (car loop-args) '(in of property from to))
+                    (cond ((eq (car loop-args) 'from) (setq from
(cl-pop2 loop-args)))
+                          ((eq (car loop-args) 'to) (setq to (cl-pop2
loop-args)))
+                          ((eq (car loop-args) 'property) (setq prop
(cl-pop2 loop-args)))
+                          ((memq (car loop-args) '(in of))
+                           (or (and (eq (cadr loop-args) 'property)
+                                    (pop loop-args)
+                                    (setq prop (cl-pop2 loop-args)))
+                               (setq buf (cl-pop2 loop-args))))
+                          (t (or (and (or (stringp (car loop-args))
(bufferp (car loop-args)))
+                                      (setq buf (car loop-args)))
+                                 (error "Expected buffer or string")))))
 		  (if (and (consp var) (symbolp (car var)) (symbolp (cdr var)))
 		      (setq var1 (car var) var2 (cdr var))
 		    (push (list var (list 'cons var1 var2)) loop-for-sets))

--
/s_P\

bug#7604: `loop', PATCH: loop, cl-parse-loop-clause, cl-map-intervals -- being the intervals of

[Lars Ingebrigtsen]

MON KEY <monkey@sandpframing.com> writes:

> Current through BZR-102620 there is a bug around `loop',
> `cl-parse-loop-clause', and `cl-map-intervals' which put them in
> conflict with the manual (which is unclear w/re intended usage):
>
>  ,----
>  | `for VAR being the intervals [of BUFFER] ...'
>  |  This clause iterates over all intervals of a buffer with constant
>  |  text properties.  The variable VAR will be bound to conses of
>  |  start and end positions, where one start position is always equal
>  |  to the previous end position.  The clause allows `of', `from',
>  |  `to', and `property' terms, where the latter term restricts the
>  |  search to just the specified property.  The `of' term may specify
>  |  either a buffer or a string.
>  `----

[...]

> There should be an additional infix filler word preceding `property'
> as their is with the hash-table loop clauses, and he right fix IMO
> would be to add a `using' syntax instead of and/or in addition to
> testing for `property' which is pretty clearly (of itself) NTRT.
> Such an addtion would allow for:
>
> (loop for <VAR> being the intervals in <BUFFER> using property <PROP>
> from <INT> to <INT>
>       collecting tp)
>
> Regardless, the ttached patch is current w/ BZR-102620. It fixes the
> problem and remains backwards compatable with the existing behaviour
> in Emacs 23.
>
> rgrep says nothing in core uses this loop idiom so it should be a
> fairly clean fix.

Since nothing uses it, and it's a non-standard Emacs extension to loop,
perhaps we should just remove the intervals stuff from cl-loop?

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

bug#7604: `loop', PATCH: loop, cl-parse-loop-clause, cl-map-intervals -- being the intervals of

[Lars Ingebrigtsen]

MON KEY <monkey@sandpframing.com> writes:

> (loop for <VAR> being the intervals in <BUFFER> property <PROP> from
> <INT> to <INT>
>       collecting tp)
>
> So, I would also expect either of these to work as well (they don't):
>
> (loop for <VAR> being the intervals in <BUFFER> of property <PROP>
> from <INT> to <INT>
>       collecting tp)
>
> (loop for <VAR> being the intervals of <BUFFER> in property <PROP>
> from <INT> to <INT>
>       collecting tp)
>
> There should be an additional infix filler word preceding `property'
> as their is with the hash-table loop clauses, and he right fix IMO
> would be to add a `using' syntax instead of and/or in addition to
> testing for `property' which is pretty clearly (of itself) NTRT.

Well, the manual says (if I understand it correctly) that "property"
itself is the keyword here, so in/of isn't necessarily valid, even if it
makes sense.

But nothing uses this stuff (fortunately), so I don't think there's any
point in extending it, so I'm closing this bug report.

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