bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Pengji Zhang]

Hello!

Currently the function `project--other-place-command' uses
`display-buffer-overriding-action' which IMO makes
project-other-{window,frame,tab}-command a bit too obtrusive. For
example,

1. Start Emacs with 'emacs -Q';
2. Press 'C-x 5 p f';
3. Press 'TAB'.

Then the '*Completions*' buffer pops up in a new frame. The expected
behavior of mine is that the '*Completions*' buffer shows at the
bottom of the current frame, which is the same as most of other 'C-x
5' commands.

Thanks!


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu) of 2023-08-22
Repository revision: 45555c1b789c547cdb5e5daf657a90b5ba431792
Repository branch: master

Configured using:
 'configure --without-x --program-transform-name=s/^ctags/ctags.emacs/'

Configured features:
GNUTLS LIBXML2 MODULES NOTIFY INOTIFY PDUMPER SOUND SQLITE3 THREADS ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow regexp-opt sort mail-extr emacsbug message mailcap yank-media
puny dired dnd dired-loaddefs rfc822 mml mml-sec password-cache epa
derived epg rfc6068 epg-config gnus-util text-property-search time-date
subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils vc-git diff-mode easy-mmode vc-dispatcher
thingatpt cl-seq project cl-loaddefs cl-lib term/tmux term/xterm xterm
byte-opt gv bytecomp byte-compile rmc iso-transl tooltip cconv eldoc
paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode
touch-screen tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads inotify multi-tty make-network-process emacs)

Memory information:
((conses 16 85761 9842) (symbols 48 6446 0) (strings 32 24432 1515)
 (string-bytes 1 689043) (vectors 16 10095)
 (vector-slots 8 122180 3896) (floats 8 26 11543)
 (intervals 56 10515 8) (buffers 984 11) (heap 1024 4154 811))

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Juri Linkov]

> Currently the function `project--other-place-command' uses
> `display-buffer-overriding-action' which IMO makes
> project-other-{window,frame,tab}-command a bit too obtrusive. For
> example,
>
> 1. Start Emacs with 'emacs -Q';
> 2. Press 'C-x 5 p f';
> 3. Press 'TAB'.
>
> Then the '*Completions*' buffer pops up in a new frame. The expected
> behavior of mine is that the '*Completions*' buffer shows at the
> bottom of the current frame, which is the same as most of other 'C-x
> 5' commands.

Thanks for the request.  I believe this will be fixed in bug#63648.

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Pengji Zhang]

Thanks for the pointer! That is a long thread so I only skimmed it. I
think you meant this snippet in [0]:

> (defun project-other-window-command ()
>   (interactive)
>   (other-window-prefix)
>   (set-transient-map (make-composed-keymap project-prefix-map
>                                            project-other-window-map)))

That indeed fixes the issue. My only concern is that
`other-window-prefix' is a new function introduced in Emacs 28.

[0] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#95

On Sun, Aug 27, 2023 at 12:21 PM Juri Linkov <juri@linkov.net> wrote:
>
> > Currently the function `project--other-place-command' uses
> > `display-buffer-overriding-action' which IMO makes
> > project-other-{window,frame,tab}-command a bit too obtrusive. For
> > example,
> >
> > 1. Start Emacs with 'emacs -Q';
> > 2. Press 'C-x 5 p f';
> > 3. Press 'TAB'.
> >
> > Then the '*Completions*' buffer pops up in a new frame. The expected
> > behavior of mine is that the '*Completions*' buffer shows at the
> > bottom of the current frame, which is the same as most of other 'C-x
> > 5' commands.
>
> Thanks for the request.  I believe this will be fixed in bug#63648.

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Juri Linkov]

> Thanks for the pointer! That is a long thread so I only skimmed it. I
> think you meant this snippet in [0]:
>
>> (defun project-other-window-command ()
>>   (interactive)
>>   (other-window-prefix)
>>   (set-transient-map (make-composed-keymap project-prefix-map
>>                                            project-other-window-map)))

Exactly.  And the same for the frame command in the most recent patch
I posted to bug#63648, and I confirm that your bug report is fixed with it.

> That indeed fixes the issue. My only concern is that
> `other-window-prefix' is a new function introduced in Emacs 28.

Indeed, it would take more efforts to provide backward-compatibility
for older Emacs versions.

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

Hi Juri,

