bug#21072: 24.5; inconsistent behaviour of `C-M-h (mark-defun)' in Emacs Lisp

[Marcin Borkowski <mbork@amu.edu.pl>]

On 2016-06-09, at 13:56, Marcin Borkowski <mbork@mbork.pl> wrote:

> 2b. Write a few tests for `mark-defun',_then_ fix the problem, and check
> whether these tests pass.  Of course, all these tests would be for Elisp
> (and maybe for C and/or JavaScript).

Hi all,

it seems nobody cared enough to answer my question, so I made the choice
of doing the Right Thing™, and started from developing some tests for
mark-defun.  Here's what I've got now.

--8<---------------cut here---------------start------------->8---
;;; elisp-mode-tests.el --- Tests for emacs-lisp-mode  -*- lexical-binding: t; -*-

;; Copyright (C) 2015-2016 Free Software Foundation, Inc.

;; Author: Marcin Borkowski <mbork@mbork.pl>
;; Author: Dmitry Gutov <dgutov@yandex.ru>
;; Author: Stephen Leake <stephen_leake@member.fsf.org>

;; 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 <http://www.gnu.org/licenses/>.

;;; Code:

(require 'ert)

\f
;;; Helpers

;; Copied and modified from `python-tests-with-temp-buffer'
(defmacro elisp-tests-with-temp-buffer (contents &rest body)
  "Create a `emacs-lisp-mode' enabled temp buffer with CONTENTS.
BODY is code to be executed within the temp buffer.  Point is
always located at the beginning of buffer."
  (declare (indent 1) (debug t))
  `(with-temp-buffer
     (emacs-lisp-mode)
     (insert ,contents)
     (goto-char (point-min))
     ,@body))

(defmacro conditional-save-excursion (arg &rest body)
  "Wrap BODY in `save-excursion', but only if ARG is non-nil." 
 (declare (indent 1) (debug t))
  `(if ,arg
       (save-excursion ,@body)
     ,@body))

(defun look-at (string &optional count restore-point)
  "Move the point to the beginning of STRING in current buffer.
Return the new point value.  If COUNT is non-nil, move to COUNTth
occurrence.  If RESTORE-POINT is non-nil, return the found
position, but do not move point."
  (conditional-save-excursion restore-point
    (setq count (or count 1))
    (when (search-forward string nil t count)
      (goto-char (match-beginning 0)))
    (point)))

\f
;;; Mark

(ert-deftest mark-defun-1 ()
  "Test `mark-defun' with point inside the defun."
  (elisp-tests-with-temp-buffer
   "
\(defun func-a ()
  \"A parameterless function.\"
  (ignore))

;; A comment right before a defun.
\(defun func-b (argument)
  \"A function with one ARGUMENT.\"
  (ignore argument)
  (message \"%s\" \"Argument ignored.\"))

\(defmacro macro-a (&rest body)
  \"A macro with BODY.\"
  `(,@body))
"
   (let ((expected-mark-beginning-position-1-2
          (progn
            (look-at "(defun func-a ")
            (previous-line 1)
            (point)))
         (expected-mark-end-position-1
          (save-excursion
            (look-at "(ignore))")
            (next-line 1)
            (point)))
         (expected-mark-end-position-2
          (save-excursion
            (point)
            (look-at "\"))")
            (next-line 1)
            (point)))
         (expected-mark-beginning-position-3
          (progn
            (look-at "(defmacro macro-a ")
            (previous-line 1)
            (point)))
         (expected-mark-end-position-3
          (progn
            (look-at "(,@body)")
            (next-line 1)
            (point))))
     ;; select the first defun
     (goto-char (point-min))
     (look-at "A parameterless function.")
     (mark-defun)
     (should (= (point) expected-mark-beginning-position-1-2))
     (should (= (marker-position (mark-marker))
                expected-mark-end-position-1))
     ;; expand to the second defun
     (mark-defun 1)
     (should (= (point) expected-mark-beginning-position-1-2))
     (should (= (marker-position (mark-marker))
                expected-mark-end-position-2))
     ;; select the macro
     (look-at "A macro")
     (mark-defun)
     (should (= (point) expected-mark-beginning-position-3))
     (should (= (marker-position (mark-marker))
                expected-mark-end-position-3)))))

(ert-deftest mark-defun-2 ()
  "Test `mark-defun' with point between defuns."
  (elisp-tests-with-temp-buffer
      "
\(defun func-a ()
  \"A parameterless function.\"
  (ignore))

;; A comment right before a defun.
\(defun func-b (argument)
  \"A function with one ARGUMENT.\"
  (ignore argument)
  (message \"%s\" \"Argument ignored.\"))

\(defmacro macro-a (&rest body)
  \"A macro with BODY.\"
  `(,@body))
"
    (let ((expected-mark-beginning-position-1
           (progn
             (look-at "(defun func-b ")
             (point)))
          (expected-mark-end-position-1
           (save-excursion
             (look-at "ignored.\"))")
             (next-line 1)
             (point))))
      ;; select the first defun
      (goto-char expected-mark-end-position-1)
      (previous-line 1)
      (mark-defun)
      (should (= (point) expected-mark-beginning-position-1))
      (should (= (marker-position (mark-marker))
                 expected-mark-end-position-1)))))

(provide 'elisp-mode-tests)
;;; elisp-mode-tests.el ends here
--8<---------------cut here---------------end--------------->8---

These two tests pass now.  The next thing would be to change them to
what /should/ pass.  Before that, though, let me ask: what are your
opinion on these simple tests?  Are they enough?  Would you add
something?

Best,

-- 
Marcin Borkowski
http://octd.wmi.amu.edu.pl/en/Marcin_Borkowski
Faculty of Mathematics and Computer Science
Adam Mickiewicz University