all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#34100: 26.1; Incomplete ? and ?? handling in rx
@ 2019-01-16 11:39 Mattias Engdegård
  2019-01-16 14:06 ` bug#34100: rx \? and \??: patch Mattias Engdegård
       [not found] ` <handler.34100.B.154763885119611.ack@debbugs.gnu.org>
  0 siblings, 2 replies; 5+ messages in thread
From: Mattias Engdegård @ 2019-01-16 11:39 UTC (permalink / raw)
  To: 34100

In rx, the ? and ?? operators can be written verbatim as ? and ?? (space and ? character), or by using symbols whose leading character needs to be escaped, \? and \?? respectively. The names come from Olin Shivers's SRE, but ? is not a special character in Scheme syntax, hence the character syntax hack.

However, the symbols only partially work:

(rx (\? "x") (\?? "y"))       --> "x?y?"  ; expected "x?y??"
(rx (minimal-match (\? "x"))) --> "x??"   ; expected "x?"

While it could be argued that only the character-based syntax should be used, the fact is that the symbols are accepted and seem to work, just in a subtly broken way.

The documentation is also not clear on this point, and a programmer knowing the elisp syntax might very well assume that the symbols are the ones to use.

Suggested fix:

diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index a39fe55c32..8b4551d0d3 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -733,8 +733,8 @@
 is non-nil."
   (rx-check form)
   (setq form (rx-trans-forms form))
