bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Theodor Thornhill]

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


Hello,

Attached is a patch to change the default behaviour of 'project-shell'.

My use case is:

- Edit some file in some project.
- 'M-x project-shell'
- Do something, then "C-x 0"
- Oops forgot to read output.
- 'M-x project-shell'
- See the same buffer.

Right now 'project-shell' opens a new buffer every time it is invoked. Now I have to find it in the buffer list. Putting previous behaviour on universal argument will in addition enable this workflow

- Edit some file in some project.
- 'M-x project-shell'
- 'C-u M-x project-shell'
- Now emacs is split it two, with two separate shell processes


In addition, I let default name be *'project-root-dir'-shell* to signify this shell buffer to be unique from other shell buffers invoked by 'M-x shell'.

All the best,
Theodor Thornhill


[-- Attachment #2: project-shell --]
[-- Type: application/octet-stream, Size: 1268 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f3df44fa7b..50e3dc3c8a 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -686,11 +686,21 @@ project-vc-dir
 
 ;;;###autoload
 (defun project-shell ()
-  "Open Shell in the current project."
+  "Open Shell in the current project.
+With \\[universal-argument] prefix, create subsequent shell buffers
+with uniquified names."
   (interactive)
-  (let ((default-directory (project-root (project-current t))))
-    ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
-    (shell (create-file-buffer "*shell*"))))
+  (let* ((default-directory (project-root (project-current t)))
+         (default-project-shell-name
+           (concat "*" (file-name-nondirectory
+                        (directory-file-name
+                         (file-name-directory default-directory)))
+                   "-shell*"))
+         (shell-buffer (get-buffer default-project-shell-name)))
+    (if (and shell-buffer (not current-prefix-arg))
+        (switch-to-buffer-other-window shell-buffer)
+      ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
+      (shell (create-file-buffer default-project-shell-name)))))
 
 ;;;###autoload
 (defun project-eshell ()

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Dmitry Gutov]

Hi!

On 14.06.2020 21:29, Theodor Thornhill wrote:
> Attached is a patch to change the default behaviour of 'project-shell'.
> 
> My use case is:
> 
> - Edit some file in some project.
> - 'M-x project-shell'
> - Do something, then "C-x 0"
> - Oops forgot to read output.
> - 'M-x project-shell'
> - See the same buffer.
> 
> Right now 'project-shell' opens a new buffer every time it is invoked. Now I have to find it in the buffer list. Putting previous behaviour on universal argument will in addition enable this workflow
> 
> - Edit some file in some project.
> - 'M-x project-shell'
> - 'C-u M-x project-shell'
> - Now emacs is split it two, with two separate shell processes
> 
> 
> In addition, I let default name be *'project-root-dir'-shell* to signify this shell buffer to be unique from other shell buffers invoked by 'M-x shell'.

Sounds good to me.

Juri, will you the review?

NB: It's difficult for me to open the attachment because it has to file 
extension.

bug#41858: Add the patch as a .patch

[Theodor Thornhill]

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



Hello,

Sorry, seemed to forget the extension to the attached file.

This file should be more accessible.

Hope everything works now.

Theodor Thornhill



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-shell.patch --]
[-- Type: text/x-patch; name=project-shell.patch, Size: 1268 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f3df44fa7b..50e3dc3c8a 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -686,11 +686,21 @@ project-vc-dir
 
 ;;;###autoload
 (defun project-shell ()
-  "Open Shell in the current project."
+  "Open Shell in the current project.
+With \\[universal-argument] prefix, create subsequent shell buffers
+with uniquified names."
   (interactive)
-  (let ((default-directory (project-root (project-current t))))
-    ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
-    (shell (create-file-buffer "*shell*"))))
+  (let* ((default-directory (project-root (project-current t)))
+         (default-project-shell-name
+           (concat "*" (file-name-nondirectory
+                        (directory-file-name
+                         (file-name-directory default-directory)))
+                   "-shell*"))
+         (shell-buffer (get-buffer default-project-shell-name)))
+    (if (and shell-buffer (not current-prefix-arg))
+        (switch-to-buffer-other-window shell-buffer)
+      ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
+      (shell (create-file-buffer default-project-shell-name)))))
 
 ;;;###autoload
 (defun project-eshell ()

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Juri Linkov]

> Right now 'project-shell' opens a new buffer every time it is invoked.
> Now I have to find it in the buffer list.  Putting previous behaviour
> on universal argument will in addition enable this workflow
>
> - Edit some file in some project.
> - 'M-x project-shell'
> - 'C-u M-x project-shell'
> - Now emacs is split it two, with two separate shell processes
>
> In addition, I let default name be *'project-root-dir'-shell* to
> signify this shell buffer to be unique from other shell buffers
> invoked by 'M-x shell'.

Thanks, it's a nice idea to prepend the project root dir name
to the shell buffer name.

Formerly, I tried to do something like this using 'uniquify',
but after creating more shell buffers in the same project,
it begins to generate inappropriately long names.

So eventually I arrived to the following line for the init file:

  (add-hook 'shell-mode-hook 'rename-uniquely)

