bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

Drew Adams
I don't understand the doc string:

 Toggle menu bar on or off, based on the status of the current frame.
 See `menu-bar-mode' for more information.

I have no idea what is meant by "based on the status of the current
frame".  Is the status whether it shows the menu-bar?

Furthermore, this seems to be used only in menu Options > Show/Hide.
Which means it's only visible when the menu-bar is visible, right?

Sorry, but I just don't get the point of this command, e.g., compared
to command `menu-bar-mode'.

What's more, the doc string tells you to see the doc string of
`menu-bar-mode'.  But that doesn't explain anything more about
`toggle-menu-bar-mode-from-frame', e.g., how it differs from
`menu-bar-mode'.  It's not even mentioned there.

What is this command - what's its use case?  And can the doc string be
improved?

In GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)
 of 2019-08-29
Repository revision: 96dd0196c28bc36779584e47fffcca433c9309cd
Windowing system distributor `Microsoft Corp.', version 10.0.18362
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''



Reply | Threaded
Open this post in threaded view
|

bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

Lars Ingebrigtsen
Drew Adams <[hidden email]> writes:

> I don't understand the doc string:
>
>  Toggle menu bar on or off, based on the status of the current frame.
>  See `menu-bar-mode' for more information.
>
> I have no idea what is meant by "based on the status of the current
> frame".  Is the status whether it shows the menu-bar?
>
> Furthermore, this seems to be used only in menu Options > Show/Hide.
> Which means it's only visible when the menu-bar is visible, right?

It's very confusing indeed, but the patch that introduces this has a
long essay about why it was added.

It's something to do with...  multi-tty?  Which clarifies the meaning of
"Toggle menu bar on or off, based on the status of the current frame."
somewhat, since the exegesis says that the (global) status may be...
wrong?

That sounds like a bug, doesn't it?

commit 6a5af08f8d3bb831a78edf17e9c38813bbfbb6f8
Author:     Karoly Lorentey <[hidden email]>
AuthorDate: Sun Mar 26 14:20:01 2006 +0000
Commit:     Karoly Lorentey <[hidden email]>
CommitDate: Sun Mar 26 14:20:01 2006 +0000

    Fix tool-bar and menu-bar toggles in Show/Hide menu to reflect the state of the current frame.
   
    * lisp/tool-bar.el (toggle-tool-bar-mode-from-frame): New function.
   
    * lisp/menu-bar.el (toggle-menu-bar-mode-from-frame): New function.
      (menu-bar-showhide-menu): Use them to change "Menu-bar" and
      "Tool-bar" toggles to reflect the state of the current frame.
   
    git-archimport-id: [hidden email]--2004/emacs--multi-tty--0--patch-537

diff --git a/README.multi-tty b/README.multi-tty
index 2ac27bafe5..442c4c92fd 100644
--- a/README.multi-tty
+++ b/README.multi-tty
@@ -404,17 +404,6 @@ is probably not very interesting for anyone else.)
 THINGS TO DO
 ------------
 
-** `tool-bar-mode', `scroll-bar-mode', `menu-bar-mode' and
-   'fringe-mode' are modes global to the entire Emacs session, not
-   just a single frame or a single terminal.  This means that their
-   status sometimes differs from what's actually displayed on the
-   screen.  As a consequence, the Options | Show/Hide menu sometimes
-   shows incorrect status, and you have to select an option twice for
-   it to have any visible effect on the current frame.
-
-   Change Emacs so that the status of the items in the Options |
-   Show/Hide menu correspond to the current frame.
-
 ** emacsclient -t on the console does not work after su:
 
  # su lorentey
@@ -1485,5 +1474,18 @@ DIARY OF CHANGES
 
    (Done in patch-500.)
 
+-- `tool-bar-mode', `scroll-bar-mode', `menu-bar-mode' and
+   'fringe-mode' are modes global to the entire Emacs session, not
+   just a single frame or a single terminal.  This means that their
+   status sometimes differs from what's actually displayed on the
+   screen.  As a consequence, the Options | Show/Hide menu sometimes
+   shows incorrect status, and you have to select an option twice for
+   it to have any visible effect on the current frame.
+
+   Change Emacs so that the status of the items in the Options |
+   Show/Hide menu correspond to the current frame.
+
+   (Done in patch-537.)
+
 ;;; arch-tag: 8da1619e-2e79-41a8-9ac9-a0485daad17d
 
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 75d68d0baa..a9987d589f 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -935,15 +935,15 @@ menu-bar-showhide-menu
  :button `(:toggle . tooltip-mode)))
 
 (define-key menu-bar-showhide-menu [menu-bar-mode]
-  '(menu-item "Menu-bar" menu-bar-mode
+  '(menu-item "Menu-bar" toggle-menu-bar-mode-from-frame
       :help "Toggle menu-bar on/off"
-      :button (:toggle . menu-bar-mode)))
+      :button (:toggle . (> (frame-parameter nil 'menu-bar-lines) 0))))
 
 (define-key menu-bar-showhide-menu [showhide-tool-bar]
-  (list 'menu-item "Tool-bar" 'tool-bar-mode
- :help "Turn tool-bar on/off"
+  (list 'menu-item "Tool-bar" 'toggle-tool-bar-mode-from-frame
+ :help "Toggle tool-bar on/off"
  :visible `(display-graphic-p)
- :button `(:toggle . tool-bar-mode)))
+ :button `(:toggle . (> (frame-parameter nil 'tool-bar-lines) 0))))
 
 (define-key menu-bar-options-menu [showhide]
   (list 'menu-item "Show/Hide" menu-bar-showhide-menu
@@ -1778,6 +1778,14 @@ menu-bar-mode
  "Menu-bar mode disabled.  Use M-x menu-bar-mode to make the menu bar appear."))
   menu-bar-mode)
 
+(defun toggle-menu-bar-mode-from-frame (&optional arg)
+  "Toggle menu bar on or off, based on the status of the current frame.
+See `menu-bar-mode' for more information."
+  (interactive (list (or current-prefix-arg 'toggle)))
+  (if (eq arg 'toggle)
+      (menu-bar-mode (if (> (frame-parameter nil 'menu-bar-lines) 0) 0 1))
+    (menu-bar-mode arg)))
+
 (provide 'menu-bar)
 
 ;;; arch-tag: 6e6a3c22-4ec4-4d3d-8190-583f8ef94ced
diff --git a/lisp/tool-bar.el b/lisp/tool-bar.el
index be552854b1..023b90af5d 100644
--- a/lisp/tool-bar.el
+++ b/lisp/tool-bar.el
@@ -61,6 +61,16 @@ tool-bar-mode
  (display-graphic-p))
    (tool-bar-setup))))
 
+;;;###autoload
+;; Used in the Show/Hide menu, to have the toggle reflect the current frame.
+(defun toggle-tool-bar-mode-from-frame (&optional arg)
+  "Toggle tool bar on or off, based on the status of the current frame.
+See `tool-bar-mode' for more information."
+  (interactive (list (or current-prefix-arg 'toggle)))
+  (if (eq arg 'toggle)
+      (tool-bar-mode (if (> (frame-parameter nil 'tool-bar-lines) 0) 0 1))
+    (tool-bar-mode arg)))
+
 ;;;###autoload
 ;; We want to pretend the toolbar by standard is on, as this will make
 ;; customize consider disabling the toolbar a customization, and save


--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



Reply | Threaded
Open this post in threaded view
|

bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

Drew Adams
> > I don't understand the doc string:
> >
> >  Toggle menu bar on or off, based on the status of the current frame.
> >  See `menu-bar-mode' for more information.
> >
> > I have no idea what is meant by "based on the status of the current
> > frame".  Is the status whether it shows the menu-bar?
> >
> > Furthermore, this seems to be used only in menu Options > Show/Hide.
> > Which means it's only visible when the menu-bar is visible, right?
>
> It's very confusing indeed, but the patch that introduces this has a
> long essay about why it was added.
>
> It's something to do with...  multi-tty?  Which clarifies the meaning of
> "Toggle menu bar on or off, based on the status of the current frame."
> somewhat, since the exegesis says that the (global) status may be...
> wrong?
>
> That sounds like a bug, doesn't it?

Sorry, but I still don't understand it.  I don't
really care about this, so I'll leave it up to
others (e.g. you, if you want) to figure out what,
if anything, should be done about it.

I suspect that even the command name is bad.

----

FWIW, I have a similarly named command, but it does
what I'd expect a command with such a name to do:

(defun toggle-menu-bar-for-frame (&optional frame)
  "Toggle showing the menu-bar for selected frame.
When called from Lisp, optional arg FRAME is used instead of the
selected frame."
  (interactive)
  (let ((now  (or (frame-parameter frame 'menu-bar-lines)  0)))
    (set-frame-parameter frame 'menu-bar-lines (if (zerop now) 1 0))))

IOW, instead of toggling all existing and future frames,
which is what `menu-bar-mode' does, it toggles only the
selected frame.



Reply | Threaded
Open this post in threaded view
|

bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

Eli Zaretskii
In reply to this post by Drew Adams
> Date: Sun, 13 Sep 2020 20:58:16 +0000 (UTC)
> From: Drew Adams <[hidden email]>
>
> I don't understand the doc string:
>
>  Toggle menu bar on or off, based on the status of the current frame.
>  See `menu-bar-mode' for more information.

Yes, the doc string "needs work".

> What's more, the doc string tells you to see the doc string of
> `menu-bar-mode'.  But that doesn't explain anything more about
> `toggle-menu-bar-mode-from-frame', e.g., how it differs from
> `menu-bar-mode'.  It's not even mentioned there.
>
> What is this command - what's its use case?

If you set the menu-bar-lines parameter of a frame to zero, but don't
turn off menu-bar-mode, the menu bar will not be displayed.  In that
situation, "M-x menu-bar-mode RET" will do the exact opposite of what
you want, whereas this command, when invoked from the menu popped up
by C-mouse-3, will do what you expect.  That's the use case.



Reply | Threaded
Open this post in threaded view
|

bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

Eli Zaretskii
In reply to this post by Lars Ingebrigtsen
> From: Lars Ingebrigtsen <[hidden email]>
> Date: Mon, 14 Sep 2020 01:30:03 +0200
> Cc: [hidden email]
>
> It's something to do with...  multi-tty?  Which clarifies the meaning of
> "Toggle menu bar on or off, based on the status of the current frame."
> somewhat, since the exegesis says that the (global) status may be...
> wrong?
>
> That sounds like a bug, doesn't it?

I think this command indeed has a bug, in that it cannot disable the
menu bar only on the frame from which it is invoked.  It _can_ enable
the menu bar on a single frame, see my response to Drew.  So I think
this command needs to be improved.  But that's another issue; the doc
string definitely needs to be improved.  Will you do that, or should
I?

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#43383: 26.3; `toggle-menu-bar-mode-from-frame'

Lars Ingebrigtsen
Eli Zaretskii <[hidden email]> writes:

> I think this command indeed has a bug, in that it cannot disable the
> menu bar only on the frame from which it is invoked.  It _can_ enable
> the menu bar on a single frame, see my response to Drew.  So I think
> this command needs to be improved.  But that's another issue; the doc
> string definitely needs to be improved.  Will you do that, or should
> I?

Please go ahead.

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no