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

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

[Raffaele Ricciardi]

Dear Emacs maintainers,

the documentation for `mark-defun' says:

     The defun marked is the one that contains point or follows point.

However, Emacs may mark the preceding defun instead, and it may
mark an unrelated preceding comment along with the defun.

Please follow this recipe:

  - emacs -Q
  - make sure that *scratch* contains the following code:

------------------------------------------------------------------------
;; This buffer is for notes you don't want to save, and for Lisp evaluation.
;; If you want to create a file, visit that file with C-x C-f,
;; then enter the text in that file's own buffer.

(defun a ()
   nil)

(defun b ()
   nil)
------------------------------------------------------------------------

  - move point anywhere inside the second defun;
  - press `C-M-h' and Emacs will correctly mark the current defun;
  - press `C-g C-M-h' and Emacs will incorrectly mark the previous defun;
  - now move point anywhere inside the first definition;
  - press `C-M-h' and Emacs will correctly mark the current defun;
  - press `C-g C-M-h' and Emacs will incorrectly mark the defun along with
  the preceding comment;
  - now make sure that *scratch* contains the following code:

------------------------------------------------------------------------
;; This buffer is for notes you don't want to save, and for Lisp evaluation.
;; If you want to create a file, visit that file with C-x C-f,
;; then enter the text in that file's own buffer.

(defun a ()
   nil)

;; This buffer is for notes you don't want to save, and for Lisp evaluation.
;; If you want to create a file, visit that file with C-x C-f,
;; then enter the text in that file's own buffer.

(defun b ()
   nil)
------------------------------------------------------------------------

  - move point anywhere inside the second comment and press `C-M-h': Emacs
  will correctly mark the following defun;
  - move point anywhere inside the first comment and press `C-M-h': Emacs
  will incorrectly mark the comment along with the defun.

Thanks for your attention.
Best Regards.

--

In GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.14.5)
  of 2015-06-23 on debian
Windowing system distributor `The X.Org Foundation', version 11.0.11604000
System Description:	Debian GNU/Linux 8.1 (jessie)

Configured using:
  `configure --prefix=/opt/emacs/emacs-24.5'

Important settings:
   value of $LC_MONETARY: en_GB.utf8
   value of $LC_NUMERIC: en_GB.utf8
   value of $LC_TIME: en_GB.utf8
   value of $LANG: en_GB.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Recent messages:
Quit
Mark set [2 times]
Quit
Mark set [2 times]
Quit
Mark set [2 times]
Quit
Mark set [2 times]
Quit
Mark set [2 times]
Quit

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils time-date tooltip electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 73007 6360)
  (symbols 48 17564 0)
  (miscs 40 53 254)
  (strings 32 9482 4902)
  (string-bytes 1 268085)
  (vectors 16 8916)
  (vector-slots 8 383708 18759)
  (floats 8 64 360)
  (intervals 56 334 13)
  (buffers 960 11)
  (heap 1024 48896 861))

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

[Marcin Borkowski]

On 2015-07-16, at 08:12, Raffaele Ricciardi <rfflrccrd@gmail.com> wrote:

> Dear Emacs maintainers,
>
> the documentation for `mark-defun' says:
>
>     The defun marked is the one that contains point or follows point.
>
> However, Emacs may mark the preceding defun instead, and it may
> mark an unrelated preceding comment along with the defun.

Confirmed on GNU Emacs 25.1.50.8 (commit 912e915).

Best,
-- 
Marcin

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

[Eli Zaretskii]

> From: Marcin Borkowski <mbork@amu.edu.pl>
> Date: Mon, 25 Apr 2016 13:11:01 +0200
> Cc: 21072@debbugs.gnu.org
> 
> On 2015-07-16, at 08:12, Raffaele Ricciardi <rfflrccrd@gmail.com> wrote:
> 
> > Dear Emacs maintainers,
> >
> > the documentation for `mark-defun' says:
> >
> >     The defun marked is the one that contains point or follows point.
> >
> > However, Emacs may mark the preceding defun instead, and it may
> > mark an unrelated preceding comment along with the defun.
> 
> Confirmed on GNU Emacs 25.1.50.8 (commit 912e915).

FWIW, I don't see why the behavior flagged "incorrect" is deemed
incorrect.  It looks correct to me.  In all of those examples point is
between 2 functions, so what exactly is "correct" is open to
discussion.

Am I missing something?

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

[Marcin Borkowski]

On 2016-05-01, at 19:17, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@amu.edu.pl>
>> Date: Mon, 25 Apr 2016 13:11:01 +0200
>> Cc: 21072@debbugs.gnu.org
>> 
>> On 2015-07-16, at 08:12, Raffaele Ricciardi <rfflrccrd@gmail.com> wrote:
>> 
>> > Dear Emacs maintainers,
>> >
>> > the documentation for `mark-defun' says:
>> >
>> >     The defun marked is the one that contains point or follows point.
>> >
>> > However, Emacs may mark the preceding defun instead, and it may
>> > mark an unrelated preceding comment along with the defun.
>> 
>> Confirmed on GNU Emacs 25.1.50.8 (commit 912e915).
>
> FWIW, I don't see why the behavior flagged "incorrect" is deemed
> incorrect.  It looks correct to me.  In all of those examples point is
> between 2 functions, so what exactly is "correct" is open to
> discussion.
>
> Am I missing something?

Yes.  Here's a fragment from `mark-defun''s docstring:

--8<---------------cut here---------------start------------->8---
Put mark at end of this defun, point at beginning.
The defun marked is the one that contains point or follows point.
--8<---------------cut here---------------end--------------->8---

And from the Emacs manual:

--8<---------------cut here---------------start------------->8---
If you use the command while point is between defuns, it uses the
following defun.
--8<---------------cut here---------------end--------------->8---

So IMHO it's not "open to discussion" - the behavior is different than
described in both the docstring (a bit implicitly) or in the manual
(explicitly).

Best,

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

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

[Eli Zaretskii]

> From: Marcin Borkowski <mbork@mbork.pl>
> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
> Date: Sun, 01 May 2016 19:49:36 +0200
> 
> --8<---------------cut here---------------start------------->8---
> Put mark at end of this defun, point at beginning.
> The defun marked is the one that contains point or follows point.
> --8<---------------cut here---------------end--------------->8---

So if we say "either precedes or follows point" instead of "follows
point", all will be well?

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

[Marcin Borkowski]

