Re: simplifying beginning-of-defun

[Andreas Roehler]

Stefan Monnier wrote:
>> simplifying forms as below should ease maintenance and speed up execution.
> 
> To what extent does it preserve compatibility?

Don't see anything incompatible for the moment. OTOH it will take some feasible 
bugs from progmodes.

It underlines a paradigm change, which was introduced
with var `beginning-of-defun-function' already.

These facility departs from all-at-once solutions,
which have been likely creating a bug in the back,
while solving one in the forehead.

With `beginning-of-defun-function',
`end-of-defun-function' python-mode for example

https://launchpad.net/python-mode

may set its own function and M-x beginning-of-defun then
will work still - which is not the case presently and my point of
depart here.


> 
> Apparently it makes beginning-of-defun-raw ignore
> beginning-of-defun-function, and it calls end-of-defun-function with one
> argument contrary to the current situation where it's called without
> any argument. 

An argument is useful here: as a repeat or specifier.

 So your code wouldn't be acceptable as is since it would
> likely break several packages.
> 
> Which performance problem is it trying to solve?

All which useless code execution causes.
Regard the lines of code saved that way to have an approximation.


> 
> The main difference I see between your beginning-of-defun and Emacs's
> one is that yours doesn't try to handle the case where
> beginning-of-defun-function, defun-prompt-regexp, and
> open-paren-in-column-0-is-defun-start and all nil.  This case was added
> fairly recently (Emacs-22, IIRC) after a long discussion. 

Gladly to see, discussions here seldom turn out that badly. :)

open-paren-in-column-0-is-defun-start is purely redundant, as the regexp may 
specify that
- and indeed does already(?) its just that what I read with "^\\s("


 I do not like
> this extra case at all, actually, but if you're trying to get rid of it,
> please make it a separate thread.
> 
> In other words, if you send new code just to simplify the existing one,
> than make sure the incompatibilities

Mentioned code of a end-of-defun-function in lisp.el is a bug.
Suggest to cancel it.

Let the -raw functions do everything needed for emacs-lisp.
Funcalls of beginning-of-defun-function, end-of-defun-function should be 
reserved for progmodes.

BTW if mode-specific, probably it should be introduced as a local var from the 
very beginning?

 are clearly understood and
> "minor". 

 Otherwise, better focus on the proposal for the change in
> behavior, and then accompany that with a patch showing how you suggest
> to implement this change.
> 
> 
>         Stefan
> 

Found a bug still in end-of-defun. Changed code below:



;; GNU's lisp.el
;; unhappily sets this var globally, ignoring its use for progmodes
(when (featurep 'emacs) (setq end-of-defun-function nil))

(setq defun-searchform '(if defun-prompt-regexp
                              (concat "^\\s(\\|"
                                      "\\(" defun-prompt-regexp "\\)\\s(")
                            "^\\s("))

(defun beginning-of-defun (&optional arg)
  "Move backward to the beginning of a functions definition. "
  (interactive "P")
  (or arg (setq arg 1))
  (if beginning-of-defun-function
      (funcall beginning-of-defun-function arg)
    (beginning-of-defun-raw arg)))

(defun beginning-of-defun-raw (&optional arg)
  "Called if progmodes didn't set beginning-of-defun-function. "
  (when
      (re-search-backward (eval defun-searchform) nil 'move (or arg 1))
    (goto-char (match-beginning 0))))

(defun end-of-defun (&optional arg)
  "Move backward to the end of a function. "
  (interactive "P")
  (or arg (setq arg 1))
  (if end-of-defun-function
      (funcall end-of-defun-function arg)
    (end-of-defun-raw arg)))

(defun end-of-defun-raw (&optional arg)
    "Called if progmodes didn't set end-of-defun-function. "
    (skip-chars-forward " \t\r\n\f")
  (when (looking-at (eval defun-searchform))
    (forward-char -1))
  (when (re-search-forward (eval defun-searchform) nil t arg)
  (goto-char (match-beginning 0))
  (forward-sexp 1)))

;;;;;;;;;;;