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

[Marcin Borkowski <mbork@mbork.pl>]

On 2016-06-21, at 11:05, Andreas Röhler <andreas.roehler@easy-emacs.de> wrote:

> On 21.06.2016 09:58, Marcin Borkowski wrote:
>> 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,
>>
>
> 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...

OK, I see.

> Don't give up :)

I don't, I'm patient - I'm also working very slowly on this...

> Andreas

Thanks, best

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