that just appends a number to the buffer name,
so your patch won't affect my usage at all,
I'll continue getting new shell buffers every time
after 'M-x project-shell'.

Regarding the C-u argument, I don't know maybe someone might want
to use it.  But shouldn't the C-u argument of 'project-shell' be
more compatible with the C-u argument of 'shell' in its behaviour?

Another possible point for better consistency between them
is to use 'pop-to-buffer' (like in 'shell') instead of
'switch-to-buffer-other-window'.

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Theodor Thornhill]

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

Hi, and thanks for looking at the code!

"Juri Linkov" <juri@linkov.net> writes:

[...]
> Thanks, it's a nice idea to prepend the project root dir name
> to the shell buffer name.

Cool!

[...]
> So eventually I arrived to the following line for the init file:
>
>   (add-hook 'shell-mode-hook 'rename-uniquely)
>
> that just appends a number to the buffer name,
> so your patch won't affect my usage at all,
> I'll continue getting new shell buffers every time
> after 'M-x project-shell'.
>
How about we just add this behaviour instead of appending the file-path? so we get the "<n>" appended to the shell buffer names. Then you can remove that line from init :)

> Regarding the C-u argument, I don't know maybe someone might want
> to use it.  But shouldn't the C-u argument of 'project-shell' be
> more compatible with the C-u argument of 'shell' in its behaviour?

I kind of agree, though it seems like the original "project-shell" was intended to be used only interactively. I kind of view it as a specialized case of "shell", maybe as a sort of "shell-dwim" but only for a very limited case. We are always free of course to add a normal shell buffer in root, and use it non-interactively in code.

However, I can add an "&optional buffer" argument to the code and do like it's done in "shell". It just seems like it is complicating the behaviour a little bit. What do you think?

In addition, how do I then avoid duplication of code between the interactive call and the default behaviour? Specifically, I am thinking of the 'default-directory' which is not set. I guess I have to set that twice then?

> Another possible point for better consistency between them
> is to use 'pop-to-buffer' (like in 'shell') instead of
> 'switch-to-buffer-other-window'.

Agreed. Done.

I've attached another patch for you to look at, with only the easy changes, awaiting some feedback on the other parts :)

Thanks again,

Theodor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-shell.patch --]
[-- Type: text/x-patch; name=project-shell.patch, Size: 1187 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f3df44fa7b..218058b195 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -686,11 +686,20 @@ project-vc-dir
 
 ;;;###autoload
 (defun project-shell ()
-  "Open Shell in the current project."
+  "Open Shell in the current project.
+With \\[universal-argument] prefix, create subsequent shell buffers
+with uniquified names."
   (interactive)
-  (let ((default-directory (project-root (project-current t))))
-    ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
-    (shell (create-file-buffer "*shell*"))))
+  (let* ((default-directory (project-root (project-current t)))
+         (default-project-shell-name
+           (concat "*" (file-name-nondirectory
+                        (directory-file-name
+                         (file-name-directory default-directory)))
+                   "-shell*"))
+         (shell-buffer (get-buffer default-project-shell-name)))
+    (if (and shell-buffer (not current-prefix-arg))
+        (pop-to-buffer shell-buffer)
+      (shell (generate-new-buffer-name default-project-shell-name)))))
 
 ;;;###autoload
 (defun project-eshell ()

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Dmitry Gutov]

On 15.06.2020 01:46, Juri Linkov wrote:
> But shouldn't the C-u argument of 'project-shell' be
> more compatible with the C-u argument of 'shell' in its behaviour?

Looking at the docstring, shell's C-u behavior seems pretty weird to me.

Why would someone input an existing buffer this way, if they could just 
switch to it instead, with the same completion interface?

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Theodor Thornhill]

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

> Looking at the docstring, shell's C-u behavior seems pretty weird to me.
>
> Why would someone input an existing buffer this way, if they could just
> switch to it instead, with the same completion interface?

+1.

Also, if you choose an existing non-shell buffer, the one you are in for example, it will actually just turn that buffer into a shell buffer. That behavior should actually be a bug imo.

Theo

[-- Attachment #2: Type: text/html, Size: 615 bytes --]

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Juri Linkov]

> How about we just add this behaviour instead of appending the
> file-path?  so we get the "<n>" appended to the shell buffer names.
> Then you can remove that line from init :)

Thanks, I see you did exactly this in your new patch.

>> Regarding the C-u argument, I don't know maybe someone might want
>> to use it.  But shouldn't the C-u argument of 'project-shell' be
>> more compatible with the C-u argument of 'shell' in its behaviour?
>
> I kind of agree, though it seems like the original "project-shell" was
> intended to be used only interactively. I kind of view it as a specialized
> case of "shell", maybe as a sort of "shell-dwim" but only for a very
> limited case. We are always free of course to add a normal shell buffer in
> root, and use it non-interactively in code.
>
> However, I can add an "&optional buffer" argument to the code and do
> like it's done in "shell".  It just seems like it is complicating the
> behaviour a little bit.  What do you think?

I agree there is no need to complicate "project-shell" with all
idiosyncrasies of "shell".

> I've attached another patch for you to look at, with only the easy
> changes, awaiting some feedback on the other parts :)