On 2016-05-01, at 20:09, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@mbork.pl>
>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>> Date: Sun, 01 May 2016 19:49:36 +0200
>> 
>> --8<---------------cut here---------------start------------->8---
>> Put mark at end of this defun, point at beginning.
>> The defun marked is the one that contains point or follows point.
>> --8<---------------cut here---------------end--------------->8---
>
> So if we say "either precedes or follows point" instead of "follows
> point", all will be well?

Formally, yes.  Though I guess it would be a bit better to fix the
commands so that it's behavior matches the docs.

Best,

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

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

[Eli Zaretskii]

> From: Marcin Borkowski <mbork@mbork.pl>
> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
> Date: Sun, 01 May 2016 21:45:37 +0200
> 
> > So if we say "either precedes or follows point" instead of "follows
> > point", all will be well?
> 
> Formally, yes.  Though I guess it would be a bit better to fix the
> commands so that it's behavior matches the docs.

Why do you think that?  Why is it better to always mark the function
that follows point?

Me, I think that the current behavior is a kind of feature, since it
lets the user control whether the leading commentary before a function
will or won't be marked.

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

[Marcin Borkowski]

On 2016-05-01, at 21:56, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@mbork.pl>
>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>> Date: Sun, 01 May 2016 21:45:37 +0200
>> 
>> > So if we say "either precedes or follows point" instead of "follows
>> > point", all will be well?
>> 
>> Formally, yes.  Though I guess it would be a bit better to fix the
>> commands so that it's behavior matches the docs.
>
> Why do you think that?  Why is it better to always mark the function
> that follows point?

I don't know, I don't use `mark-defun' personally.  My train of thought
was that if someone wrote something like that inthe docs, s?he thought
it was reasonable.

> Me, I think that the current behavior is a kind of feature, since it
> lets the user control whether the leading commentary before a function
> will or won't be marked.

I don't understand that.  How do I control that?

Best,

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

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

[Eli Zaretskii]

> From: Marcin Borkowski <mbork@mbork.pl>
> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
> Date: Tue, 03 May 2016 20:58:27 +0200
> 
> > Me, I think that the current behavior is a kind of feature, since it
> > lets the user control whether the leading commentary before a function
> > will or won't be marked.
> 
> I don't understand that.  How do I control that?

By invoking the command before or after the commentary that precedes a
function.

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

[Marcin Borkowski]

On 2016-05-04, at 16:45, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@mbork.pl>
>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>> Date: Tue, 03 May 2016 20:58:27 +0200
>> 
>> > Me, I think that the current behavior is a kind of feature, since it
>> > lets the user control whether the leading commentary before a function
>> > will or won't be marked.
>> 
>> I don't understand that.  How do I control that?
>
> By invoking the command before or after the commentary that precedes a
> function.

Sorry, but I still don't get it.  (I think this means at least that the
docs need rewording.)  Take this text:

--8<---------------cut here---------------start------------->8---
(defun spam ()
  (spam))

;; Lorem ipsum
(defun lorem-ipsum ()
  "Lorem ipsum."
  (message "Lorem ipsum."))
--8<---------------cut here---------------end--------------->8---

How do I place point so that `mark-defun' would mark `lorem-ipsum'
together with the comment before?

Best,

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

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

[Eli Zaretskii]

> From: Marcin Borkowski <mbork@mbork.pl>
> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
> Date: Fri, 06 May 2016 14:27:49 +0200
> 
> (defun spam ()
>   (spam))
> 
> ;; Lorem ipsum
> (defun lorem-ipsum ()
>   "Lorem ipsum."
>   (message "Lorem ipsum."))
> --8<---------------cut here---------------end--------------->8---
> 
> How do I place point so that `mark-defun' would mark `lorem-ipsum'
> together with the comment before?

I'm sorry, I no longer remember.  It's hard to communicate when days
pass between messages.

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

[Marcin Borkowski]

On 2016-05-06, at 14:59, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@mbork.pl>
>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>> Date: Fri, 06 May 2016 14:27:49 +0200
>> 
>> (defun spam ()
>>   (spam))
>> 
>> ;; Lorem ipsum
>> (defun lorem-ipsum ()
>>   "Lorem ipsum."
>>   (message "Lorem ipsum."))
>> --8<---------------cut here---------------end--------------->8---
>> 
>> How do I place point so that `mark-defun' would mark `lorem-ipsum'
>> together with the comment before?
>
> I'm sorry, I no longer remember.  It's hard to communicate when days
> pass between messages.

I'm really sorry, but for me the asynchronicity of email and the
possibility of responding after a few days/weeks instead of a few
minutes is one of the main advantages of email as such (as opposed to
"tools" like Facebook, which I consider more of "toys", also because of
the difficulty to spread the conversation to weeks).  I can only spend
that much time on Emacs bugs each day - similarly with other things -
and I often respond after long time (especially if the response needs
some time/effort to compose).

I'm not sure what to do next with this bug.  I tried with the
abovementioned short Elisp file and failed.  Since no-one said that the
current behavior is wrong, maybe I'll try to study the code of
`mark-defun' and modify the docstring/manual accordingly.

Best,

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

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

[Drew Adams]

I agree that the behavior is not particularly consistent, and it
does not completely correspond to the doc.  What the best fix is,
I don't know.

It's been this way for a long time, so there might be people
who expect or like it to do what it does, in which case the
doc should probably be fixed somewhat.

On the other hand, I'd bet that few, if any, would complain if
better behavior were implemented.

In any case, the behavior of being able to repeat to keep selecting
more defuns further down should be kept.

I'd suggest that Someone (TM) (Marcin?) implement a better
behavior and propose it to emacs-devel. ;-)

What might be better?

1. At least consistency wrt which defun gets selected, when
betweeen defuns.  The doc suggests a general rule (the next
defun), but that is not always respected.

2. Something consistent also wrt a comment before the defun
that will be selected.

3. It could be good for a numeric prefix arg to select that
many defuns.

4. It could be good for a negative prefix arg to select in
the opposite direction.  This is the main improvement I'd
like to see.  E.g. `M-- C-M-h' selects the previous defun;
`M-2 C-M-h' selects the two previous defuns.

Someone should play around and dream up something useful.

Wrt #2, I'm not sure what the best approach might be.

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

[Eli Zaretskii]

> From: Marcin Borkowski <mbork@mbork.pl>
> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
> Date: Sat, 07 May 2016 05:47:18 +0200
> 
> I'm not sure what to do next with this bug.  I tried with the
> abovementioned short Elisp file and failed.  Since no-one said that the
> current behavior is wrong, maybe I'll try to study the code of
> `mark-defun' and modify the docstring/manual accordingly.

