[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [patch v3] showing menu bar temporarily when f10 is pressed
From: |
Stefan Monnier |
Subject: |
Re: [patch v3] showing menu bar temporarily when f10 is pressed |
Date: |
Thu, 19 Jul 2012 02:54:19 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1.50 (gnu/linux) |
>>> Why do you need the menu-bar? Why not pop up the menu usually bound to
>>> C-mouse-3?
>> To be honest, I didn't know that.
>> I've just tried C-mouse-3; and found some advantages of C-mouse-3
>> comparing with f10 for KEYBOARD USER.
While I agree we should also support the f10 behavior (so your current
patch is acceptable), I think the problem you point out for the
C-mouse-3 behavior could/should be fixed.
>> 1. With C-mouse-3, the menu is pop up where mouse pointer is.
That should be easy to fix so that the menu is popped up at point.
>> 2. With C-mouse-3, the contents of menu is chosen based
>> on the window where the mouse pointer is.
Same here.
Some comments on your code:
> +(defcustom x-menu-bar-temorary-visibility t
> + "Control whether use menu bar or `tmm-menubar' when `x-menu-bar-open' is
> invoked.
> +Non-nil means showing menu bar temporarily even if `menu-bar-mode' is turned
> off.
> +If nil, the behavior is changed whether `menu-bar-mode' is on or off. See
> `x-menu-bar-open'
> +for more detail."
> + :type 'boolean
> + :group 'x
> + :version "24.2")
Please stay within 80 columns and use two spaces to separate sentences.
> +(defun x-accelerate-menu-with-temporary-visible-menu-bar (frame)
Please only use "x-" for functions that are specific to X11.
> + (when (or (frame-live-p old-frame)
> + ;; Handle the case that if menu-bar-mode is
> + ;; turned on/off with choosing a menu item for
> controling the mode.
> + (or (eq old-menu-bar-mode menu-bar-mode)
> + (not menu-bar-mode)))
I don't understand this condition:
- shouldn't the outer `or' be an `and'?
- why check (not menu-bar-mode)?
> + (prog1
> + (accelerate-menu frame)
> + (add-hook 'deactivate-menubar-hook hide))))))
Why put the add-hook after rather than before calling accelerate-menu?
If it can't come before, then the prog1 needs to be replaced by an
unwind-protect in case accelerate-menu signals an error.
> (defun x-menu-bar-open (&optional frame)
> - "Open the menu bar if `menu-bar-mode' is on, otherwise call `tmm-menubar'."
> + "Open the menu bar if `menu-bar-mode' is on or
> `x-menu-bar-temorary-visibility' is non-nil, otherwise call `tmm-menubar'."
This docstring's first line is too long. It has to be less than 80
columns and it has to "stand on its own" (you can't just line-wrap it)
since modes like apropos will only display the first line.
> (interactive "i")
> - (if (and menu-bar-mode
> - (fboundp 'accelerate-menu))
> - (accelerate-menu frame)
> + (if (fboundp 'accelerate-menu)
> + (cond
> + (x-menu-bar-temorary-visibility
> + (x-accelerate-menu-with-temporary-visible-menu-bar frame))
> + (menu-bar-mode
> + (accelerate-menu frame))
To be on the safer side, I recommend you only use
x-accelerate-menu-with-temporary-visible-menu-bar when accelerate-menu
won't work (e.g. when menu-bar-mode is nil).
> popup_deactivate_callback (GtkWidget *widget, gpointer client_data)
> {
> + safe_run_hooks (Qdeactivate_menubar_hook);
> popup_activated_flag = 0;
> }
Could someone more knowledgeable tell me if this is safe?
I.e. is popup_deactivate_callback called asynchronously (e.g. from
a separate thread or from a signal handler)?
If not, then we'll need to delay running deactivate-menubar-hook to
a safe moment (maybe export popup_activated_flag to Lisp and then use
a pre/post-command-hook that checks this var before running
deactivate-menubar-hook).
Stefan
- Re: [patch v3] showing menu bar temporarily when f10 is pressed, Masatake YAMATO, 2012/07/18
- Re: [patch v3] showing menu bar temporarily when f10 is pressed,
Stefan Monnier <=
- Where the menu should be appeared when C-mouse-3 is triggered (Was: [patch v3] showing menu bar temporarily when f10 is pressed), Masatake YAMATO, 2012/07/19
- Re: Where the menu should be appeared when C-mouse-3 is triggered, Stefan Monnier, 2012/07/19
- Re: Where the menu should be appeared when C-mouse-3 is triggered, Masatake YAMATO, 2012/07/20
- Re: Where the menu should be appeared when C-mouse-3 is triggered, Stefan Monnier, 2012/07/20
- Re: Where the menu should be appeared when C-mouse-3 is triggered, Masatake YAMATO, 2012/07/20
- Re: Where the menu should be appeared when C-mouse-3 is triggered, Masatake YAMATO, 2012/07/28
- Re: Where the menu should be appeared when C-mouse-3 is triggered, Masatake YAMATO, 2012/07/20