[V] Re: beginning-of-defun bug

[Steve Youngs]

* Andreas Roehler <andreas.roehler at online.de> writes:

  > Steve Youngs wrote:
  >> VETO
  >> 
  >> * Andreas Roehler <andreas.roehler at online.de> writes:
  >> 
  >> > FOR REVIEW
  >> > `beginning-of-defun' contains a bug sourcing redundancy.
  >> 
  >> > With `beginning-of-defun' basically are to discriminate two cases:
  >> > if `beginning-of-defun-function' is set or not.
  >> 
  >> > If not set, `beginning-of-defun-raw' is called, which
  >> > provides for an arguments default:
  >> 
  >> > (unless arg (setq arg 1))
  >> 
  >> > That's useful for the default routine, but not for
  >> > beginning-of-defun-function.
  >> 
  >> > Python-mode for example sets `beginning-of-defun-function'
  >> >  to `py-beginning-of-def-or-class'
  >> 
  >> > (py-beginning-of-def-or-class &optional class count)
  >> 
  >> > As seen, its first argument, class, now will provided
  >> > with default `1', which causes
  >> > py-beginning-of-def-or-class not to look for the
  >> > beginning of a function, but for a class-definition.
  >> 
  >> > That's a bug.
  >> 
  >> In my eyes, the bug is in #'py-beginning-of-def-or-class, in that it
  >> has its arguments backwards. 

  > No. At the contrary. It's vain, saying from the core side: we know
  > best how the prog-modes functions must define and use it's arguments.

We do when those arguments are being used in calls to core functions.

BTW, when are you planning to submit a similar patch to GNU/Emacs?  I
just looked and their #'beginning-of-defun-raw has your "bug" too.

  > If you know a reason to fill in an argument for
  > beginning-of-defun-function at this place in advance, I'm listening.

The function takes the same argument as `beginning-of-defun' and should
behave similarly, returning non-nil if it found the beginning of a defun.
Ideally it should move to a point right before an open-paren which encloses
the body of the defun.

That is the 2nd paragraph to the doc string of
beginning-of-defun-function in GNU/Emacs.  You know, the one you left
off when you submitted it to us.

  > Behaviour of XEmacs for the rest won't change AFAIS.

  > OTOH changing the python-mode, what you are asking for, will break
  > things there and change XEmacs behavior finally.

  > If you don't want to change this wrongly set specification,
  > we can't use beginning-of-defun-function in python-mode for example.

I don't believe that it is wrong.  Ugly, perhaps, but not wrong.

When I suggested that #'py-beginning-of-def-or-class was at fault, it
was an educated guess.  Now that I have looked at both XEmacs and
GNU/Emacs #'beginning-of-defun I'm convinced that I'm right.

  > So please reflect your veto.

I really can't, I'm sorry, Andreas.  However, I won't get in the way of
another reviewer overriding the veto.

  > Thanks anyway BTW and for contributing FFI especially.

The real heroes are my great team at SXEmacs, especially Evgeny Zajcev,
Sebastian Freundt, and Nelson Ferreira.  All I did was to port their
brilliant work from SXEmacs to XEmacs.

  >> > Moved `beginning-of-defun-function' upwards in function and
  >> > changed "_p" in `beginning-of-defun' into "P".
  >> 
  >> This has got to be wrong.  Think of the use case... `C-u C-M-a'
  >> 

  > Thats a typo in this mail, not in code.

And here is your code...

diff -r a46d7513a364 lisp/lisp.el
--- a/lisp/lisp.el	Wed Oct 07 19:42:42 2009 +0200
+++ b/lisp/lisp.el	Wed Oct 07 20:56:56 2009 +0200
@@ -183,9 +183,11 @@ non-nil, then a string which matches tha
 non-nil, then a string which matches that regexp may precede the
 open-parenthesis, and point ends up at the beginning of the line."
   ;; XEmacs change (for zmacs regions)
-  (interactive "_p")
-  (and (beginning-of-defun-raw arg)
-       (progn (beginning-of-line) t)))
+  (interactive "_P")
+  (if beginning-of-defun-function
+      (funcall beginning-of-defun-function arg)
+    (and (beginning-of-defun-raw arg)
+         (progn (beginning-of-line) t))))

Is it a typo there too?



-- 
|---<Steve Youngs>---------------<GnuPG KeyID: A94B3003>---|
|       SXEmacs - The only _______ you'll ever need.       |
|         Fill in the blank, yes, it's THAT good!          |
|------------------------------------<steve at sxemacs.org>---|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 317 bytes
Desc: not available
Url : http://calypso.tux.org/pipermail/xemacs-patches/attachments/20091008/30094dfc/attachment-0003.bin