I'd suggest to change the doc string according to what I recommended a
few messages ago.  I think that would be good enough.

Thanks.

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

[Marcin Borkowski]

On 2016-05-07, at 08:47, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@mbork.pl>
>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>> Date: Sat, 07 May 2016 05:47:18 +0200
>> 
>> I'm not sure what to do next with this bug.  I tried with the
>> abovementioned short Elisp file and failed.  Since no-one said that the
>> current behavior is wrong, maybe I'll try to study the code of
>> `mark-defun' and modify the docstring/manual accordingly.
>
> I'd suggest to change the doc string according to what I recommended a
> few messages ago.  I think that would be good enough.

Hi Eli, hi all,

and sorry for replying after such a long time (again) - I'm afraid
I cannot help this, my time has become extremely limited recently.

I studied the code of mark-defun, and it seems that the reason for
#21072 is quite simple.  I enclose a patch where the two (seemingly)
offending lines are commented; if this is acceptable, of course I'll
prepare a proper patch.  (And while at that, I'll propose replacing
`(and transient-mark-mode mark-active)' with `(use-region-p)'.)

The problem is, I'm not sure whether this change won't break anything.
The only tests I found that deal with `mark-defun' are in
python-tests.el, and my version passes all three of them.

> Thanks.

Best,

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

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

[Marcin Borkowski]

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

And a forgotten attachment.

On 2016-06-05, at 08:30, Marcin Borkowski <mbork@mbork.pl> wrote:

> On 2016-05-07, at 08:47, Eli Zaretskii <eliz@gnu.org> wrote:
>
>>> From: Marcin Borkowski <mbork@mbork.pl>
>>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>>> Date: Sat, 07 May 2016 05:47:18 +0200
>>> 
>>> I'm not sure what to do next with this bug.  I tried with the
>>> abovementioned short Elisp file and failed.  Since no-one said that the
>>> current behavior is wrong, maybe I'll try to study the code of
>>> `mark-defun' and modify the docstring/manual accordingly.
>>
>> I'd suggest to change the doc string according to what I recommended a
>> few messages ago.  I think that would be good enough.
>
> Hi Eli, hi all,
>
> and sorry for replying after such a long time (again) - I'm afraid
> I cannot help this, my time has become extremely limited recently.
>
> I studied the code of mark-defun, and it seems that the reason for
> #21072 is quite simple.  I enclose a patch where the two (seemingly)
> offending lines are commented; if this is acceptable, of course I'll
> prepare a proper patch.  (And while at that, I'll propose replacing
> `(and transient-mark-mode mark-active)' with `(use-region-p)'.)
>
> The problem is, I'm not sure whether this change won't break anything.
> The only tests I found that deal with `mark-defun' are in
> python-tests.el, and my version passes all three of them.
>
>> Thanks.
>
> Best,


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-bug-21072.patch --]
[-- Type: text/x-diff, Size: 788 bytes --]

From e717c6e8a1a0645baa8ea1a3dc965bac9fe9b540 Mon Sep 17 00:00:00 2001
From: Marcin Borkowski <mbork@mbork.pl>
Date: Sun, 5 Jun 2016 08:25:36 +0200
Subject: [PATCH] Fix bug#21072

---
 lisp/emacs-lisp/lisp.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el
index ea7cce6..4b98b80 100644
--- a/lisp/emacs-lisp/lisp.el
+++ b/lisp/emacs-lisp/lisp.el
@@ -505,8 +505,9 @@ mark-defun
 	   (setq beg (point))
 	   (end-of-defun)
 	   (setq end (point))
-	   (while (looking-at "^\n")
-	     (forward-line 1))
+           ;; The two lines below seemingly caused bug#21072.
+	   ;; (while (looking-at "^\n")
+	   ;;   (forward-line 1))
 	   (if (> (point) opoint)
 	       (progn
 		 ;; We got the right defun.
-- 
2.8.3

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

[Marcin Borkowski]

Hi everyone,

it's me again.

I looked at the tests of `mark-defun' for Python, and they did shed some
light on the weird implementation.  In short, `mark-defun' first jumps
to the beginning and the to the end of the current defun, checks whether
the point is now before or after the initial position, and if before,
back up and jump to the end first and to the beginning next.  This might
actually happen, since e.g. in Python, when point is on a "def"
beginning a function definition, `beginning-of-defun' goes to the start
of the _preceding_ defun.  In fact, I think this is a bug in
beginning-of-defun (or more precisely, its Python version), and
consequently I think the weird code in `mark-defun' is a workaround for
a deeper bug.

I can now do one of a few things.  (Not ATM, I'll have to take my time.)

1. Fix the docs so that #21072 is no longer a bug (i.e., change the docs
so that there's no discrepancy between them and code).

2a. Just fix the problem reported in #21072, hoping that it doesn't break
anything else.

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).

3. Try to come up with a better `mark-defun', still given the
limitations of `beginning-of-defun', e.g., make `mark-defun' use the
numeric prefix argument in a reasonable way (i.e., what Drew proposed).

4. Report a bug in `beginning-of-defun', and hope someone (probably not
me, I do not know enough about ppss now and do not have time to learn
that now), and then write a simpler `mark-defun' from scratch.

WDYT?

Best,
Marcin



On 2016-06-05, at 08:30, Marcin Borkowski <mbork@mbork.pl> wrote:

> On 2016-05-07, at 08:47, Eli Zaretskii <eliz@gnu.org> wrote:
>
>>> From: Marcin Borkowski <mbork@mbork.pl>
>>> Cc: rfflrccrd@gmail.com, 21072@debbugs.gnu.org
>>> Date: Sat, 07 May 2016 05:47:18 +0200
>>> 
>>> I'm not sure what to do next with this bug.  I tried with the
>>> abovementioned short Elisp file and failed.  Since no-one said that the
>>> current behavior is wrong, maybe I'll try to study the code of
>>> `mark-defun' and modify the docstring/manual accordingly.
>>
>> I'd suggest to change the doc string according to what I recommended a
>> few messages ago.  I think that would be good enough.
>
> Hi Eli, hi all,
>
> and sorry for replying after such a long time (again) - I'm afraid
> I cannot help this, my time has become extremely limited recently.
>
> I studied the code of mark-defun, and it seems that the reason for
> #21072 is quite simple.  I enclose a patch where the two (seemingly)
> offending lines are commented; if this is acceptable, of course I'll
> prepare a proper patch.  (And while at that, I'll propose replacing
> `(and transient-mark-mode mark-active)' with `(use-region-p)'.)
>
> The problem is, I'm not sure whether this change won't break anything.
> The only tests I found that deal with `mark-defun' are in
> python-tests.el, and my version passes all three of them.
>
>> Thanks.
>
> Best,


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

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

[Marcin Borkowski]

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

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

[Andreas Röhler]

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...

Don't give up :)

Andreas

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

[Marcin Borkowski]

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

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

[Marcin Borkowski]

On 2016-05-07, at 07:07, Drew Adams <drew.adams@oracle.com> wrote:

> I agree that the behavior is not particularly consistent, and it
> does not completely correspond to the doc.  What the best fix is,
> I don't know.
>
> It's been this way for a long time, so there might be people
> who expect or like it to do what it does, in which case the
> doc should probably be fixed somewhat.
>
> On the other hand, I'd bet that few, if any, would complain if
> better behavior were implemented.
>
> In any case, the behavior of being able to repeat to keep selecting
> more defuns further down should be kept.
>
> I'd suggest that Someone (TM) (Marcin?) implement a better
> behavior and propose it to emacs-devel. ;-)
>
> What might be better?
>
> 1. At least consistency wrt which defun gets selected, when
> betweeen defuns.  The doc suggests a general rule (the next
> defun), but that is not always respected.
>
> 2. Something consistent also wrt a comment before the defun
> that will be selected.
>
> 3. It could be good for a numeric prefix arg to select that
> many defuns.
>
> 4. It could be good for a negative prefix arg to select in
> the opposite direction.  This is the main improvement I'd
> like to see.  E.g. `M-- C-M-h' selects the previous defun;
> `M-2 C-M-h' selects the two previous defuns.
>
> Someone should play around and dream up something useful.
>
> Wrt #2, I'm not sure what the best approach might be.

Hi all,

it's been some time, and I implemented a better (?) behavior indeed.
(Yes, I know a few months passed.  But I don't have a lot of time to
work on Emacs bugs now.  This bug took me almost 8 hours over the past
few months.)

Basically, I took into consideration all the points Drew mentioned.  I'd
like to submit my work for review.  I have one technical problem,
though: I have it as a series of commits on a separate branch in my
repo, and I'm not Git-literate enough to make one patch from them.
Should I submit a series of patches?  Should I rebase my commits into
one big one?  (No big deal, this might be a good idea anyway.)

In the course of my work, I have also implemented a macro to help write
tests for the new behavior (there are 6 ERT tests, with 40 assertions
total).  Here it is (for review):

--8<---------------cut here---------------start------------->8---
(defvar elisp-test-point-marker-regex "=!\\([a-zA-Z0-9-]+\\)="
  "A regexp matching placeholders for point position for
`elisp-tests-with-temp-buffer'.")