-  (let ((suffix (cond ((memq (car form) '(* + ?\s)) "")
-		      ((memq (car form) '(*? +? ??)) "?")
+  (let ((suffix (cond ((memq (car form) '(* + \? ?\s)) "")
+		      ((memq (car form) '(*? +? \?? ??)) "?")
 		      (rx-greedy-flag "")
 		      (t "?")))
 	(op (cond ((memq (car form) '(* *? 0+ zero-or-more)) "*")







^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#34100: rx \? and \??: patch
  2019-01-16 11:39 bug#34100: 26.1; Incomplete ? and ?? handling in rx Mattias Engdegård
@ 2019-01-16 14:06 ` Mattias Engdegård
       [not found] ` <handler.34100.B.154763885119611.ack@debbugs.gnu.org>
  1 sibling, 0 replies; 5+ messages in thread
From: Mattias Engdegård @ 2019-01-16 14:06 UTC (permalink / raw)
  To: 34100

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

Here is a patch with a test.

[-- Attachment #2: 0001-Make-the-rx-operators-and-behave-correctly.patch --]
[-- Type: application/octet-stream, Size: 2741 bytes --]

From b7ffa6c7220032ee1ea4d8773f58a37a647ecc22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Wed, 16 Jan 2019 14:57:12 +0100
Subject: [PATCH] Make the rx operators \? and \?? behave correctly

* lisp/emacs-lisp/rx.el (rx-kleene):
Treat \? and \?? like ? and ?? (Bug#34100).
* test/lisp/emacs-lisp/rx-tests.el: Add tests for all repetition operators.
---
 lisp/emacs-lisp/rx.el            |  4 ++--
 test/lisp/emacs-lisp/rx-tests.el | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index a39fe55c32..8b4551d0d3 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -733,8 +733,8 @@ If OP is anything else, produce a greedy regexp if `rx-greedy-flag'
 is non-nil."
   (rx-check form)
   (setq form (rx-trans-forms form))
-  (let ((suffix (cond ((memq (car form) '(* + ?\s)) "")
-		      ((memq (car form) '(*? +? ??)) "?")
+  (let ((suffix (cond ((memq (car form) '(* + \? ?\s)) "")
+		      ((memq (car form) '(*? +? \?? ??)) "?")
 		      (rx-greedy-flag "")
 		      (t "?")))
 	(op (cond ((memq (car form) '(* *? 0+ zero-or-more)) "*")
diff --git a/test/lisp/emacs-lisp/rx-tests.el b/test/lisp/emacs-lisp/rx-tests.el
index 392a38ab95..f15e1016f7 100644
--- a/test/lisp/emacs-lisp/rx-tests.el
+++ b/test/lisp/emacs-lisp/rx-tests.el
@@ -65,5 +65,28 @@
                     (list u v)))
                  '("1" "3"))))
 
+(ert-deftest rx-kleene ()
+  "Test greedy and non-greedy repetition operators."
+  (should (equal (rx (* "a") (+ "b") (\? "c") (?\s "d")
+                     (*? "e") (+? "f") (\?? "g") (?? "h"))
+                 "a*b+c?d?e*?f+?g??h??"))
+  (should (equal (rx (zero-or-more "a") (0+ "b")
+                     (one-or-more "c") (1+ "d")
+                     (zero-or-one "e") (optional "f") (opt "g"))
+                 "a*b*c+d+e?f?g?"))
+  (should (equal (rx (minimal-match
+                      (seq (* "a") (+ "b") (\? "c") (?\s "d")
+                           (*? "e") (+? "f") (\?? "g") (?? "h"))))
+                 "a*b+c?d?e*?f+?g??h??"))
+  (should (equal (rx (minimal-match
+                      (seq (zero-or-more "a") (0+ "b")
+                           (one-or-more "c") (1+ "d")
+                           (zero-or-one "e") (optional "f") (opt "g"))))
+                 "a*?b*?c+?d+?e??f??g??"))
+  (should (equal (rx (maximal-match
+                      (seq (* "a") (+ "b") (\? "c") (?\s "d")
+                         (*? "e") (+? "f") (\?? "g") (?? "h"))))
+                 "a*b+c?d?e*?f+?g??h??")))
+
 (provide 'rx-tests)
 ;; rx-tests.el ends here.
-- 
2.17.2 (Apple Git-113)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#34100: Acknowledgement (26.1; Incomplete ? and ?? handling in rx)
       [not found] ` <handler.34100.B.154763885119611.ack@debbugs.gnu.org>
@ 2019-01-25 10:09   ` Mattias Engdegård
  2019-01-28 15:53     ` Michael Heerdegen
  0 siblings, 1 reply; 5+ messages in thread
From: Mattias Engdegård @ 2019-01-25 10:09 UTC (permalink / raw)
  To: 34100

Anything more I can do? I thought this one would be rather straightforward.
I do understand that there are more pressing matters.






^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#34100: Acknowledgement (26.1; Incomplete ? and ?? handling in rx)
  2019-01-25 10:09   ` bug#34100: Acknowledgement (26.1; Incomplete ? and ?? handling in rx) Mattias Engdegård
@ 2019-01-28 15:53     ` Michael Heerdegen
  2019-02-01  9:44       ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Heerdegen @ 2019-01-28 15:53 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: 34100

Mattias Engdegård <mattiase@acm.org> writes:

> Anything more I can do? I thought this one would be rather
> straightforward.  I do understand that there are more pressing
> matters.

I guess you just have to be patient until someone has a look.  That can
sometimes take some time, there are a lot of open bug reports...

For me what you suggest looks like a good idea btw.

Michael.





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#34100: Acknowledgement (26.1; Incomplete ? and ?? handling in rx)
  2019-01-28 15:53     ` Michael Heerdegen
@ 2019-02-01  9:44       ` Eli Zaretskii
  0 siblings, 0 replies; 5+ messages in thread
From: Eli Zaretskii @ 2019-02-01  9:44 UTC (permalink / raw)
  To: mattiase; +Cc: Michael Heerdegen, 34100-done

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Mon, 28 Jan 2019 16:53:28 +0100
> Cc: 34100@debbugs.gnu.org
> 
> Mattias Engdegård <mattiase@acm.org> writes:
> 
> > Anything more I can do? I thought this one would be rather
> > straightforward.  I do understand that there are more pressing
> > matters.
> 
> I guess you just have to be patient until someone has a look.  That can
> sometimes take some time, there are a lot of open bug reports...
> 
> For me what you suggest looks like a good idea btw.

Sorry for the long delay, Mattias.  I've now pushed your changes to
the master branch, and I'm marking this bug done.

Thanks.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-02-01  9:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-16 11:39 bug#34100: 26.1; Incomplete ? and ?? handling in rx Mattias Engdegård
2019-01-16 14:06 ` bug#34100: rx \? and \??: patch Mattias Engdegård
     [not found] ` <handler.34100.B.154763885119611.ack@debbugs.gnu.org>
2019-01-25 10:09   ` bug#34100: Acknowledgement (26.1; Incomplete ? and ?? handling in rx) Mattias Engdegård
2019-01-28 15:53     ` Michael Heerdegen
2019-02-01  9:44       ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.