Re: [Bashdb-devel] bashdb.el code review

[Rocky B. <roc...@gm...>]

Thanks for the comments. Yes, your message in fact was classified as
"trash". So thanks for not letting this slip and posting.

On Nov 16, 2007 12:23 AM, Masatake YAMATO <je...@gy...> wrote:

> (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
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Bashdb-devel mailing list
> Bas...@li...
> https://lists.sourceforge.net/lists/listinfo/bashdb-devel
>