;; Copied and heavily 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.  Special markers of
the form =!NAME= in CONTENTS are removed, and a for each one
a variable called NAME is bound to the position of such
a marker."
  (declare (indent 1) (debug t))
  `(with-temp-buffer
     (emacs-lisp-mode)
     (insert ,contents)
     (goto-char (point-min))
     (while (re-search-forward elisp-test-point-marker-regex nil t)
       (delete-region (match-beginning 0)
		      (match-end 0)))
     (goto-char (point-min))
     ,(let (marker-list)
	(with-temp-buffer
	  (insert (cond ((symbolp contents)
                         (symbol-value contents))
                        (t contents)))
	  (goto-char (point-min))
	  (while (re-search-forward elisp-test-point-marker-regex nil t)
	    (push (list (intern (match-string-no-properties 1))
			(match-beginning 0))
		  marker-list)
	    (delete-region (match-beginning 0)
			   (match-end 0))))
	`(let ,marker-list
	   ,@body))))
--8<---------------cut here---------------end--------------->8---

And it can be used like this:

--8<---------------cut here---------------start------------->8---
(defvar mark-defun-test-buffer
  ";; Comment header
=!before-1=
\(defun func-1 (arg)
  =!inside-1=\"docstring\"
  body)
=!after-1==!before-2=
;; Comment before a defun
\(d=!inside-2=efun func-2 (arg)
  \"docstring\"
  body)
=!after-2==!before-3=
\(defun func-3 (arg)
  \"docstring\"=!inside-3=
  body)
=!after-3==!before-4=(defun func-4 (arg)
  \"docstring\"=!inside-4=
  body)
=!after-4=
;; end
"
  "Test buffer for `mark-defun'.")

(ert-deftest mark-defun-no-arg-region-inactive ()
  "Test `mark-defun' with no prefix argument and inactive
region."
  (elisp-tests-with-temp-buffer
      mark-defun-test-buffer
    ;; mark-defun inside a defun, with comments and an empty line
    ;; before
    (goto-char inside-1)
    (mark-defun)
    (should (= (point) before-1))
    (should (= (mark) after-1))))
--8<---------------cut here---------------end--------------->8---

WDYT?

-- 
Marcin Borkowski

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

[Drew Adams]

> Basically, I took into consideration all the points Drew mentioned.  I'd
> like to submit my work for review.

Thanks for working on this, Marcin.

> I have one technical problem,
> though: I have it as a series of commits on a separate branch in my
> repo, and I'm not Git-literate enough to make one patch from them.

Hopefully someone else can help with that.

(For my part, if the code needed is small and you just include it here
then I will give it a try and let you know what I think (FWIW).)

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

[Marcin Borkowski]

On 2016-10-11, at 17:30, Drew Adams <drew.adams@oracle.com> wrote:

>> Basically, I took into consideration all the points Drew mentioned.  I'd
>> like to submit my work for review.
>
> Thanks for working on this, Marcin.

You're welcome.

BTW, as I probably already mentioned, while working on this, I found
a bug in end-of-defun, and until it's fixed, I have to use a silly
workaround.

>> I have one technical problem,
>> though: I have it as a series of commits on a separate branch in my
>> repo, and I'm not Git-literate enough to make one patch from them.
>
> Hopefully someone else can help with that.

I'd hope so!

> (For my part, if the code needed is small and you just include it here
> then I will give it a try and let you know what I think (FWIW).)

