[PATCH] Unit tests and lexical-binding for Tempo

[PATCH] Unit tests and lexical-binding for Tempo

[Federico Tedin]

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

Hi all,

I have added the lexical-binding header to tempo.el, and added in some
unit tests that test the different tags/elements that a Tempo template can
contain. I've also expanded the documentation for
`tempo-define-template' to mention the user variable
`tempo-user-elements', and made three variables buffer-local.

Any feedback is greatly appreciated.
Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 11605 bytes --]

From b377640c04e9318d67fcb74dfcfd9b455d78dca9 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Mon, 13 May 2019 15:55:09 -0300
Subject: [PATCH 1/1] Use lexical-binding in tempo.el and add tests

* lisp/tempo.el: Use lexical-binding.
(tempo-define-template): Expand documentation to mention
`tempo-user-elements'.
(tempo-named-insertions, tempo-region-start, tempo-region-stop): Make
them buffer-local.
* test/lisp/tempo-tests.el: Add tests for tempo.el.
---
 lisp/tempo.el            |  18 ++-
 test/lisp/tempo-tests.el | 229 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 241 insertions(+), 6 deletions(-)
 create mode 100644 test/lisp/tempo-tests.el

diff --git a/lisp/tempo.el b/lisp/tempo.el
index 28afbec0f4..66192439cc 100644
--- a/lisp/tempo.el
+++ b/lisp/tempo.el
@@ -1,4 +1,4 @@
-;;; tempo.el --- Flexible template insertion
+;;; tempo.el --- Flexible template insertion -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1994-1995, 2001-2019 Free Software Foundation, Inc.
 
@@ -211,6 +211,9 @@ tempo-region-stop
 (make-variable-buffer-local 'tempo-match-finder)
 (make-variable-buffer-local 'tempo-collection)
 (make-variable-buffer-local 'tempo-dirty-collection)
+(make-variable-buffer-local 'tempo-named-insertions)
+(make-variable-buffer-local 'tempo-region-start)
+(make-variable-buffer-local 'tempo-region-stop)
 
 ;;; Functions
 
@@ -268,11 +271,14 @@ tempo-define-template
  - `n>': Inserts a newline and indents line.
  - `o': Like `%' but leaves the point before the newline.
  - nil: It is ignored.
