Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Stefan Monnier]

> +      ;; Update `this-command' and run `pre-command-hook' so that
> +      ;; things like `delete-selection-pre-hook' will work correctly.
> +      (setq this-command cmd)
> +      (run-hooks 'pre-command-hook)
>        ;; mouse-major-mode-menu was using `command-execute' instead.
>        (call-interactively cmd))))
 
Hmm... shouldn't it then also run post-command-hook, and set
last-command (and probably a few more of the things done in the usual
command loop, such as handling undo-boundaries)?

IOW, we should move the C code which runs the commands from the command
loop and make it available to Elisp.


        Stefan

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Noam Postavsky]

On Tue, Jul 18, 2017 at 9:41 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> +      ;; Update `this-command' and run `pre-command-hook' so that
>> +      ;; things like `delete-selection-pre-hook' will work correctly.
>> +      (setq this-command cmd)
>> +      (run-hooks 'pre-command-hook)
>>        ;; mouse-major-mode-menu was using `command-execute' instead.
>>        (call-interactively cmd))))
>
> Hmm... shouldn't it then also run post-command-hook, and set
> last-command (and probably a few more of the things done in the usual
> command loop, such as handling undo-boundaries)?

I'm not sure, won't we end up with 1 1/2 of the command stuff done in
that case? (the half being the popup-menu we began executing to get
here).

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Stefan Monnier]

>>> +      ;; Update `this-command' and run `pre-command-hook' so that
>>> +      ;; things like `delete-selection-pre-hook' will work correctly.
>>> +      (setq this-command cmd)
>>> +      (run-hooks 'pre-command-hook)
>>> ;; mouse-major-mode-menu was using `command-execute' instead.
>>> (call-interactively cmd))))
>> 
>> Hmm... shouldn't it then also run post-command-hook, and set
>> last-command (and probably a few more of the things done in the usual
>> command loop, such as handling undo-boundaries)?

> I'm not sure, won't we end up with 1 1/2 of the command stuff done in
> that case? (the half being the popup-menu we began executing to get
> here).

AFAICT the current case is the one that ends up with 1½ done.
What I suggest is to bring it back to a whole number (i.e. 2).


        Stefan

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Noam Postavsky]

On Tue, Jul 18, 2017 at 1:28 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>>>> +      ;; Update `this-command' and run `pre-command-hook' so that
>>>> +      ;; things like `delete-selection-pre-hook' will work correctly.
>>>> +      (setq this-command cmd)
>>>> +      (run-hooks 'pre-command-hook)
>>>> ;; mouse-major-mode-menu was using `command-execute' instead.
>>>> (call-interactively cmd))))
>>>
>>> Hmm... shouldn't it then also run post-command-hook, and set
>>> last-command (and probably a few more of the things done in the usual
>>> command loop, such as handling undo-boundaries)?
>
>> I'm not sure, won't we end up with 1 1/2 of the command stuff done in
>> that case? (the half being the popup-menu we began executing to get
>> here).
>
> AFAICT the current case is the one that ends up with 1½ done.
> What I suggest is to bring it back to a whole number (i.e. 2).

Right, I got mixed up.

Actually, now that I think more on this, could `popup-menu' behave
more like `universal-argument' (apart from actually setting any prefix
arg)? It feels kind of prefixy.

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Stefan Monnier]

> Actually, now that I think more on this, could `popup-menu' behave
> more like `universal-argument' (apart from actually setting any prefix
> arg)? It feels kind of prefixy.

Not sure exactly what you have in mind, but it seems difficult to do
something along these lines while preserving compatibility for existing
callers of popup-menu.

But maybe we could have a new function which ends up doing something
like push onto unread-command-events a special event which popups the
specified menu [ maybe using some other mechanism than
unread-command-events ].


        Stefan

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Noam Postavsky]

On Tue, Jul 18, 2017 at 2:14 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Actually, now that I think more on this, could `popup-menu' behave
>> more like `universal-argument' (apart from actually setting any prefix
>> arg)? It feels kind of prefixy.
>
> Not sure exactly what you have in mind,

Yes, that idea was rather vague and half-baked. While looking at other
mouse menus, I found this alternate solution to Bug#27569 (following
the technique used for C-down-mouse-3):

(define-key global-map [mouse-3]
  `(menu-item "Edit menu" menu-bar-edit-menu
          :filter ,(lambda (_) menu-bar-edit-menu)))
;; delete-selection now works correctly with yank invoked via the mouse-3 menu
(delete-selection-mode +1)

So perhaps my change to popup-menu isn't really needed...

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Stefan Monnier]

> (define-key global-map [mouse-3]
>   `(menu-item "Edit menu" menu-bar-edit-menu
>           :filter ,(lambda (_) menu-bar-edit-menu)))

Why not just

  (define-key global-map [mouse-3] menu-bar-edit-menu)

or

  (define-key global-map [down-mouse-3] menu-bar-edit-menu)
  (define-key global-map [mouse-3] nil)


-- Stefan

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Tak Kunihiro]