Here's the code (sans tests):

--8<---------------cut here---------------start------------->8---
(defun in-comment-line-p ()
  "Return non-nil if the point is in a comment line.
See http://lists.gnu.org/archive/html/help-gnu-emacs/2016-08/msg00141.html"
  (save-excursion
    (forward-line 0)
    (unless (looking-at "^\\s-*$")
      (< (line-end-position)
         (let ((ppss (syntax-ppss)))
           (when (nth 4 ppss)
             (goto-char (nth 8 ppss)))
           (forward-comment (point-max))
           (point))))))

(defun beginning-of-defun-comments (&optional arg)
  "Move to the beginning of ARGth defun, including comments."
  (interactive "^p")
  (unless arg (setq arg 1))
  (beginning-of-defun arg)
  (unless (bobp)
    (while (progn
             (forward-line -1)
             (in-comment-line-p)))
    (forward-line 1)))

(defun mark-defun (&optional arg)
  "Put mark at end of this defun, point at beginning.
The defun marked is the one that contains point or follows point.
With positive ARG, mark this and that many next defuns.

If (in Transient Mark mode) the mark is active, it marks the next
defun after the one(s) already marked.  With positive ARG, mark
that many more defuns.  With negative ARG, mark that many more
previous defuns."
  (interactive "p")
  (setq arg (or arg 1))
  (cond ((use-region-p)
         (if (>= arg 0)
             (set-mark
              (save-excursion
                (goto-char (mark))
                ;; change the dotimes below to (end-of-defun arg) once bug #24427 is fixed
                (dotimes (_ignore arg)
                  (end-of-defun))
                (point)))
           (beginning-of-defun-comments (- arg))))
        (t
         (let ((opoint (point))
               beg end)
           (push-mark opoint)
           ;; Try first in this order for the sake of languages with nested
           ;; functions where several can end at the same place as with the
           ;; offside rule, e.g. Python.
           (beginning-of-defun-comments)
           (setq beg (point))
           (end-of-defun)
           (setq end (point))
           (when (and (<= (point) opoint)
                      (> arg 0))
             ;; beginning-of-defun moved back one defun so we got the wrong
             ;; one.  If ARG < 0, however, we actually want to go back.
             (goto-char opoint)
             (end-of-defun)
             (setq end (point))
             (beginning-of-defun-comments)
             (setq beg (point)))
           (goto-char beg)
           (cond ((> arg 0)
                  ;; change the dotimes below to (end-of-defun arg) once bug #24427 is fixed
                  (dotimes (_ignore arg)
                    (end-of-defun))
                  (setq end (point))
                  (push-mark end nil t)
                  (goto-char beg))
                 (t
                  (goto-char beg)
                  (beginning-of-defun (1- (- arg)))
                  (push-mark end nil t))))))
  (while (progn
           (forward-line -1)
           (looking-at "^\\s-*$")))
  (forward-line 1))
--8<---------------cut here---------------end--------------->8---

Best,

-- 
Marcin Borkowski

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

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 491 bytes --]

On 2016-10-11 13:07, Marcin Borkowski wrote:
>>> >> I have one technical problem,
>>> >> though: I have it as a series of commits on a separate branch in my
>>> >> repo, and I'm not Git-literate enough to make one patch from them.
>> >
>> > Hopefully someone else can help with that.
> I'd hope so!

Would http://stackoverflow.com/questions/616556/how-do-you-squash-commits-into-one-patch-with-git-format-patch help?  The top answer is the one I'd recommend.

Cheers,
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Drew Adams]

BTW, IMO this should be removed: "(in Transient Mark Mode)".  (This is
not really related to this bug, however.)

Unless transient-mark-mode is enabled there is not even any notion of
the mark being "active".  That notion is valid only for t-m-m.  It
makes no sense to speak of an active mark or region, otherwise.

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

[Drew Adams]

1. Respect of the optional arg even for non-interactive use is an
   improvement, I think.

2. And it seems that no code distributed with Emacs calls `mark-defun'
   with the optional arg.  So no problem there.  And it is unlikely
   that 3rd-party code would call with the arg and expect it to be
   ignored (as was the case before).

3. With this at the top of *scratch* (note the blank line at top)
   and point between the comment and the defun, each of `M-- C-M-h'
   and `C-M-h' seems to loop indefinitely.

-------------8<----------------

;; This buffer is for notes you don't want to save, and for Lisp evaluation.
;; If you want to create a file, visit that file with C-x C-f,
;; then enter the text in that file's own buffer.

(defun a ()
  nil)
-------------8<----------------

4. And with the same thing, but without the blank line at the top,
   both `M-- C-M-h' and `C-M-h' select the defun plus the comment,
   except that they do not select the first comment line.  Intended?

5. `M-- C-M-h' and `C-M-h' always seem to select blank lines before
   the defun.  Should they (what for)?