On 30/08/2023 19:50, Juri Linkov wrote:
>> Thanks for the pointer! That is a long thread so I only skimmed it. I
>> think you meant this snippet in [0]:
>>
>>> (defun project-other-window-command ()
>>>    (interactive)
>>>    (other-window-prefix)
>>>    (set-transient-map (make-composed-keymap project-prefix-map
>>>                                             project-other-window-map)))
> Exactly.  And the same for the frame command in the most recent patch
> I posted to bug#63648, and I confirm that your bug report is fixed with it.
> 
>> That indeed fixes the issue. My only concern is that
>> `other-window-prefix' is a new function introduced in Emacs 28.
> Indeed, it would take more efforts to provide backward-compatibility
> for older Emacs versions.

I think we can install this (tweaked) part of your changes from that 
other bug to close this report.

[-- Attachment #2: project-other-place-prefix.diff --]
[-- Type: text/x-patch, Size: 2702 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index b9ecb770e60..508ade2e0c7 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -880,6 +880,17 @@ project--other-place-command
         (call-interactively cmd)
       (user-error "%s is undefined" (key-description key)))))
 
+(defun project--other-place-prefix (place &optional extra-keymap)
+  (cl-assert (member place '(window frame tab)))
+  (prefix-command-preserve-state)
+  (let ((inhibit-message t)) (funcall (intern (format "other-%s-prefix" place))))
+  (message "Display next project command buffer in a new %s..." place)
+  ;; Should return exitfun from set-transient-map
+  (set-transient-map (if extra-keymap
+                         (make-composed-keymap project-prefix-map
+                                               extra-keymap)
+                       project-prefix-map)))
+
 ;;;###autoload
 (defun project-other-window-command ()
   "Run project command, displaying resultant buffer in another window.
@@ -889,10 +900,13 @@ project-other-window-command
 \\{project-prefix-map}
 \\{project-other-window-map}"
   (interactive)
-  (project--other-place-command '((display-buffer-pop-up-window)
-                                  (inhibit-same-window . t))
-                                project-other-window-map))
+  (if (< emacs-major-version 30)
+      (project--other-place-command '((display-buffer-pop-up-window)
+                                      (inhibit-same-window . t))
+                                    project-other-window-map)
+    (project--other-place-prefix 'window project-other-window-map)))
 
+;; TODO: maybe rename to project-other-window-prefix
 ;;;###autoload (define-key ctl-x-4-map "p" #'project-other-window-command)
 
 ;;;###autoload
@@ -904,8 +918,10 @@ project-other-frame-command
 \\{project-prefix-map}
 \\{project-other-frame-map}"
   (interactive)
-  (project--other-place-command '((display-buffer-pop-up-frame))
-                                project-other-frame-map))
+  (if (< emacs-major-version 30)
+      (project--other-place-command '((display-buffer-pop-up-frame))
+                                    project-other-frame-map)
+    (project--other-place-prefix 'frame project-other-frame-map)))
 
 ;;;###autoload (define-key ctl-x-5-map "p" #'project-other-frame-command)
 
@@ -917,7 +933,9 @@ project-other-tab-command
 
 \\{project-prefix-map}"
   (interactive)
-  (project--other-place-command '((display-buffer-in-new-tab))))
+  (if (< emacs-major-version 30)
+      (project--other-place-command '((display-buffer-in-new-tab)))
+    (project--other-place-prefix 'tab)))
 
 ;;;###autoload
 (when (bound-and-true-p tab-prefix-map)

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Juri Linkov]

>>> Thanks for the pointer! That is a long thread so I only skimmed it. I
>>> think you meant this snippet in [0]:
>>>
>>>> (defun project-other-window-command ()
>>>>    (interactive)
>>>>    (other-window-prefix)
>>>>    (set-transient-map (make-composed-keymap project-prefix-map
>>>>                                             project-other-window-map)))
>> Exactly.  And the same for the frame command in the most recent patch
>> I posted to bug#63648, and I confirm that your bug report is fixed with it.
>>
>>> That indeed fixes the issue. My only concern is that
>>> `other-window-prefix' is a new function introduced in Emacs 28.
>> Indeed, it would take more efforts to provide backward-compatibility
>> for older Emacs versions.
>
> I think we can install this (tweaked) part of your changes from that other
> bug to close this report.

Since it could be installed independently from bug#63648,
I think it would be nice to close this bug report with this patch.

> +;; TODO: maybe rename to project-other-window-prefix
>  ;;;###autoload (define-key ctl-x-4-map "p" #'project-other-window-command)

Maybe this TODO not needed anymore?

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Dmitry Gutov]

On 19/10/2023 20:49, Juri Linkov wrote:
>> I think we can install this (tweaked) part of your changes from that other
>> bug to close this report.
> Since it could be installed independently from bug#63648,
> I think it would be nice to close this bug report with this patch.

Very good. If you can install it, please go ahead.

>> +;; TODO: maybe rename to project-other-window-prefix
>>   ;;;###autoload (define-key ctl-x-4-map "p" #'project-other-window-command)
> Maybe this TODO not needed anymore?

I think you had a point with it, but I'm not sure how strong it is, and 
where is the line between a "x-y-z-command" that does a thing like this 
and a proper "prefix" command. So I'd say it's up to you.

We can remove the TODO, or go ahead with the renaming, or keep the TODO 
and return to it sometime years later when Emacs 30 is the least 
supported version.

bug#65558: 30.0.50; project-other-{window,frame,tab}-command a bit too obtrusive

[Juri Linkov]

close 65558 30.0.50
thanks

>>> +;; TODO: maybe rename to project-other-window-prefix
>>>   ;;;###autoload (define-key ctl-x-4-map "p" #'project-other-window-command)
>> Maybe this TODO not needed anymore?
>
> I think you had a point with it, but I'm not sure how strong it is, and
> where is the line between a "x-y-z-command" that does a thing like this and
> a proper "prefix" command. So I'd say it's up to you.
>
> We can remove the TODO, or go ahead with the renaming, or keep the TODO and
> return to it sometime years later when Emacs 30 is the least supported
> version.

With this TODO item I meant something like this:

  ;;;###autoload
  (define-key ctl-x-4-map "p"
    (if (< emacs-major-version 30)
        #'project-other-window-command
      #'project-other-window-prefix))

But now I see it's quite ugly, so I installed the current patch
without TODO.