[Bashdb-devel] bashdb.el code review

[Masatake Y. <je...@gy...>]

(I've sent this mail privately to Rocky. However, I'm afraid
 that my mail is dropped by spam filter. So I'll send this here.)

I've read new bashdb.el code briefly.
Changes are much larger than I thought, so I give up detailed
review. I'll read deeply after bash4 is released.


Random some comments:

> ;;-----------------------------------------------------------------------------
> ;; ALB - redefinition of gud-reset for our own purposes

> (defvar bashdb--orig-gud-reset (symbol-function 'gud-reset))

> (defun gud-reset ()
>   "Redefinition of `gud-reset' to take care of bashdb cleanup."
>   (funcall bashdb--orig-gud-reset)
>   (dolist (buffer (buffer-list))
>     (when (string-match "\\*bashdb-[a-z]+\\*" (buffer-name buffer))
>       (let ((w (get-buffer-window buffer)))
>         (when w (delete-window w)))
>       (kill-buffer buffer))))


You should use defadvice. See advice.el if this is the first time
you hear defadvice. With defadvice the code can be rewritten like
this:

    (defadvice gud-reset (after bashdb-kill-buffer-and-window activate)
      "Cleanup bashdb related buffers and windows."

       ;; See how gdb-reset is called in gud.el::gud-sentinel.
       (when (or (memq gud-minor-mode-type '(bashdb))
		 (memq (buffer-local-value 'gud-minor-mode gud-comint-buffer)
		       '(bashdb)))
	  (bashdb-reset)))

    (defun bashdb-reset ()
       (dolist (buffer (buffer-list))
	(when (string-match "\\*bashdb-[a-z]+\\*" (buffer-name buffer))
	  (let ((w (get-buffer-window buffer)))
	    (when w (delete-window w)))
	  (kill-buffer buffer))))

I think bashdb-bashdbtrack-* should be moved to bashdb-track.el.
(Further more I think bashdb-track.el could be splited into comint-track.el(or comint-trace.el)
and bashdh-track.el.)


> (or (assq 'bashdb-bashdbtrack-is-tracking-p minor-mode-alist)
>     (push '(bashdb-bashdbtrack-is-tracking-p
> 	    bashdb-bashdbtrack-minor-mode-string)
> 	  minor-mode-alist))

On GNU Emacs, `define-minor-mode' is available.

Masatake YAMATO