I see no more problems with your patch, it seems it covers the needs
of most users, thanks.

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Theodor Thornhill]

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

Hello!

"Juri Linkov" <juri@linkov.net> writes:
> I see no more problems with your patch, it seems it covers the needs
> of most users, thanks.

If this one seems ok, then maybe a commit message could be:

-------------------------------------------------------------------
Change default behaviour of 'project-shell'

* lisp/progmodes/project.el (project-shell):   If a shell already is open, pop to that buffer. Otherwise, or if universal argument is used, open a subsequent shell buffer and jump to it.

Also sets shell buffer name with a prefix of project root name.
-------------------------------------------------------------------

Have a nice day!

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-shell.patch --]
[-- Type: text/x-patch; name=project-shell.patch, Size: 1187 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index e24d81c1b4..aad3eed887 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -714,11 +714,20 @@ project-vc-dir
 
 ;;;###autoload
 (defun project-shell ()
-  "Open Shell in the current project."
+  "Open Shell in the current project.
+With \\[universal-argument] prefix, create subsequent shell buffers
+with uniquified names."
   (interactive)
-  (let ((default-directory (project-root (project-current t))))
-    ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
-    (shell (create-file-buffer "*shell*"))))
+  (let* ((default-directory (project-root (project-current t)))
+         (default-project-shell-name
+           (concat "*" (file-name-nondirectory
+                        (directory-file-name
+                         (file-name-directory default-directory)))
+                   "-shell*"))
+         (shell-buffer (get-buffer default-project-shell-name)))
+    (if (and shell-buffer (not current-prefix-arg))
+        (pop-to-buffer shell-buffer)
+      (shell (generate-new-buffer-name default-project-shell-name)))))
 
 ;;;###autoload
 (defun project-eshell ()

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Eli Zaretskii]

> Date: Thu, 18 Jun 2020 09:57:55 +0000
> From: Theodor Thornhill <theo@thornhill.no>
> Cc: 41858@debbugs.gnu.org, Dmitry Gutov <dgutov@yandex.ru>
> 
> Change default behaviour of 'project-shell'
> 
> * lisp/progmodes/project.el (project-shell):   If a shell already is open, pop to that buffer. Otherwise, or if universal argument is used, open a subsequent shell buffer and jump to it.

Please always mention the bug number, if there is any, in the commit
log message.  A reference to a bug makes it easy to find discussions
relevant to a change, which is very important when one needs to
understand better the rationale for a change.

Also, please format the log message according to our conventions, as
described in CONTRIBUTE.

TIA.

>  (defun project-shell ()
> -  "Open Shell in the current project."
> +  "Open Shell in the current project.

This doesn't say enough about what the command does.  How about

  Start an inferior shell in the current project's root directory.

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Theodor Thornhill]

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

Hello!

"Eli Zaretskii" <eliz@gnu.org> writes:

> Please always mention the bug number, if there is any, in the commit
> log message.
Ok!

> Also, please format the log message according to our conventions, as
> described in CONTRIBUTE.

Read, and new attempt further down.

> This doesn't say enough about what the command does.  How about
>
>   Start an inferior shell in the current project's root directory.

Yeah, that is clearer. I've added it in the attached patch.

Second attempt of a commit message:

-------------------------------------------------------------------
Change default behaviour of 'project-shell'

* lisp/progmodes/project.el (project-shell): If a shell
already is open, pop to that buffer. Otherwise, or if
universal argument is used, open a subsequent shell buffer
and jump to it. Also sets shell buffer name with a prefix of
project root name. (Bug#41858)
-------------------------------------------------------------------



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-shell.patch --]
[-- Type: text/x-patch; name=project-shell.patch, Size: 1217 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index e24d81c1b4..ca650bdd52 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -714,11 +714,20 @@ project-vc-dir
 
 ;;;###autoload
 (defun project-shell ()
-  "Open Shell in the current project."
+  "Start an inferior shell in the current project's root directory.
+With \\[universal-argument] prefix, create subsequent shell buffers
+with uniquified names."
   (interactive)
-  (let ((default-directory (project-root (project-current t))))
-    ;; Use ‘create-file-buffer’ to uniquify shell buffer names.
-    (shell (create-file-buffer "*shell*"))))
+  (let* ((default-directory (project-root (project-current t)))
+         (default-project-shell-name
+           (concat "*" (file-name-nondirectory
+                        (directory-file-name
+                         (file-name-directory default-directory)))
+                   "-shell*"))
+         (shell-buffer (get-buffer default-project-shell-name)))
+    (if (and shell-buffer (not current-prefix-arg))
+        (pop-to-buffer shell-buffer)
+      (shell (generate-new-buffer-name default-project-shell-name)))))
 
 ;;;###autoload
 (defun project-eshell ()

bug#41858: 28.0.50; [PATCH]: Make project-shell use universal argument

[Dmitry Gutov]

On 18.06.2020 21:00, Theodor Thornhill wrote:
> Yeah, that is clearer. I've added it in the attached patch.

Thank you. I've pushed the patch with a modified commit message.

Closing.

We should probably do something like this for project-eshell as well.