> menu-bar-edit-menu
>           :filter ,(lambda (_) menu-bar-edit-menu)))
> ;; delete-selection now works correctly with yank invoked via the mouse-3 menu
> (delete-selection-mode +1)
> 
> So perhaps my change to popup-menu isn't really needed...

I want to construct edit-menu dynamically depending on event.
Using the technique, code will look:

(define-key global-map [mouse-3]
  (lambda (event)
    (interactive "e")
    (let ((my-edit-map (my-edit-map event)))
      `(menu-item "Edit menu" my-edit-map
                  :filter ,(lambda (_) my-edit-map)))))
;; delete-selection now works correctly with yank invoked via the mouse-3 menu
(delete-selection-mode +1)

(defun my-edit-map (start-event)
  ; ...
  menu-bar-edit-menu)

CONCLUSION:
I appreciate your change and I think it is needed.

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Noam Postavsky]

On Tue, Jul 18, 2017 at 11:18 PM, Tak Kunihiro
<tkk@misasa.okayama-u.ac.jp> wrote:
>
> I want to construct edit-menu dynamically depending on event.
> Using the technique, code will look:

I think that should be

> (defun my-edit-map (start-event)
>   ; ...
>   menu-bar-edit-menu)

(define-key global-map [mouse-3]
  `(menu-item "Edit menu" my-edit-map
          :filter ,(lambda (_) (my-edit-map (aref (this-command-keys) 0)))))
;; delete-selection now works correctly with yank invoked via the mouse-3 menu
(delete-selection-mode +1)

>
> CONCLUSION:
> [...] your change [...] is needed.

Still?

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Tak Kunihiro]

>> I want to construct edit-menu dynamically depending on event.
>> Using the technique, code will look:
> 
> I think that should be
> 
>> (defun my-edit-map (start-event)
>>   ; ...
>>   menu-bar-edit-menu)
> 
> (define-key global-map [mouse-3]
>   `(menu-item "Edit menu" my-edit-map
>           :filter ,(lambda (_) (my-edit-map (aref (this-command-keys) 0)))))
> ;; delete-selection now works correctly with yank invoked via the mouse-3 menu
> (delete-selection-mode +1)

That works.  Thank you for the revision.

I want to call flyspell-correct-word when word under event is
flyspell-incorrect (mouse-2 is too hard to access).  When not, then
the dynamic edit menu.

This technique binds a keymap.  I cannot find a way to call
flyspell-correct-word.

CONCLUSION:
Still your change is needed.

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Noam Postavsky]

On Wed, Jul 19, 2017 at 8:14 PM, Tak Kunihiro
<tkk@misasa.okayama-u.ac.jp> wrote:
>>
>> (define-key global-map [mouse-3]
>>   `(menu-item "Edit menu" my-edit-map
>>           :filter ,(lambda (_) (my-edit-map (aref (this-command-keys) 0)))))
>> ;; delete-selection now works correctly with yank invoked via the mouse-3 menu
>> (delete-selection-mode +1)
>
> I want to call flyspell-correct-word when word under event is
> flyspell-incorrect (mouse-2 is too hard to access).  When not, then
> the dynamic edit menu.

I don't have flyspell working here to test this properly, but
shouldn't something like this work:

(defun my-edit-map (event)
  (if (get-text-property (posn-point (event-start event))
             'flyspell-incorrect)
      #'flyspell-correct-word
  menu-bar-edit-menu))

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Stefan Monnier]

> This technique binds a keymap.  I cannot find a way to call
> flyspell-correct-word.

Just return the `flyspell-correct-word' symbol.


        Stefan

Re: [Emacs-diffs] master a2ee819: Let delete-selection-mode work with popup-menu commands (Bug#27569)

[Tak Kunihiro]

>>> (define-key global-map [mouse-3]
>>>   `(menu-item "Edit menu" my-edit-map
>>>           :filter ,(lambda (_) (my-edit-map (aref (this-command-keys) 0)))))
>>> ;; delete-selection now works correctly with yank invoked via the mouse-3 menu
>>> (delete-selection-mode +1)
>>
>> I want to call flyspell-correct-word when word under event is
>> flyspell-incorrect (mouse-2 is too hard to access).  When not, then
>> the dynamic edit menu.
>
> (defun my-edit-map (event)
>   (if (get-text-property (posn-point (event-start event))
>              'flyspell-incorrect)
>       #'flyspell-correct-word
>   menu-bar-edit-menu))
>

> Just return the `flyspell-correct-word' symbol.


Now I got rid of usage of popup-menu in the code.

(defun my-edit-map (event)
  (let ((faces-at-point (mapcar (lambda (xxx) (overlay-get xxx 'face))
                                (overlays-at (posn-point (event-start event))))))
    (cond
     ((member 'flyspell-incorrect faces-at-point)
      #'flyspell-correct-word)
     (t
      menu-bar-edit-menu))))


I confirmed that without the patch of menu-bar, yank via edit-menu
deletes region.  Thank you for instructions.

CONCLUSION:
Your change is not needed.