CEDET

Hi Kai,

>>IMVHO it would be nicer for Senator to set the variable
>>beginning-of-defun-function instead of advising the function
>>beginning-of-defun.  If the variable is available, that is.
>>
>>WDYT?
> 
> 
> That's a good idea.  I'll look at it as soon as I can.

I just did the following patch to use the variables
`beginning/end-of-defun-function' when available, instead of advising
`beginning/end-of-defun' to use semantic navigation.

Could you please, apply it to your local copy of senator, use it
during some days, and tell me if you encounter problems?

Thanks for your help!

David

2004-11-23  David Ponce  <david@...>

	* cedet/semantic/senator.el

	(senator-minor-mode-setup): When possible, instead of advising,
	enable semantic navigation through the variables
	beginning/end-of-defun-function.
	(beginning-of-defun, end-of-defun-function): Don't advise if
	respectively the variables beginning/end-of-defun-function exist.

Index: senator.el
===================================================================
RCS file: /cvsroot/cedet/cedet/semantic/senator.el,v
retrieving revision 1.100
diff -c -r1.100 senator.el
*** senator.el	19 Jun 2004 15:00:27 -0000	1.100
--- senator.el	23 Nov 2004 08:39:52 -0000
***************
*** 2095,2101 ****
          (semantic-make-local-hook
           'semantic-after-partial-cache-change-hook)
          (add-hook 'semantic-after-partial-cache-change-hook
!                   'senator-completion-cache-flush-fcn nil t))
      ;; XEmacs needs this
      (if (featurep 'xemacs)
          (easy-menu-remove senator-minor-menu))
--- 2095,2109 ----
          (semantic-make-local-hook
           'semantic-after-partial-cache-change-hook)
          (add-hook 'semantic-after-partial-cache-change-hook
!                   'senator-completion-cache-flush-fcn nil t)
!         ;; Use semantic to navigate around functions
!         (and (boundp 'beginning-of-defun-function)
!              (set (make-local-variable 'beginning-of-defun-function)
!                   'senator-beginning-of-defun))
!         (and (boundp 'end-of-defun-function)
!              (set (make-local-variable 'end-of-defun-function)
!                   'senator-end-of-defun))
!         )
      ;; XEmacs needs this
      (if (featurep 'xemacs)
          (easy-menu-remove senator-minor-menu))
***************
*** 2105,2111 ****
      (remove-hook 'semantic-after-partial-cache-change-hook
                   'senator-completion-cache-flush-fcn t)
      ;; Disable semantic isearch
!     (setq senator-isearch-semantic-mode nil))
    senator-minor-mode)
  
  ;;;###autoload
--- 2113,2129 ----
      (remove-hook 'semantic-after-partial-cache-change-hook
                   'senator-completion-cache-flush-fcn t)
      ;; Disable semantic isearch
!     (setq senator-isearch-semantic-mode nil)
!     ;; Reset semantic navigation.
!     (and (boundp 'beginning-of-defun-function)
!          (local-variable-p 'beginning-of-defun-function)
!          (eq beginning-of-defun-function 'senator-beginning-of-defun)
!          (kill-local-variable 'beginning-of-defun-function))
!     (and (boundp 'end-of-defun-function)
!          (local-variable-p 'end-of-defun-function)
!          (eq end-of-defun-function 'senator-end-of-defun)
!          (kill-local-variable 'end-of-defun-function))
!     )
    senator-minor-mode)
  
  ;;;###autoload
***************
*** 2229,2247 ****
      (goto-char start) ;; beginning-of-defun
      (re-search-backward "^\n" (- (point) 1) t)))
  
! (defadvice beginning-of-defun (around senator activate)
!   "Move backward to the beginning of a defun.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p))
!       (senator-beginning-of-defun (ad-get-arg 0))
!     ad-do-it))
  
! (defadvice end-of-defun (around senator activate)
!   "Move forward to next end of defun.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p))
!       (senator-end-of-defun (ad-get-arg 0))
!     ad-do-it))
  
  (defadvice narrow-to-defun (around senator activate)
    "Make text outside current defun invisible.
--- 2247,2267 ----
      (goto-char start) ;; beginning-of-defun
      (re-search-backward "^\n" (- (point) 1) t)))
  
! (unless (boundp 'beginning-of-defun-function)
!   (defadvice beginning-of-defun (around senator activate)
!     "Move backward to the beginning of a defun.
  If semantic tags are available, use them to navigate."
!     (if (and senator-minor-mode (interactive-p))
!         (senator-beginning-of-defun (ad-get-arg 0))
!       ad-do-it)))
  
! (unless (boundp 'end-of-defun-function)
!   (defadvice end-of-defun (around senator activate)
!     "Move forward to next end of defun.
  If semantic tags are available, use them to navigate."
!     (if (and senator-minor-mode (interactive-p))
!         (senator-end-of-defun (ad-get-arg 0))
!       ad-do-it)))
  
  (defadvice narrow-to-defun (around senator activate)
    "Make text outside current defun invisible.

David PONCE <david.ponce@...> writes:

> Could you please, apply it to your local copy of senator, use it
> during some days, and tell me if you encounter problems?

I've just applied the patch, recompiled Senator, and will start to use
it now.

(The first few C-M-e and C-M-a keypresses were successful.)

Kai

Hi Kai,

>>Could you please, apply it to your local copy of senator, use it
>>during some days, and tell me if you encounter problems?
> 
> 
> I've just applied the patch, recompiled Senator, and will start to use
> it now.
> 
> (The first few C-M-e and C-M-a keypresses were successful.)

With my patch applied, I found a problem with the lisp indent engine
that depends on the behavior of the standard `beginning-of-defun'
function.

Here is a new patch to fix that.  I introduced the new option
`senator-preserved-modes' to customize the major modes where to
preserve the standard behavior of the `beginning-of-defun',
`end-of-defun', `narrow-to-defun', and `mark-defun' functions, when
`senator-minor-mode' is enabled.  By default the standard behavior is
preserved in lisp-like modes.

Could you please apply it to the current CVS version of senator.el and
tell me if you notice something wrong?

Eric, any thoughts on this patch?  If you agree I will commit it.

Thanks!
David

Index: senator.el
===================================================================
RCS file: /cvsroot/cedet/cedet/semantic/senator.el,v
retrieving revision 1.100
diff -c -r1.100 senator.el
*** senator.el	19 Jun 2004 15:00:27 -0000	1.100
--- senator.el	26 Nov 2004 13:53:49 -0000
***************
*** 2071,2076 ****
--- 2071,2089 ----
                      ""))))
    (semantic-mode-line-update))
  
+ (defcustom senator-preserved-modes
+   '(lisp-mode emacs-lisp-mode common-lisp-mode lisp-interaction-mode)
+   "*Major modes where to preserve standard functions.
+ That is where to use the default `beginning-of-defun', `end-of-defun',
+ `narrow-to-defun', and `mark-defun' functions."
+   :group 'senator
+   :type '(repeat (symbol)))
+ 
+ (defsubst senator-override-standard-functions-p ()
+   "Return non-nil if it is allowed to override standard functions.
+ See also `senator-preserved-modes'."
+   (not (memq major-mode senator-preserved-modes)))
+ 
  (defun senator-minor-mode-setup ()
    "Actually setup the senator minor mode.
  The minor mode can be turned on only if semantic feature is available
***************
*** 2095,2101 ****
          (semantic-make-local-hook
           'semantic-after-partial-cache-change-hook)
          (add-hook 'semantic-after-partial-cache-change-hook
!                   'senator-completion-cache-flush-fcn nil t))
      ;; XEmacs needs this
      (if (featurep 'xemacs)
          (easy-menu-remove senator-minor-menu))
--- 2108,2123 ----
          (semantic-make-local-hook
           'semantic-after-partial-cache-change-hook)
          (add-hook 'semantic-after-partial-cache-change-hook
!                   'senator-completion-cache-flush-fcn nil t)
!         (when (senator-override-standard-functions-p)
!           ;; Use semantic to navigate around functions
!           (and (boundp 'beginning-of-defun-function)
!                (set (make-local-variable 'beginning-of-defun-function)
!                     'senator-beginning-of-defun))
!           (and (boundp 'end-of-defun-function)
!                (set (make-local-variable 'end-of-defun-function)
!                     'senator-end-of-defun)))
!         )
      ;; XEmacs needs this
      (if (featurep 'xemacs)
          (easy-menu-remove senator-minor-menu))
***************
*** 2105,2111 ****
      (remove-hook 'semantic-after-partial-cache-change-hook
                   'senator-completion-cache-flush-fcn t)
      ;; Disable semantic isearch
!     (setq senator-isearch-semantic-mode nil))
    senator-minor-mode)
  
  ;;;###autoload
--- 2127,2143 ----
      (remove-hook 'semantic-after-partial-cache-change-hook
                   'senator-completion-cache-flush-fcn t)
      ;; Disable semantic isearch
!     (setq senator-isearch-semantic-mode nil)
!     ;; Reset semantic navigation.
!     (and (boundp 'beginning-of-defun-function)
!          (local-variable-p 'beginning-of-defun-function)
!          (eq beginning-of-defun-function 'senator-beginning-of-defun)
!          (kill-local-variable 'beginning-of-defun-function))
!     (and (boundp 'end-of-defun-function)
!          (local-variable-p 'end-of-defun-function)
!          (eq end-of-defun-function 'senator-end-of-defun)
!          (kill-local-variable 'end-of-defun-function))
!     )
    senator-minor-mode)
  
  ;;;###autoload
***************
*** 2229,2253 ****
      (goto-char start) ;; beginning-of-defun
      (re-search-backward "^\n" (- (point) 1) t)))
  
! (defadvice beginning-of-defun (around senator activate)
!   "Move backward to the beginning of a defun.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p))
!       (senator-beginning-of-defun (ad-get-arg 0))
!     ad-do-it))
  
! (defadvice end-of-defun (around senator activate)
!   "Move forward to next end of defun.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p))
!       (senator-end-of-defun (ad-get-arg 0))
!     ad-do-it))
  
  (defadvice narrow-to-defun (around senator activate)
    "Make text outside current defun invisible.
  The defun visible is the one that contains point or follows point.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p))
        (senator-narrow-to-defun)
      ad-do-it))
  
--- 2261,2290 ----
      (goto-char start) ;; beginning-of-defun
      (re-search-backward "^\n" (- (point) 1) t)))
  
! (unless (boundp 'beginning-of-defun-function)
!   (defadvice beginning-of-defun (around senator activate)
!     "Move backward to the beginning of a defun.
  If semantic tags are available, use them to navigate."
!     (if (and senator-minor-mode (interactive-p)
!              (senator-override-standard-functions-p))
!         (senator-beginning-of-defun (ad-get-arg 0))
!       ad-do-it)))
  
! (unless (boundp 'end-of-defun-function)
!   (defadvice end-of-defun (around senator activate)
!     "Move forward to next end of defun.
  If semantic tags are available, use them to navigate."
!     (if (and senator-minor-mode (interactive-p)
!              (senator-override-standard-functions-p))
!         (senator-end-of-defun (ad-get-arg 0))
!       ad-do-it)))
  
  (defadvice narrow-to-defun (around senator activate)
    "Make text outside current defun invisible.
  The defun visible is the one that contains point or follows point.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p)
!            (senator-override-standard-functions-p))
        (senator-narrow-to-defun)
      ad-do-it))
  
***************
*** 2255,2261 ****
    "Put mark at end of this defun, point at beginning.
  The defun marked is the one that contains point or follows point.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p))
        (senator-mark-defun)
      ad-do-it))
  
--- 2292,2299 ----
    "Put mark at end of this defun, point at beginning.
  The defun marked is the one that contains point or follows point.
  If semantic tags are available, use them to navigate."
!   (if (and senator-minor-mode (interactive-p)
!            (senator-override-standard-functions-p))
        (senator-mark-defun)
      ad-do-it))

David PONCE <david.ponce@...> writes:

> With my patch applied, I found a problem with the lisp indent engine
> that depends on the behavior of the standard `beginning-of-defun'
> function.

How do the behaviors differ?  Is it possible to have the Senator
functions move to the same spot as the standard Lisp functions?

(Except for the bogus matches that the standard functions find;
presumably the lisp indent engine does not rely on the bogus matches.)

But I haven't looked at what's really happening.

Kai

>>With my patch applied, I found a problem with the lisp indent engine
>>that depends on the behavior of the standard `beginning-of-defun'
>>function.
> 
> 
> How do the behaviors differ?  Is it possible to have the Senator
> functions move to the same spot as the standard Lisp functions?
> 
> (Except for the bogus matches that the standard functions find;
> presumably the lisp indent engine does not rely on the bogus matches.)

The standard beginning-of-defun function considers that a lisp
function begins at an open parenthesis encountered at beginning of
line.

The senator version does not depend on the open parenthesis at
beginning of line.

The indentation problem occurs in code like this:

(eval-when-compile
 
  (defun foo ()
    )

  (defun bar ()
 -|-)

) 

Suppose the point is where indicated by -|-, the standard
beginning-of-defun moves it at the beginning of the
"(eval-when-compile" line. The senator version moves it at the
beginning of the "(defun bar ()" line.

David

David PONCE <david.ponce@...> writes:

>>>With my patch applied, I found a problem with the lisp indent engine
>>>that depends on the behavior of the standard `beginning-of-defun'
>>>function.
>> 
>> 
>> How do the behaviors differ?  Is it possible to have the Senator
>> functions move to the same spot as the standard Lisp functions?
>> 
>> (Except for the bogus matches that the standard functions find;
>> presumably the lisp indent engine does not rely on the bogus matches.)
>
> The standard beginning-of-defun function considers that a lisp
> function begins at an open parenthesis encountered at beginning of
> line.
>
> The senator version does not depend on the open parenthesis at
> beginning of line.

Ah, and you're saying that the Emacs Lisp indentation code depends on
this behavior?  Hm.

Kai