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

From: Andreas Röhler <[email-masked]>
Date: Tue, 21 Jun 2016 11:05:39 +0200

On 21.06.2016 09:58, Marcin Borkowski wrote:
> On 2016-06-09, at 13:56, Marcin Borkowski <[email-masked]> 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 <[email-masked]>
> ;; Author: Dmitry Gutov <[email-masked]>
> ;; Author: Stephen Leake <[email-masked]>
>
> ;; 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)
>
> 
> ;;; 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)))
>
> 
> ;;; 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,
>

Hi Marcin,

should no one else respond for now: please come back after release.
AFAIU the current implemention of beginning-of-defun prevents a 
consistent setup of related commands. A change here would include resp. 
require a major change...

Don't give up :)

Andreas