- - Anything else: It is evaluated and the result is treated as an
-   element to be inserted.  One additional tag is useful for these
-   cases.  If an expression returns a list (l foo bar), the elements
-   after `l' will be inserted according to the usual rules.  This makes
-   it possible to return several elements from one expression."
+ - Anything else: Each function in `tempo-user-elements' is called
+   with it as argument until one of them returns non-nil, and the
+   result is inserted. If all of them return nil, it is evaluated and
+   the result is treated as an element to be inserted.  One additional
+   tag is useful for these cases.  If an expression returns a list (l
+   foo bar), the elements after `l' will be inserted according to the
+   usual rules.  This makes it possible to return several elements
+   from one expression."
   (let* ((template-name (intern (concat "tempo-template-"
 				       name)))
 	 (command-name template-name))
diff --git a/test/lisp/tempo-tests.el b/test/lisp/tempo-tests.el
new file mode 100644
index 0000000000..4ce0830700
--- /dev/null
+++ b/test/lisp/tempo-tests.el
@@ -0,0 +1,229 @@
+;;; tempo-tests.el --- Test suite for tempo.el  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; Author: Federico Tedin <federicotedin@gmail.com>
+;; Keywords: tempo
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'tempo)
+
+(ert-deftest string-element-test ()
+  "Test a template containing a string element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("GNU Emacs Tempo test"))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "GNU Emacs Tempo test"))))
+
+(ert-deftest p-bare-element-test ()
+  "Test a template containing a bare `p' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("abcde" p))
+    (tempo-insert-template 'tempo-template-test nil)
+    (tempo-forward-mark)
+    (should (equal (point) 6))))
+
+(ert-deftest r-bare-element-test ()
+  "Test a template containing a bare `r' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("abcde" r "ghijk"))
+    (insert "F")
+    (set-mark-command nil)
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test t)
+    (should (equal (buffer-string) "abcdeFghijk"))))
+
+(ert-deftest p-element-test ()
+  "Testing template containing a `p' (prompt) element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("hello " (p ">")))
+    (setq tempo-interactive t)
+    (cl-letf (((symbol-function 'read-string) (lambda (_) "world")))
+      (tempo-insert-template 'tempo-template-test nil))
+    (should (equal (buffer-string) "hello world"))))
+
+(ert-deftest P-element-test ()
+  "Testing template containing a `P' (prompt) element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("hello " (P ">")))
+    (setq tempo-interactive nil) ;; `P' will ignore this
+    (cl-letf (((symbol-function 'read-string) (lambda (_) "world")))
+      (tempo-insert-template 'tempo-template-test nil))
+    (should (equal (buffer-string) "hello world"))))
+
+(ert-deftest r-element-test ()
+  "Testing template containing an `r' (with prompt) element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("abcde" (r ">") "ghijk"))
+    (setq tempo-interactive t)
+    (cl-letf (((symbol-function 'read-string) (lambda (_) "F")))
+      (tempo-insert-template 'tempo-template-test nil))
+    (should (equal (buffer-string) "abcdeFghijk"))))
+
+(ert-deftest s-element-test ()
+  "Testing template containing an `s' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("hello " (p ">" P1) " " (s P1)))
+    (setq tempo-interactive t)
+    (cl-letf (((symbol-function 'read-string) (lambda (_) "world!")))
+      (tempo-insert-template 'tempo-template-test nil))
+    (should (equal (buffer-string) "hello world! world!"))))
+
+(ert-deftest &-element-test ()
+  "Testing template containing an `&' element."
+  (tempo-define-template "test" '(& "test"))
+  (with-temp-buffer
+    (insert "  ")
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "  test")))
+  (with-temp-buffer
+    (insert "hello")
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "hello\ntest"))))
+
+(ert-deftest %-element-test ()
+  "Testing template containing an `%' element."
+  (tempo-define-template "test" '("test" %))
+  (with-temp-buffer
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test")))
+  (with-temp-buffer
+    (insert "hello")
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test\nhello"))))
+
+(ert-deftest n-element-test ()
+  "Testing template containing an `n' element."
+  (tempo-define-template "test" '("test" n "test"))
+  (with-temp-buffer
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test\ntest"))))
+
+(ert-deftest n>-element-test ()
+  "Testing template containing an `n>' element."
+  (tempo-define-template "test" '("(progn" n> "(list 1 2 3))"))
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (tempo-insert-template 'tempo-template-test nil)
+    ;; Tempo should have inserted two spaces before (list 1 2 3)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest >-element-test ()
+  "Testing template containing a `>' element."
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert "(progn\n)")
+    (backward-char)
+    (tempo-define-template "test" '("(list 1 2 3)" >))
+    (tempo-insert-template 'tempo-template-test nil)
+    ;; Tempo should have inserted two spaces before (list 1 2 3)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest r>-bare-element-test ()
+  "Testing template containing a bare `r>' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("(progn" n r> ")"))
+    (emacs-lisp-mode)
+    (insert "(list 1 2 3)")
+    (set-mark-command nil)
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test t)
+    ;; Tempo should have inserted two spaces before (list 1 2 3)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest r>-element-test ()
+  "Testing template containing an `r>' (with prompt) element."
+  (tempo-define-template "test" '("(progn" n (r> ":") ")"))
+  (with-temp-buffer
+    ;; Test on-region use
+    (emacs-lisp-mode)
+    (setq tempo-interactive nil)
+    (insert "(list 1 2 3)")
+    (set-mark-command nil)
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test t)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))")))
+  (with-temp-buffer
+    ;; Test interactive use
+    (emacs-lisp-mode)
+    (setq tempo-interactive t)
+    (cl-letf (((symbol-function 'read-string) (lambda (_) "  (list 1 2 3)")))
+      (tempo-insert-template 'tempo-template-test nil))
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest o-element-test ()
+  "Testing template containing an `o' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("test" o))
+    (insert "hello")
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test\nhello"))
+    (should (equal (point) 5))))
+
+(ert-deftest nil-element-test ()
+  "Testing template with nil elements."
+  (with-temp-buffer
+    (tempo-define-template "test" '("Hello," nil " World!"))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "Hello, World!"))))
+
+(ert-deftest eval-element-test ()
+  "Testing template with Emacs Lisp expressions."
+  (with-temp-buffer
+    (tempo-define-template "test" '((int-to-string (+ 1 1)) "=" (concat "1" "+1")))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "2=1+1"))))
+
+(ert-deftest l-element-test ()
+  "Testing template containing an `l' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("list: " (l "1, " "2, " (int-to-string (+ 1 2)))))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "list: 1, 2, 3"))))
+
+(ert-deftest tempo-user-elements-test ()
+  "Testing a template containing an element to be used as an argument
+in a call to a function in `tempo-user-elements'."
+  (with-temp-buffer
+    (make-variable-buffer-local 'tempo-user-elements)
+    (add-to-list 'tempo-user-elements (lambda (x) (int-to-string (* x x))))
+    (tempo-define-template "test" '(1 " " 2 " " 3 " " 4))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "1 4 9 16"))))
+
+(ert-deftest expand-tag-test ()
+  "Testing expansion of a template with a tag."
+  (with-temp-buffer
+    (tempo-define-template "test" '("Hello, World!") "hello")
+    (insert "hello")
+    (tempo-complete-tag)
+    (should (equal (buffer-string) "Hello, World!"))))
+
+(ert-deftest expand-partial-tag-test ()
+  "Testing expansion of a template with a tag, with a partial match."
+  (with-temp-buffer
+    (tempo-define-template "test" '("Hello, World!") "hello")
+    (insert "hel")
+    (tempo-complete-tag)
+    (should (equal (buffer-string) "Hello, World!"))))
+
+(provide 'tempo-tests)
+;;; tempo-tests.el ends here
-- 
2.17.1

Re: [PATCH] Unit tests and lexical-binding for Tempo

[Basil L. Contovounesios]

Thanks for working on this.

Federico Tedin <federicotedin@gmail.com> writes:

> Any feedback is greatly appreciated.

Just some minor comments and questions from me.

> diff --git a/lisp/tempo.el b/lisp/tempo.el
> index 28afbec0f4..66192439cc 100644
> --- a/lisp/tempo.el
> +++ b/lisp/tempo.el
> @@ -1,4 +1,4 @@
> -;;; tempo.el --- Flexible template insertion
> +;;; tempo.el --- Flexible template insertion -*- lexical-binding: t; -*-
>  
>  ;; Copyright (C) 1994-1995, 2001-2019 Free Software Foundation, Inc.
>  
> @@ -211,6 +211,9 @@ tempo-region-stop
>  (make-variable-buffer-local 'tempo-match-finder)
>  (make-variable-buffer-local 'tempo-collection)
>  (make-variable-buffer-local 'tempo-dirty-collection)
> +(make-variable-buffer-local 'tempo-named-insertions)
> +(make-variable-buffer-local 'tempo-region-start)
> +(make-variable-buffer-local 'tempo-region-stop)

Why not define these with defvar-local instead?
  
>  ;;; Functions
>  
> @@ -268,11 +271,14 @@ tempo-define-template
>   - `n>': Inserts a newline and indents line.
>   - `o': Like `%' but leaves the point before the newline.
>   - nil: It is ignored.
> - - Anything else: It is evaluated and the result is treated as an
> -   element to be inserted.  One additional tag is useful for these
> -   cases.  If an expression returns a list (l foo bar), the elements
> -   after `l' will be inserted according to the usual rules.  This makes
> -   it possible to return several elements from one expression."
> + - Anything else: Each function in `tempo-user-elements' is called
> +   with it as argument until one of them returns non-nil, and the
> +   result is inserted. If all of them return nil, it is evaluated and
                        ^^^
Missing space.

> +   the result is treated as an element to be inserted.  One additional
> +   tag is useful for these cases.  If an expression returns a list (l
> +   foo bar), the elements after `l' will be inserted according to the
> +   usual rules.  This makes it possible to return several elements
> +   from one expression."
>    (let* ((template-name (intern (concat "tempo-template-"
>  				       name)))
>  	 (command-name template-name))
> diff --git a/test/lisp/tempo-tests.el b/test/lisp/tempo-tests.el
> new file mode 100644
> index 0000000000..4ce0830700
> --- /dev/null
> +++ b/test/lisp/tempo-tests.el
> @@ -0,0 +1,229 @@
> +;;; tempo-tests.el --- Test suite for tempo.el  -*- lexical-binding: t; -*-
> +
> +;; Copyright (C) 2019 Free Software Foundation, Inc.
> +
> +;; Author: Federico Tedin <federicotedin@gmail.com>
> +;; Keywords: tempo

No need for this; see (info "(elisp) Library Headers") and M-x
finder-list-keywords RET for valid keywords and their meanings.

> +
> +;; This file is part of GNU Emacs.
> +
> +;; GNU Emacs is free software: you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +
> +;; GNU Emacs is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
> +
> +;;; Code:
> +
> +(require 'tempo)
> +
> +(ert-deftest string-element-test ()

Shouldn't test names have a package-specific prefix?

> +  "Test a template containing a string element."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("GNU Emacs Tempo test"))
> +    (tempo-insert-template 'tempo-template-test nil)
> +    (should (equal (buffer-string) "GNU Emacs Tempo test"))))
> +
> +(ert-deftest p-bare-element-test ()
> +  "Test a template containing a bare `p' element."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("abcde" p))
> +    (tempo-insert-template 'tempo-template-test nil)
> +    (tempo-forward-mark)
> +    (should (equal (point) 6))))
> +
> +(ert-deftest r-bare-element-test ()
> +  "Test a template containing a bare `r' element."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("abcde" r "ghijk"))
> +    (insert "F")
> +    (set-mark-command nil)

Why not set-mark?

> +    (goto-char (point-min))
> +    (tempo-insert-template 'tempo-template-test t)
> +    (should (equal (buffer-string) "abcdeFghijk"))))
> +
> +(ert-deftest p-element-test ()
> +  "Testing template containing a `p' (prompt) element."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("hello " (p ">")))
> +    (setq tempo-interactive t)

AFAICT this changes the user option's global value, which both normal
programs and tests should avoid doing; see (info "(ert) Tests and Their
Environment").  Just let-bind instead.

> +    (cl-letf (((symbol-function 'read-string) (lambda (_) "world")))

Before calling cl-letf you need (eval-when-compile (require 'cl-lib)).
Also, shouldn't the lambda accept (&rest _) args?

> +      (tempo-insert-template 'tempo-template-test nil))
> +    (should (equal (buffer-string) "hello world"))))
> +
> +(ert-deftest P-element-test ()
> +  "Testing template containing a `P' (prompt) element."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("hello " (P ">")))
> +    (setq tempo-interactive nil) ;; `P' will ignore this

RHS comments are usually written with a single semicolon;
see (info "(elisp) Comment Tips").

> +    (cl-letf (((symbol-function 'read-string) (lambda (_) "world")))
> +      (tempo-insert-template 'tempo-template-test nil))
> +    (should (equal (buffer-string) "hello world"))))

[...]

> +(ert-deftest tempo-user-elements-test ()
> +  "Testing a template containing an element to be used as an argument
> +in a call to a function in `tempo-user-elements'."
> +  (with-temp-buffer
> +    (make-variable-buffer-local 'tempo-user-elements)

This makes tempo-user-elements automatically buffer-local globally.
I think you're after make-local-variable instead.

> +    (add-to-list 'tempo-user-elements (lambda (x) (int-to-string (* x x))))
> +    (tempo-define-template "test" '(1 " " 2 " " 3 " " 4))
> +    (tempo-insert-template 'tempo-template-test nil)
> +    (should (equal (buffer-string) "1 4 9 16"))))
> +
> +(ert-deftest expand-tag-test ()
> +  "Testing expansion of a template with a tag."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("Hello, World!") "hello")
> +    (insert "hello")
> +    (tempo-complete-tag)
> +    (should (equal (buffer-string) "Hello, World!"))))
> +
> +(ert-deftest expand-partial-tag-test ()
> +  "Testing expansion of a template with a tag, with a partial match."
> +  (with-temp-buffer
> +    (tempo-define-template "test" '("Hello, World!") "hello")
> +    (insert "hel")
> +    (tempo-complete-tag)
> +    (should (equal (buffer-string) "Hello, World!"))))
> +
> +(provide 'tempo-tests)
> +;;; tempo-tests.el ends here

-- 
Basil

Re: [PATCH] Unit tests and lexical-binding for Tempo

[Federico Tedin]

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

Hi Basil, thanks for taking the time to review the patch. I've fixed the
mistakes using the suggestions you made. I've also added the 'abbrev' tag
to tempo.el.


[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 14153 bytes --]

From dad0f0a6841e321cfca4c2962e3ce384199ff51e Mon Sep 17 00:00:00 2001
From: federicotdn <federicotedin.ar@gmail.com>
Date: Tue, 14 May 2019 09:16:00 -0300
Subject: [PATCH 1/1] Use lexical-binding in tempo.el and add tests

* lisp/tempo.el: Use lexical-binding.
(tempo-define-template): Expand documentation to mention
`tempo-user-elements'.
(tempo-named-insertions, tempo-region-start, tempo-region-stop): Make
them buffer-local.
* test/lisp/tempo-tests.el: Add tests for tempo.el.
---
 lisp/tempo.el            |  41 ++++-----
 test/lisp/tempo-tests.el | 229 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 247 insertions(+), 23 deletions(-)
 create mode 100644 test/lisp/tempo-tests.el

diff --git a/lisp/tempo.el b/lisp/tempo.el
index 28afbec..e3b9c76 100644
--- a/lisp/tempo.el
+++ b/lisp/tempo.el
@@ -1,11 +1,11 @@
-;;; tempo.el --- Flexible template insertion
+;;; tempo.el --- Flexible template insertion -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1994-1995, 2001-2019 Free Software Foundation, Inc.
 
 ;; Author: David Kågedal <davidk@lysator.liu.se>
 ;; Created: 16 Feb 1994
 ;; Kågedal's last version number: 1.2.4
-;; Keywords: extensions, languages, tools
+;; Keywords: abbrev, extensions, languages, tools
 
 ;; This file is part of GNU Emacs.
 
@@ -152,7 +152,7 @@ tempo-insert-string-functions
 (defvar tempo-tags nil
   "An association list with tags and corresponding templates.")
 
-(defvar tempo-local-tags '((tempo-tags . nil))
+(defvar-local tempo-local-tags '((tempo-tags . nil))
   "A list of locally installed tag completion lists.
 It is an association list where the car of every element is a symbol
 whose variable value is a template list.  The cdr part, if non-nil,
@@ -161,16 +161,16 @@ tempo-local-tags
 
 `tempo-tags' is always in the last position in this list.")
 
-(defvar tempo-collection nil
+(defvar-local tempo-collection nil
   "A collection of all the tags defined for the current buffer.")
 
-(defvar tempo-dirty-collection t
+(defvar-local tempo-dirty-collection t
   "Indicates if the tag collection needs to be rebuilt.")
 
-(defvar tempo-marks nil
+(defvar-local tempo-marks nil
   "A list of marks to jump to with `\\[tempo-forward-mark]' and `\\[tempo-backward-mark]'.")
 
-(defvar tempo-match-finder "\\b\\([[:word:]]+\\)\\="
+(defvar-local tempo-match-finder "\\b\\([[:word:]]+\\)\\="
   "The regexp or function used to find the string to match against tags.
 
 If `tempo-match-finder' is a string, it should contain a regular
@@ -195,23 +195,15 @@ tempo-user-elements
 This function should return something to be sent to `tempo-insert' if
 it recognizes the argument, and nil otherwise.")
 
-(defvar tempo-named-insertions nil
+(defvar-local tempo-named-insertions nil
   "Temporary storage for named insertions.")
 
-(defvar tempo-region-start (make-marker)
+(defvar-local tempo-region-start (make-marker)
   "Region start when inserting around the region.")
 
-(defvar tempo-region-stop (make-marker)
+(defvar-local tempo-region-stop (make-marker)
   "Region stop when inserting around the region.")
 
-;; Make some variables local to every buffer
-
-(make-variable-buffer-local 'tempo-marks)
-(make-variable-buffer-local 'tempo-local-tags)
-(make-variable-buffer-local 'tempo-match-finder)
-(make-variable-buffer-local 'tempo-collection)
-(make-variable-buffer-local 'tempo-dirty-collection)
-
 ;;; Functions
 
 ;;
@@ -268,11 +260,14 @@ tempo-define-template
  - `n>': Inserts a newline and indents line.
  - `o': Like `%' but leaves the point before the newline.
  - nil: It is ignored.
- - Anything else: It is evaluated and the result is treated as an
-   element to be inserted.  One additional tag is useful for these
-   cases.  If an expression returns a list (l foo bar), the elements
-   after `l' will be inserted according to the usual rules.  This makes
-   it possible to return several elements from one expression."
+ - Anything else: Each function in `tempo-user-elements' is called
+   with it as argument until one of them returns non-nil, and the
+   result is inserted.  If all of them return nil, it is evaluated and
+   the result is treated as an element to be inserted.  One additional
+   tag is useful for these cases.  If an expression returns a list (l
+   foo bar), the elements after `l' will be inserted according to the
+   usual rules.  This makes it possible to return several elements
+   from one expression."
   (let* ((template-name (intern (concat "tempo-template-"
 				       name)))
 	 (command-name template-name))
diff --git a/test/lisp/tempo-tests.el b/test/lisp/tempo-tests.el
new file mode 100644
index 0000000..6dac856
--- /dev/null
+++ b/test/lisp/tempo-tests.el
@@ -0,0 +1,229 @@
+;;; tempo-tests.el --- Test suite for tempo.el  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; Author: Federico Tedin <federicotedin@gmail.com>
+;; Keywords: abbrev
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'tempo)
+(eval-when-compile (require 'cl-lib))
+
+(ert-deftest tempo-string-element-test ()
+  "Test a template containing a string element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("GNU Emacs Tempo test"))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "GNU Emacs Tempo test"))))
+
+(ert-deftest tempo-p-bare-element-test ()
+  "Test a template containing a bare `p' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("abcde" p))
+    (tempo-insert-template 'tempo-template-test nil)
+    (tempo-forward-mark)
+    (should (equal (point) 6))))
+
+(ert-deftest tempo-r-bare-element-test ()
+  "Test a template containing a bare `r' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("abcde" r "ghijk"))
+    (insert "F")
+    (set-mark (point))
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test t)
+    (should (equal (buffer-string) "abcdeFghijk"))))
+
+(ert-deftest tempo-p-element-test ()
+  "Testing template containing a `p' (prompt) element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("hello " (p ">")))
+    (let ((tempo-interactive t))
+      (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "world")))
+        (tempo-insert-template 'tempo-template-test nil))
+      (should (equal (buffer-string) "hello world")))))
+
+(ert-deftest tempo-P-element-test ()
+  "Testing template containing a `P' (prompt) element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("hello " (P ">")))
+    ;; By default, `tempo-interactive' is nil, `P' should ignore this.
+    (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "world")))
+      (tempo-insert-template 'tempo-template-test nil))
+    (should (equal (buffer-string) "hello world"))))
+
+(ert-deftest tempo-r-element-test ()
+  "Testing template containing an `r' (with prompt) element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("abcde" (r ">") "ghijk"))
+    (let ((tempo-interactive t))
+      (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "F")))
+        (tempo-insert-template 'tempo-template-test nil))
+      (should (equal (buffer-string) "abcdeFghijk")))))
+
+(ert-deftest tempo-s-element-test ()
+  "Testing template containing an `s' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("hello " (p ">" P1) " " (s P1)))
+    (let ((tempo-interactive t))
+      (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "world!")))
+        (tempo-insert-template 'tempo-template-test nil))
+      (should (equal (buffer-string) "hello world! world!")))))
+
+(ert-deftest tempo-&-element-test ()
+  "Testing template containing an `&' element."
+  (tempo-define-template "test" '(& "test"))
+  (with-temp-buffer
+    (insert "  ")
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "  test")))
+  (with-temp-buffer
+    (insert "hello")
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "hello\ntest"))))
+
+(ert-deftest tempo-%-element-test ()
+  "Testing template containing an `%' element."
+  (tempo-define-template "test" '("test" %))
+  (with-temp-buffer
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test")))
+  (with-temp-buffer
+    (insert "hello")
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test\nhello"))))
+
+(ert-deftest tempo-n-element-test ()
+  "Testing template containing an `n' element."
+  (tempo-define-template "test" '("test" n "test"))
+  (with-temp-buffer
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test\ntest"))))
+
+(ert-deftest tempo-n>-element-test ()
+  "Testing template containing an `n>' element."
+  (tempo-define-template "test" '("(progn" n> "(list 1 2 3))"))
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (tempo-insert-template 'tempo-template-test nil)
+    ;; Tempo should have inserted two spaces before (list 1 2 3)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest tempo->-element-test ()
+  "Testing template containing a `>' element."
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert "(progn\n)")
+    (backward-char)
+    (tempo-define-template "test" '("(list 1 2 3)" >))
+    (tempo-insert-template 'tempo-template-test nil)
+    ;; Tempo should have inserted two spaces before (list 1 2 3)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest tempo-r>-bare-element-test ()
+  "Testing template containing a bare `r>' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("(progn" n r> ")"))
+    (emacs-lisp-mode)
+    (insert "(list 1 2 3)")
+    (set-mark (point))
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test t)
+    ;; Tempo should have inserted two spaces before (list 1 2 3)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))"))))
+
+(ert-deftest tempo-r>-element-test ()
+  "Testing template containing an `r>' (with prompt) element."
+  (tempo-define-template "test" '("(progn" n (r> ":") ")"))
+  (with-temp-buffer
+    ;; Test on-region use
+    (emacs-lisp-mode)
+    (insert "(list 1 2 3)")
+    (set-mark (point))
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test t)
+    (should (equal (buffer-string) "(progn\n  (list 1 2 3))")))
+  (with-temp-buffer
+    ;; Test interactive use
+    (emacs-lisp-mode)
+    (let ((tempo-interactive t))
+      (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "  (list 1 2 3)")))
+        (tempo-insert-template 'tempo-template-test nil))
+      (should (equal (buffer-string) "(progn\n  (list 1 2 3))")))))
+
+(ert-deftest tempo-o-element-test ()
+  "Testing template containing an `o' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("test" o))
+    (insert "hello")
+    (goto-char (point-min))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "test\nhello"))
+    (should (equal (point) 5))))
+
+(ert-deftest tempo-nil-element-test ()
+  "Testing template with nil elements."
+  (with-temp-buffer
+    (tempo-define-template "test" '("Hello," nil " World!"))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "Hello, World!"))))
+
+(ert-deftest tempo-eval-element-test ()
+  "Testing template with Emacs Lisp expressions."
+  (with-temp-buffer
+    (tempo-define-template "test" '((int-to-string (+ 1 1)) "=" (concat "1" "+1")))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "2=1+1"))))
+
+(ert-deftest tempo-l-element-test ()
+  "Testing template containing an `l' element."
+  (with-temp-buffer
+    (tempo-define-template "test" '("list: " (l "1, " "2, " (int-to-string (+ 1 2)))))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "list: 1, 2, 3"))))
+
+(ert-deftest tempo-tempo-user-elements-test ()
+  "Testing a template containing an element to be used as an argument
+in a call to a function in `tempo-user-elements'."
+  (with-temp-buffer
+    (make-local-variable 'tempo-user-elements)
+    (add-to-list 'tempo-user-elements (lambda (x) (int-to-string (* x x))))
+    (tempo-define-template "test" '(1 " " 2 " " 3 " " 4))
+    (tempo-insert-template 'tempo-template-test nil)
+    (should (equal (buffer-string) "1 4 9 16"))))
+
+(ert-deftest tempo-expand-tag-test ()
+  "Testing expansion of a template with a tag."
+  (with-temp-buffer
+    (tempo-define-template "test" '("Hello, World!") "hello")
+    (insert "hello")
+    (tempo-complete-tag)
+    (should (equal (buffer-string) "Hello, World!"))))
+
+(ert-deftest tempo-expand-partial-tag-test ()
+  "Testing expansion of a template with a tag, with a partial match."
+  (with-temp-buffer
+    (tempo-define-template "test" '("Hello, World!") "hello")
+    (insert "hel")
+    (tempo-complete-tag)
+    (should (equal (buffer-string) "Hello, World!"))))
+
+(provide 'tempo-tests)
+;;; tempo-tests.el ends here
-- 
2.7.4

Re: [PATCH] Unit tests and lexical-binding for Tempo

[Basil L. Contovounesios]

Federico Tedin <federicotedin@gmail.com> writes:

> Hi Basil, thanks for taking the time to review the patch. I've fixed the
> mistakes using the suggestions you made. I've also added the 'abbrev' tag
> to tempo.el.

Thanks, looks fine to me.

> From dad0f0a6841e321cfca4c2962e3ce384199ff51e Mon Sep 17 00:00:00 2001
> From: federicotdn <federicotedin.ar@gmail.com>
> Date: Tue, 14 May 2019 09:16:00 -0300
> Subject: [PATCH 1/1] Use lexical-binding in tempo.el and add tests

I was about to push your patch when I noticed its From: header lists a
different name and email address from that of your email and other Emacs
contributions.  Is it okay to amend the patch to use the latter?

Thanks,

-- 
Basil

Re: [PATCH] Unit tests and lexical-binding for Tempo

[Federico Tedin]

Hi Basil,
Good catch, sorry about that. The correct header should use "Federico
Tedin <federicotedin@gmail.com>", like you mentioned.
Thanks!

On Mon, May 20, 2019 at 12:26 PM Basil L. Contovounesios
<contovob@tcd.ie> wrote:
>
> Federico Tedin <federicotedin@gmail.com> writes:
>
> > Hi Basil, thanks for taking the time to review the patch. I've fixed the
> > mistakes using the suggestions you made. I've also added the 'abbrev' tag
> > to tempo.el.
>
> Thanks, looks fine to me.
>
> > From dad0f0a6841e321cfca4c2962e3ce384199ff51e Mon Sep 17 00:00:00 2001
> > From: federicotdn <federicotedin.ar@gmail.com>
> > Date: Tue, 14 May 2019 09:16:00 -0300
> > Subject: [PATCH 1/1] Use lexical-binding in tempo.el and add tests
>
> I was about to push your patch when I noticed its From: header lists a
> different name and email address from that of your email and other Emacs
> contributions.  Is it okay to amend the patch to use the latter?
>
> Thanks,
>
> --
> Basil

Re: [PATCH] Unit tests and lexical-binding for Tempo

[Basil L. Contovounesios]

Federico Tedin <federicotedin@gmail.com> writes:

> Good catch, sorry about that. The correct header should use "Federico
> Tedin <federicotedin@gmail.com>", like you mentioned.

No worries.  I took the liberties of extending the commit message to
reference this discussion and mention the other variables which were
switched to defvar-local, and shortening the docstring of
tempo-tempo-user-elements-test to more closely follow the conventions in
(info "(elisp) Documentation Tips"), and pushed to master[1].  Let me
know if I messed anything up.

[1: eb2e9a2ca2]: Use lexical-binding in tempo.el and add tests
  2019-05-21 15:23:23 +0100
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=eb2e9a2ca29f7d5e3b97709e9eca14fa5556ac63

Thanks,

-- 
Basil

Re: [PATCH] Unit tests and lexical-binding for Tempo

[Federico Tedin]

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

Looks perfect, thanks.

On Tue, 21 May 2019 at 11:31 Basil L. Contovounesios <contovob@tcd.ie>
wrote:

> Federico Tedin <federicotedin@gmail.com> writes:
>
> > Good catch, sorry about that. The correct header should use "Federico
> > Tedin <federicotedin@gmail.com>", like you mentioned.
>
> No worries.  I took the liberties of extending the commit message to
> reference this discussion and mention the other variables which were
> switched to defvar-local, and shortening the docstring of
> tempo-tempo-user-elements-test to more closely follow the conventions in
> (info "(elisp) Documentation Tips"), and pushed to master[1].  Let me
> know if I messed anything up.
>
> [1: eb2e9a2ca2]: Use lexical-binding in tempo.el and add tests
>   2019-05-21 15:23:23 +0100
>
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=eb2e9a2ca29f7d5e3b97709e9eca14fa5556ac63
>
> Thanks,
>
> --
> Basil
>

[-- Attachment #2: Type: text/html, Size: 1547 bytes --]