GNU bug report logs - #76789
31.0.50; [PATCH] speedbar: New speedbar-window-mode

Previous Next

Full log

Message #26 received at 76789 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76789 <at> debbugs.gnu.org
Subject: Re: bug#76789: 31.0.50; [PATCH] speedbar: New speedbar-window-mode
Date: Tue, 11 Mar 2025 16:41:22 +0100

[Message part 1 (text/plain, inline)]

Thank you Eli. 
In this updated version of the patch I corrected what you indicated and added 
something in the manual, rather short :-(.

Thanks.
Vincenzo

In data domenica 9 marzo 2025 07:53:50 Ora standard dell’Europa centrale, Eli 
Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 76789 <at> debbugs.gnu.org
> > Date: Sat, 08 Mar 2025 23:06:13 +0100
> > 
> > > Speedbar has its own manual.  Did you consider updating that manual
> > > with this new feature?
> > 
> > I wrote something in speedbar.texi (not included in this patch), but I
> > also
> > saw that the emacs manual has section 18.9 Speedbar Frame (chapter 18
> > Frames and Graphical Display), and I am not sure how to edit it.
> 
> Just add short text there saying that speedbar can optionally be
> displayed as a window, not a frame.  That section has a reference to
> the Speedbar manual, so the details are covered by that.
> 
> Thanks.  Please see a few more comments below.
> 
> > +*** The new command 'speedbar-window-mode' open Speedbar in a window
> > instead +of a frame.                                ^^^^
> 
> "opens"
> 
> > +*** New alias 'speedbar-window' is an alias for 'speedbar-window-mode'.
> 
> "New command 'speedbar-window' is an alias for 'speedbar-window-mode'."
> 
> > +*** The new user option 'speedbar-prefer-window', tell 'speedbar' to open
> > +a side window instead of a frame.                 ^^^^
> 
> "tells"
> 
> > +(defcustom speedbar-prefer-window nil
> > +  "If t, the command `speedbar' open the speedbar in a window."
> 
>                                    ^^^^
> "opens"
> 
> > +(defcustom speedbar-window-dedicated-window t
> > +  "Make the `speedbar-window' dedicated."
> 
> "Whether to make the `speedbar-window' dedicated."
> 
> > +  :group 'speedbar
> > +  :type 'boolean
> > +  :version "31.1")
> > +
> > +(defcustom speedbar-window-side 'left
> > +  "Show the `speedbar-window' on the `left', `right', `top' or `bottom'.
> > +See `display-buffer-in-side-window' for more details."
> 
> Our style is to make the first line of the doc string a kind of
> summary:
> 
>     "Control the side of the frame on which to show the speedbar window.
>   The value can be `left', `right', `top' or `bottom'.
>   See `display-buffer-in-side-window' for more details."
> 
> > +(defcustom speedbar-window-default-width 20
> > +  "Initial width in characters of `speedbar-window' under window system.
> 
> Why "under window system"?  Doesn't this work on TTY frames in the
> same way?
> 
> > +(defcustom speedbar-window-max-width 40
> > +  "The maximum allowed width in characters of the `speedbar-window'."
> 
> This begs the question: what happens with wider items?  I suggest to
> tell that in the doc string.
> 
> > +(defun speedbar-easymenu-definition-trailer ()
> > +  "Menu items appearing at the end of the speedbar menu."
> 
> I guess you meant "Return menu items appearing..."?
> 
> > +A nil ARG means toggle.  If `speedbar-prefer-window' is t, open the
> > +speedbar in a window istead of in a frame."
> 
>                         ^^^^^^^^^^^^^^^^^^^^
> "...instead of in a separate frame" is better here, because any Emacs
> display is always "in a frame", even if it's in some window.
> 
> > +(defun speedbar-is-frame-or-window-p ()
> 
> This function is not a predicate, since its value is not a boolean.
> So its name should be something like speedbar-frame-or-window, without
> "is" and without "-p".
> 
> > +(defun speedbar-window-mode (&optional arg)
> > +  "Enable or disable speedbar window.
> 
> I suggest
> 
>   Enable or disable speedbar window mode.
> 
> or
> 
>   "Enable or disable speedbar display in a separate window.
> 
> > +(defsubst speedbar-window--window-live-p ()
> > +    "Return non-nil if  `speedbar--window' is defined and live."
> 
>                          ^^
> Excess whitespace there.
> 
> > +(defsubst speedbar-window--buffer-live-p ()
> > +    "Return non-nil `speedbar-buffer' is defined and live."
> 
>                       ^^
> "if" is missing there.  Also, what do you mean by "buffer is defined"?
> I suggest to remove it and leave only "buffer is live".
> 
> > +(defun speedbar-window--live-p ()
> > +  "Return t if `speedbar-window' is opened."
> 
>                                     ^^^^^^^^^
> I suggest "is live" or "is displayed".
> 
> > +(defsubst speedbar-window-current-window ()
> > +  "Return t if the current windows is the `speedbar--window'."
> 
> "Return t if the selected window is the `speedbar--window'."
> 
> > +(defsubst speedbar-window--width ()
> > +  "Return the width of `speedbar-window' WINDOW."
> 
>                                             ^^^^^^
> This function has no argument named WINDOW.
> 
> > +(defun speedbar-width ()
> > +  "Returns the width of the `speedbar'.
> 
>       ^^^^^^^
> "Return"
> 
> > +(defun speedbar--speedbar-live-p ()
> > +  "Return non-nil if `speedbar-window-mode' or `speedbar-frame-mode' are
> > open."
>                                                                        
> ^^^^^^^^^
> 
> "are active", I think?

[0001-Add-a-new-command-speedbar-window.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.