Subject: bug#17699: [PATCH 1/7] tildify.el: Fix end-regex building in `tildify-find-env'

From: Michal Nazarewicz <[email-masked]>
Date: Thu, 5 Jun 2014 13:27:30 +0200

* lisp/textmodes/tildify.el (tildify-find-env): The
`tildify-ignored-environments-alist' allows the end-regex
to be provided not as a static string but mix of strings and
indexes of groups matched the begin-regex.  For example, the
“\verb!…!” TeX-command (where “!” is an arbitrary character)
is handled using:

    ("\\\\verb\\*?\\(.\\)" . (1))

In the same way, the following should be supported as well:

    ("open-\\(.\\)" . ("end-" 1))

However the tildify-find-env function fails at

    (concat result
            (if (stringp (setq aux (car expression)))
                 expression  ; BUG: expression is a list
               (regexp-quote (match-string aux))))

where the string part is handled incorrectly.

The most trivial fix would be to replace `expression'
in the true-part of the if-statement with `aux', but
instead, this commit optimises `tildify-find-env' by
changing it to use `mapconcat' rather than open-coded
while-loop.

* tests/automated/tildify-tests.el (tildify-test-find-env-end-re-bug):
New test validating fix to the above bug.
---
 lisp/ChangeLog                  | 31 +++++++++++++++++++++++++++++++
 lisp/textmodes/tildify.el       | 40 ++++++++++++++++++----------------------
 test/ChangeLog                  |  6 ++++++
 test/automated/tildify-tests.el | 11 +++++++++++
 4 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index a6ae6f8..dc3c42a 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,34 @@
+2014-06-05  Michal Nazarewicz  <[email-masked]>
+
+	* textmodes/tildify.el (tildify-find-env): Fix end-regex building
+	in `tildify-find-env'
+
+	The `tildify-ignored-environments-alist' allows the end-regex to
+	be provided not as a static string but mix of strings and indexes
+	of groups matched the begin-regex.  For example, the “\verb!…!”
+	TeX-command (where “!” is an arbitrary character) is handled
+	using:
+
+	    ("\\\\verb\\*?\\(.\\)" . (1))
+
+	In the same way, the following should be supported as well:
+
+	    ("open-\\(.\\)" . ("end-" 1))
+
+	However the tildify-find-env function fails at
+
+	    (concat result
+	            (if (stringp (setq aux (car expression)))
+	                 expression  ; BUG: expression is a list
+	               (regexp-quote (match-string aux))))
+
+	where the string part is handled incorrectly.
+
+	The most trivial fix would be to replace `expression' in the
+	true-part of the if-statement with `aux', but instead, this commit
+	optimises `tildify-find-env' by changing it to use `mapconcat'
+	rather than open-coded while-loop.
+
 2014-06-03  Rüdiger Sonderfeld  <[email-masked]>
 
 	* register.el: Add link to Emacs manual in Commentary.
diff --git a/lisp/textmodes/tildify.el b/lisp/textmodes/tildify.el
index 339f900..7e4b39b 100644
--- a/lisp/textmodes/tildify.el
+++ b/lisp/textmodes/tildify.el
@@ -3,7 +3,7 @@
 ;; Copyright (C) 1997-2014 Free Software Foundation, Inc.
 
 ;; Author:     Milan Zamazal <[email-masked]>
-;; Version:    4.5.1
+;; Version:    4.5.2
 ;; Keywords:   text, TeX, SGML, wp
 
 ;; This file is part of GNU Emacs.
@@ -270,27 +270,23 @@ won't be prompted for confirmation of each substitution."
 Return regexp for the end of the environment or nil if no environment was
 found."
   ;; Find environment
-  (if (re-search-forward regexp nil t)
-      ;; Build end-env regexp
-      (let ((match (match-string 0))
-	    (alist (tildify-mode-alist tildify-ignored-environments-alist))
-	    expression)
-	(save-match-data
-	  (while (not (eq (string-match (caar alist) match) 0))
-	    (setq alist (cdr alist))))
-	(if (stringp (setq expression (cdar alist)))
-	    expression
-	  (let ((result "")
-		aux)
-	    (while expression
-	      (setq result (concat result
-				   (if (stringp (setq aux (car expression)))
-				       expression
-				     (regexp-quote (match-string aux)))))
-	      (setq expression (cdr expression)))
-	    result)))
-    ;; Return nil if not found
-    nil))
+  (when (re-search-forward regexp nil t)
+    ;; Build end-env regexp
+    (let ((match (match-string 0))
+          (alist (tildify-mode-alist tildify-ignored-environments-alist)))
+      (save-match-data
+        (while (not (eq (string-match (caar alist) match) 0))
+          (setq alist (cdr alist))))
+      (let ((expression (cdar alist)))
+        (if (stringp expression)
+            expression
+          (mapconcat
+           (lambda (expr)
+             (if (stringp expr)
+                 expr
+               (regexp-quote (match-string expr))))
+           expression
+           ""))))))
 
 (defun tildify-tildify (beg end ask)
   "Add tilde characters in the region between BEG and END.
diff --git a/test/ChangeLog b/test/ChangeLog
index b6b3dd3..db32aae 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,9 @@
+2014-06-05  Michal Nazarewicz  <[email-masked]>
+
+	* automated/tildify-tests.el (tildify-test-find-env-end-re-bug): New
+	test checking end-regex building in `tildify-find-env' function when
+	integers (denoting capture groups) and strings are mixed together.
+
 2014-06-02  Michael Albinus  <[email-masked]>
 
 	* automated/tramp-tests.el (tramp-remote-process-environment): Declare.
diff --git a/test/automated/tildify-tests.el b/test/automated/tildify-tests.el
index 4223029..25e9f22 100644
--- a/test/automated/tildify-tests.el
+++ b/test/automated/tildify-tests.el
@@ -101,6 +101,17 @@ latter is missing, SENTENCE will be used in all placeholder positions."
                         (tildify-test--example-tex sentence sentence)
                         (tildify-test--example-tex sentence with-nbsp))))
 
+
+(ert-deftest tildify-test-find-env-end-re-bug ()
+    "Tests generation of end-regex using mix of indexes and strings"
+  (with-temp-buffer
+    (let ((tildify-ignored-environments-alist
+           `((,major-mode ("foo\\|bar" . ("end-" 0))))))
+      (insert "foo whatever end-foo")
+      (goto-char (point-min))
+      (should (string-equal "end-foo" (tildify-find-env "foo\\|bar"))))))
+
+
 (provide 'tildify-tests)
 
 ;;; tildify-tests.el ends here
-- 
2.0.0.526.g5318336