6. Interactively, I would rather see repeated use of `C-M-h', after an
   initial use of `C-M-h' with a negative prefix arg (e.g. `M-- C-M-h'),
   continue to select defuns backward.  IOW, not need to use `M--'
   explicitly for each `C-M-h'.

   You can just hold down `C-M-h', to select multiple defuns forward.
   I would like to be able to do the same thing, but backward, by
   using `M-- C-M-h C-M-h C-M-h C-M-h...' (just hold down the chord).

   If you do that, then a negative prefix arg should not mean backward;
   it should just mean change direction (backward if previous command
   was not `mark-defun').

7. Someone will really need to test this with more than just Emacs Lisp.
   The comments talk about Python and nesting, etc.

HTH.

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

[Marcin Borkowski]

On 2016-10-11, at 23:15, Drew Adams <drew.adams@oracle.com> wrote:

> 1. Respect of the optional arg even for non-interactive use is an
>    improvement, I think.

Thanks.

> 2. And it seems that no code distributed with Emacs calls `mark-defun'
>    with the optional arg.  So no problem there.  And it is unlikely
>    that 3rd-party code would call with the arg and expect it to be
>    ignored (as was the case before).

I would guess so.

> 3. With this at the top of *scratch* (note the blank line at top)
>    and point between the comment and the defun, each of `M-- C-M-h'
>    and `C-M-h' seems to loop indefinitely.
>
> -------------8<----------------
>
> ;; This buffer is for notes you don't want to save, and for Lisp evaluation.
> ;; If you want to create a file, visit that file with C-x C-f,
> ;; then enter the text in that file's own buffer.
>
> (defun a ()
>   nil)
> -------------8<----------------

I'll investigate it, thanks for the report.

> 4. And with the same thing, but without the blank line at the top,
>    both `M-- C-M-h' and `C-M-h' select the defun plus the comment,
>    except that they do not select the first comment line.  Intended?

No, see above.

> 5. `M-- C-M-h' and `C-M-h' always seem to select blank lines before
>    the defun.  Should they (what for)?

I'm pretty sure that they should select blank lines either before or
after the defun.  I just chose the "before" way.

> 6. Interactively, I would rather see repeated use of `C-M-h', after an
>    initial use of `C-M-h' with a negative prefix arg (e.g. `M-- C-M-h'),
>    continue to select defuns backward.  IOW, not need to use `M--'
>    explicitly for each `C-M-h'.
>
>    You can just hold down `C-M-h', to select multiple defuns forward.
>    I would like to be able to do the same thing, but backward, by
>    using `M-- C-M-h C-M-h C-M-h C-M-h...' (just hold down the chord).
>
>    If you do that, then a negative prefix arg should not mean backward;
>    it should just mean change direction (backward if previous command
>    was not `mark-defun').

That's interesting.  I'd like to implement it, but this will take time
(apart from other things, I have a 10-days-old son now:-).)

> 7. Someone will really need to test this with more than just Emacs Lisp.
>    The comments talk about Python and nesting, etc.

Definitely.  I do not feel competent enough for that, though.  (I'm
pretty sure I've seen some ERT stuff for testing that with Python, I'll
check it.)

> HTH.

Yes, definitely - thanks a lot!

Best,

-- 
Marcin Borkowski

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

[Drew Adams]

> > 5. `M-- C-M-h' and `C-M-h' always seem to select blank lines
> >    before the defun.  Should they (what for)?
> 
> I'm pretty sure that they should select blank lines either before or
> after the defun.  I just chose the "before" way.

Yes, that is consistent with, say `mark-paragraph'.

> > 6. Interactively, I would rather see repeated use of `C-M-h',
> >    after an initial use of `C-M-h' with a negative prefix arg
> >    (e.g. `M-- C-M-h'), continue to select defuns backward.  IOW,
> >    not need to use `M--' explicitly for each `C-M-h'.
> >
> >    You can just hold down `C-M-h', to select multiple defuns
> >    forward.  I would like to be able to do the same thing, but
> >    backward, by using `M-- C-M-h C-M-h C-M-h C-M-h...' (just
> >    hold down the chord).
> >
> >    If you do that, then a negative prefix arg should not mean
> >    backward; it should just mean change direction (backward if
> >    previous command was not `mark-defun').
> 
> That's interesting.  I'd like to implement it, but this will take
> time (apart from other things, I have a 10-days-old son now:-).)

Thanks for considering it.  And congratulations on young Borkowski!

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

[Marcin Borkowski]

On 2016-10-11, at 23:15, Drew Adams <drew.adams@oracle.com> wrote:

> 3. With this at the top of *scratch* (note the blank line at top)
>    and point between the comment and the defun, each of `M-- C-M-h'
>    and `C-M-h' seems to loop indefinitely.
>
> -------------8<----------------
>
> ;; This buffer is for notes you don't want to save, and for Lisp evaluation.
> ;; If you want to create a file, visit that file with C-x C-f,
> ;; then enter the text in that file's own buffer.
>
> (defun a ()
>   nil)
> -------------8<----------------
>
> 4. And with the same thing, but without the blank line at the top,
>    both `M-- C-M-h' and `C-M-h' select the defun plus the comment,
>    except that they do not select the first comment line.  Intended?

Well, both these behaviors are manifestations of the same bug.  Below is
the corrected version.  (And below that a question.)

--8<---------------cut here---------------start------------->8---

(defun mark-defun (&optional arg)
  "Put mark at end of this defun, point at beginning.
The defun marked is the one that contains point or follows point.
With positive ARG, mark this and that many next defuns.

If the mark is active, it marks the next defun after the one(s)
already marked.  With positive ARG, mark that many more defuns.
With negative ARG, mark that many more previous defuns."
  (interactive "p")
  (setq arg (or arg 1))
  (cond ((use-region-p)
         (if (>= arg 0)
             (set-mark
              (save-excursion
                (goto-char (mark))
                ;; change the dotimes below to (end-of-defun arg) once bug #24427 is fixed
                (dotimes (_ignore arg)
                  (end-of-defun))
                (point)))
           (beginning-of-defun-comments (- arg))))
        (t
         (let ((opoint (point))
               beg end)
           (push-mark opoint)
           ;; Try first in this order for the sake of languages with nested
           ;; functions where several can end at the same place as with the
           ;; offside rule, e.g. Python.
           (beginning-of-defun-comments)
           (setq beg (point))
           (end-of-defun)
           (setq end (point))
           (when (and (<= (point) opoint)
                      (> arg 0))
             ;; beginning-of-defun moved back one defun so we got the wrong
             ;; one.  If ARG < 0, however, we actually want to go back.
             (goto-char opoint)
             (end-of-defun)
             (setq end (point))
             (beginning-of-defun-comments)
             (setq beg (point)))
           (goto-char beg)
           (cond ((> arg 0)
                  ;; change the dotimes below to (end-of-defun arg) once bug #24427 is fixed
                  (dotimes (_ignore arg)
                    (end-of-defun))
                  (setq end (point))
                  (push-mark end nil t)
                  (goto-char beg))
                 (t
                  (goto-char beg)
                  (beginning-of-defun (1- (- arg)))
                  (push-mark end nil t))))))
  (while (progn
           (forward-line -1)
           (and (looking-at "^\\s-*$")
                (not (bobp)))))
  (or (bobp) (forward-line 1)))
--8<---------------cut here---------------end--------------->8---

> 6. Interactively, I would rather see repeated use of `C-M-h', after an
>    initial use of `C-M-h' with a negative prefix arg (e.g. `M-- C-M-h'),
>    continue to select defuns backward.  IOW, not need to use `M--'
>    explicitly for each `C-M-h'.
>
>    You can just hold down `C-M-h', to select multiple defuns forward.
>    I would like to be able to do the same thing, but backward, by
>    using `M-- C-M-h C-M-h C-M-h C-M-h...' (just hold down the chord).
>
>    If you do that, then a negative prefix arg should not mean backward;
>    it should just mean change direction (backward if previous command
>    was not `mark-defun').

Just to be sure: you mean only the minus sign as argument, not
a negative number?  I'm also wondering whether to allow that for
non-interactive use, too: I'm pretty sure nobody would want to call
(mark-defun '-) from Lisp code, and it might make testing slightly
easier.

Best,

-- 
Marcin Borkowski

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

[Drew Adams]

> Well, both these behaviors are manifestations of the same bug.
> Below is the corrected version.  (And below that a question.)

The test cases I mentioned work now.  Thx.  I didn't try anything
beyond those cases.  Hopefully others will test a bit more.

> > 6. Interactively, I would rather see repeated use of `C-M-h',
> >    after an initial use of `C-M-h' with a negative prefix arg 
> >    (e.g. `M-- C-M-h'), continue to select defuns backward.
> >    IOW, not need to use `M--' explicitly for each `C-M-h'.
> >
> >    You can just hold down `C-M-h', to select multiple defuns
> >    forward.  I would like to be able to do the same thing,
> >    but backward, by using `M-- C-M-h C-M-h C-M-h C-M-h...'
> >    (just hold down the chord).
> >
> >    If you do that, then a negative prefix arg should not mean
> >    backward; it should just mean change direction (backward if
> >    previous command was not `mark-defun').
> 
> Just to be sure: you mean only the minus sign as argument, not
> a negative number?

No, not really.  But use your own judgment, I guess.

This is the kind of behavior I had in mind.  This is for
`transpose-sexps', but it shows the behavior.  _Any_ negative
arg flips the direction.  At the outset, a negative arg means
move backward.  The absolute value of ARG is the number of
sexps to move over.

(defun reversible-transpose-sexps (arg)
  "Reversible and repeatable `transpose-sexps'.
Like `transpose-sexps', but:
 1. Leaves point after the moved sexp.
 2. When repeated, a negative prefix arg flips the direction."
  (interactive "p")
  (when (eq last-command 'rev-transp-sexps-back) (setq arg  (- arg)))
  (transpose-sexps arg)
  (unless (natnump arg)
    (backward-sexp (abs arg))
    (skip-syntax-backward " .")
    (setq this-command  'rev-transp-sexps-back)))

(If you happen to try this with ARG=0, be aware that what you
see is just the peculiar `transpose-sexps' behavior for ARG=0.
This is not related to the code here.)

> I'm also wondering whether to allow that for
> non-interactive use, too: I'm pretty sure nobody would want to call
> (mark-defun '-) from Lisp code, and it might make testing slightly
> easier.

I think the behavior should be the same.  But see above.  The
arg passed should be numeric (positive, zero, or negative), IMO.

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

[Marcin Borkowski]

On 2016-10-28, at 16:32, Drew Adams <drew.adams@oracle.com> wrote:

> Thanks for considering it.  And congratulations on young Borkowski!

Thanks!

On 2016-11-02, at 19:25, Drew Adams <drew.adams@oracle.com> wrote:

>> Well, both these behaviors are manifestations of the same bug.
>> Below is the corrected version.  (And below that a question.)
>
> The test cases I mentioned work now.  Thx.  I didn't try anything
> beyond those cases.  Hopefully others will test a bit more.

I would hope so.

> No, not really.  But use your own judgment, I guess.

I guess your version is simpler (no need to use raw prefix arg), and
hence better.

> This is the kind of behavior I had in mind.  This is for
> `transpose-sexps', but it shows the behavior.  _Any_ negative
> arg flips the direction.  At the outset, a negative arg means
> move backward.  The absolute value of ARG is the number of
> sexps to move over.
>
> (defun reversible-transpose-sexps (arg)
>   "Reversible and repeatable `transpose-sexps'.
> Like `transpose-sexps', but:
>  1. Leaves point after the moved sexp.
>  2. When repeated, a negative prefix arg flips the direction."
>   (interactive "p")
>   (when (eq last-command 'rev-transp-sexps-back) (setq arg  (- arg)))
>   (transpose-sexps arg)
>   (unless (natnump arg)
>     (backward-sexp (abs arg))
>     (skip-syntax-backward " .")
>     (setq this-command  'rev-transp-sexps-back)))

Very nice trick with the 'last-command, thanks!  I included this in my
code.  I will also write some tests for that (it seems to work, but...)
and send the code soon.

>> I'm also wondering whether to allow that for
>> non-interactive use, too: I'm pretty sure nobody would want to call
>> (mark-defun '-) from Lisp code, and it might make testing slightly
>> easier.
>
> I think the behavior should be the same.  But see above.  The
> arg passed should be numeric (positive, zero, or negative), IMO.

Again - I agree, this makes coding (though not necessarily testing!)
simpler.

Best,

-- 
Marcin Borkowski

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

[Marcin Borkowski]

On 2016-11-04, at 08:48, Marcin Borkowski <mbork@mbork.pl> wrote:

> Very nice trick with the 'last-command, thanks!  I included this in my
> code.  I will also write some tests for that (it seems to work, but...)
> and send the code soon.

Hi Drew, hi all,

so it turned out that my code only seemed to work.  (The last-command
trick was fine, but what I thought corrected the bob bug only introduced
another one.)  Here's the (yet another) version.  (I'm not sharing my
tests yet - I haven't yet written any tests covering the last-command
trick.)  I'd be very thankful for any feedback (although I will have
even less time for Emacs bug work during the next two months - not only
my son needs considerable time and effort, the same goes for my
students;-).)

--8<---------------cut here---------------start------------->8---
(defun in-comment-line-p ()
  "Return non-nil if the point is in a comment line.
See http://lists.gnu.org/archive/html/help-gnu-emacs/2016-08/msg00141.html"
  (save-excursion
    (forward-line 0)
    (unless (looking-at "^\\s-*$")
      (< (line-end-position)
         (let ((ppss (syntax-ppss)))
           (when (nth 4 ppss)
             (goto-char (nth 8 ppss)))
           (forward-comment (point-max))
           (point))))))

(defun beginning-of-defun-comments (&optional arg)
  "Move to the beginning of ARGth defun, including comments."
  (interactive "^p")
  (unless arg (setq arg 1))
  (beginning-of-defun arg)
  (let (nbobp)
    (while (progn
             (setq nbobp (zerop (forward-line -1)))
             (and (in-comment-line-p)
                  nbobp)))
    (when nbobp
      (forward-line 1))))

(defun mark-defun (&optional arg)
  "Put mark at end of this defun, point at beginning.
The defun marked is the one that contains point or follows point.
With positive ARG, mark this and that many next defuns; with negative
ARG, change the direction of marking.

If the mark is active, it marks the next defun after the one(s)
already marked.  With positive ARG, mark that many more defuns.
With negative ARG, mark that many more previous defuns."
  (interactive "p")
  (setq arg (or arg 1))
  ;; Trick with 'mark-defun-back due to Drew Adams
  (when (eq last-command 'mark-defun-back)
    (setq arg (- arg)))
  (when (< arg 0)
    (setq this-command 'mark-defun-back))
  (cond ((use-region-p)
         (if (>= arg 0)
             (set-mark
              (save-excursion
                (goto-char (mark))
                ;; change the dotimes below to (end-of-defun arg) once bug #24427 is fixed
                (dotimes (_ignore arg)
                  (end-of-defun))
                (point)))
           (beginning-of-defun-comments (- arg))))
        (t
         (let ((opoint (point))
               beg end)
           (push-mark opoint)
           ;; Try first in this order for the sake of languages with nested
           ;; functions where several can end at the same place as with the
           ;; offside rule, e.g. Python.
           (beginning-of-defun-comments)
           (setq beg (point))
           (end-of-defun)
           (setq end (point))
           (when (or (and (<= (point) opoint)
                          (> arg 0))
                     (= beg (point-min))) ; we were before the first defun!
             ;; beginning-of-defun moved back one defun so we got the wrong
             ;; one.  If ARG < 0, however, we actually want to go back.
             (goto-char opoint)
             (end-of-defun)
             (setq end (point))
             (beginning-of-defun-comments)
             (setq beg (point)))
           (goto-char beg)
           (cond ((> arg 0)
                  ;; change the dotimes below to (end-of-defun arg) once bug #24427 is fixed
                  (dotimes (_ignore arg)
                    (end-of-defun))
                  (setq end (point))
                  (push-mark end nil t)
                  (goto-char beg))
                 (t
                  (goto-char beg)
                  (beginning-of-defun (1- (- arg)))
                  (push-mark end nil t))))))
  (let (nbobp)
    (while (progn
             (setq nbobp (zerop (forward-line -1)))
             (and (looking-at "^\\s-*$")
                  nbobp)))
    (when nbobp
      (forward-line 1))))
--8<---------------cut here---------------end--------------->8---

Best,

-- 
Marcin Borkowski

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

[Drew Adams]

Again, thanks for working on this.

> (defun in-comment-line-p ()
>   "Return non-nil if the point is in a comment line.
> See http://lists.gnu.org/archive/html/help-gnu-emacs/2016-
> 08/msg00141.html"

The second doc-string sentence should just be a comment in the
code, if it is needed at all.

> If the mark is active, it marks the next defun after the one(s)
> already marked.  With positive ARG, mark that many more defuns.

more -> next

>   ;; Trick with 'mark-defun-back due to Drew Adams

No need for the attribution. ;-)

I didn't really test, but for this:

(defun a ()
  nil)
(defun b ()
  nil)
;;;;
(defun c ()
  nil)

With point anywhere in either of the last two defuns or
on the comment line between them, `M-- C-M-h' selects not
only the expected defun but also the last line of the
defun before it.

E.g., with point at the beginning of the comment line,
this is selected:

  nil)
(defun a ()
  nil)

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

[Marcin Borkowski]

Hi Drew, hi all,

sorry for the delay - as I mentioned last time, I was going to have
a very busy semester, which now came to an end - and hence I'm back.

On 2016-11-27, at 19:51, Drew Adams <drew.adams@oracle.com> wrote:

> Again, thanks for working on this.
>
>> (defun in-comment-line-p ()
>>   "Return non-nil if the point is in a comment line.
>> See http://lists.gnu.org/archive/html/help-gnu-emacs/2016-
>> 08/msg00141.html"
>
> The second doc-string sentence should just be a comment in the
> code, if it is needed at all.

Done, thanks.

>
>> If the mark is active, it marks the next defun after the one(s)
>> already marked.  With positive ARG, mark that many more defuns.
>
> more -> next

Done, thanks.

>
>>   ;; Trick with 'mark-defun-back due to Drew Adams
>
> No need for the attribution. ;-)

Why not;-)?  I'll change it to a link to your message, however - this
might be actually more useful for future developers.

> I didn't really test, but for this:
>
> (defun a ()
>   nil)
> (defun b ()
>   nil)
> ;;;;
> (defun c ()
>   nil)
>
> With point anywhere in either of the last two defuns or
> on the comment line between them, `M-- C-M-h' selects not
> only the expected defun but also the last line of the
> defun before it.
>
> E.g., with point at the beginning of the comment line,
> this is selected:
>
>   nil)
> (defun a ()
>   nil)

Yep, you're right.  However, this seems to be a strange feature of
beginning-of-defun.  Place the point at the very same place at say M-:
(beginning-of-defun 0).  See?

Since I guess almost nobody follows this discussion anymore, I'll ask
about it in a separate thread on emacs-devel.  The question remains,
however: should I "fix" beginning-of-defun or just circumvent this
behavior in my code?

Best,

--
Marcin Borkowski

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

[Drew Adams]

> >>   ;; Trick with 'mark-defun-back due to Drew Adams
> >
> > No need for the attribution. ;-)
> 
> Why not;-)?  I'll change it to a link to your message, however - this
> might be actually more useful for future developers.

Yes, that's more useful; thx.

> > I didn't really test, but for this:
> >
> > (defun a ()
> >   nil)
> > (defun b ()
> >   nil)
> > ;;;;
> > (defun c ()
> >   nil)
> >
> > With point anywhere in either of the last two defuns or
> > on the comment line between them, `M-- C-M-h' selects not
> > only the expected defun but also the last line of the
> > defun before it.
> >
> > E.g., with point at the beginning of the comment line,
> > this is selected:
> >
> >   nil)
> > (defun a ()
> >   nil)
> 
> Yep, you're right.  However, this seems to be a strange feature of
> beginning-of-defun.  Place the point at the very same place at say M-:
> (beginning-of-defun 0).  See?
> 
> Since I guess almost nobody follows this discussion anymore, I'll ask
> about it in a separate thread on emacs-devel.  The question remains,
> however: should I "fix" beginning-of-defun or just circumvent this
> behavior in my code?

Yes, please pose the question for emacs-devel.  And thanks for
working on this bug.