Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Stefan Kangas]

simenheg@gmail.com (Simen Heggestøyl) writes:

> branch: master
> commit 432c1aaa80ce109250a93f50858a03ce3d01ca34
> Author: Simen Heggestøyl <simenheg@gmail.com>
> Commit: Simen Heggestøyl <simenheg@runbox.com>
>
>     Use `pop-to-buffer-same-window' in `project-eshell'
>
>     * lisp/progmodes/project.el (project-eshell): Use
>     `pop-to-buffer-same-window' instead of `pop-to-buffer' to match the
>     behavior of `M-x eshell'.

I'm fine with this change.  (I've personally started using
`other-window-prefix' to open eshell in another window, which is what I
almost always want here.)

But now `project-eshell' and `project-shell' behave differently, which
seems unfortunate.  So if this is the behavior we want, shouldn't we
change the latter as well for consistency?

(OTOH, I checked the `projectile' package, and it seems to be exhibiting
the same difference in behavior between shell and eshell...)

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 04:06, Stefan Kangas wrote:
> But now `project-eshell' and `project-shell' behave differently, which
> seems unfortunate.  So if this is the behavior we want, shouldn't we
> change the latter as well for consistency?

Perhaps the answer is to first decide which behavior makes most sense, 
then make M-x eshell and M-x shell both adopt it, and then do it for 
project-* versions as well (which would be the easy part)?

Otherwise, the difference in behavior should remain unnecessary evil as 
long as project-xyz follows xyz.

I'm guessing the average user is more likely to more often use, say, 
eshell and project-eshell, rather than alternate between shell and 
eshell evenly.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill]

Hi there!

> 20. mar. 2021 kl. 18:08 skrev Dmitry Gutov <dgutov@yandex.ru>:
> 
> On 20.03.2021 04:06, Stefan Kangas wrote:
>> But now `project-eshell' and `project-shell' behave differently, which
>> seems unfortunate.  So if this is the behavior we want, shouldn't we
>> change the latter as well for consistency?
> 
> Perhaps the answer is to first decide which behavior makes most sense, then make M-x eshell and M-x shell both adopt it, and then do it for project-* versions as well (which would be the easy part)?

When I first implemented those functions I made eshell behave more like shell, and also made shell behave more sensibly (to me... which also is the reason universal argument acts differently from mx shell). Right now I am thinking we should let display-buffer decide. 

Should we force one behavior over another?

> 
> Otherwise, the difference in behavior should remain unnecessary evil as long as project-xyz follows xyz.
> 
> I'm guessing the average user is more likely to more often use, say, eshell and project-eshell, rather than alternate between shell and eshell evenly.

Not sure - i usually fall back to shell more and more often

—
Theodor

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Stefan Kangas]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Perhaps the answer is to first decide which behavior makes most sense,
> then make M-x eshell and M-x shell both adopt it, and then do it for
> project-* versions as well (which would be the easy part)?

Yes, that's a good approach.

> I'm guessing the average user is more likely to more often use, say,
> eshell and project-eshell, rather than alternate between shell and
> eshell evenly.

Sounds like a reasonable first guess.

I have no idea if my use is typical, but I use eshell when I can and
shell when I have to.  That is, I prefer eshell but sometimes run into
problems with this or that and cowardly revert to shell instead of
trying to fix it.

For example, I'm currently too lazy to figure out how to use a Python
virtualenv/venv with eshell so I'm back to using shell for a bit.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 19:21, Theodor Thornhill wrote:
> Hi there!
> 
>> 20. mar. 2021 kl. 18:08 skrev Dmitry Gutov <dgutov@yandex.ru>:
>>
>> On 20.03.2021 04:06, Stefan Kangas wrote:
>>> But now `project-eshell' and `project-shell' behave differently, which
>>> seems unfortunate.  So if this is the behavior we want, shouldn't we
>>> change the latter as well for consistency?
>>
>> Perhaps the answer is to first decide which behavior makes most sense, then make M-x eshell and M-x shell both adopt it, and then do it for project-* versions as well (which would be the easy part)?
> 
> When I first implemented those functions I made eshell behave more like shell, and also made shell behave more sensibly (to me... which also is the reason universal argument acts differently from mx shell). Right now I am thinking we should let display-buffer decide.

Meaning not use pop-to-buffer-same-window?

> Should we force one behavior over another?

FWIW, my (moderate) preference is Eshell's behavior because it's also 
consistent with IELM or Dired. And I use it more.

>> Otherwise, the difference in behavior should remain unnecessary evil as long as project-xyz follows xyz.
>>
>> I'm guessing the average user is more likely to more often use, say, eshell and project-eshell, rather than alternate between shell and eshell evenly.
> 
> Not sure - i usually fall back to shell more and more often

If Eshell + Shell fallback is a frequent pattern, then indeed 
consistency between them is desirable.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 19:31, Stefan Kangas wrote:

>> I'm guessing the average user is more likely to more often use, say,
>> eshell and project-eshell, rather than alternate between shell and
>> eshell evenly.
> 
> Sounds like a reasonable first guess.
> 
> I have no idea if my use is typical, but I use eshell when I can and
> shell when I have to.  That is, I prefer eshell but sometimes run into
> problems with this or that and cowardly revert to shell instead of
> trying to fix it.

I guess we have now two reports of this usage pattern.

So this inconsistency might be a significant issue.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill]

>> When I first implemented those functions I made eshell behave more like shell, and also made shell behave more sensibly (to me... which also is the reason universal argument acts differently from mx shell). Right now I am thinking we should let display-buffer decide.
> 
> Meaning not use pop-to-buffer-same-window?
> 

Too often i feel emacs places buffers or windows in seemingly arbitrary places, destroying my window setup. To circumvent this, my default display-buffer behavior is to always reuse same window. If we make more and more parts of emacs decide this on their own, I have to add more and more edge cases to make emacs behave. 


>> Should we force one behavior over another?
> 
> FWIW, my (moderate) preference is Eshell's behavior because it's also consistent with IELM or Dired. And I use it more.
> 

Yeah, Im inclined to agree, though I dont think we should force it.

—
Theodor

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 21:29, Theodor Thornhill wrote:

> Too often i feel emacs places buffers or windows in seemingly arbitrary places, destroying my window setup. To circumvent this, my default display-buffer behavior is to always reuse same window. If we make more and more parts of emacs decide this on their own, I have to add more and more edge cases to make emacs behave.

That seems to be common practice among advanced users.

I generally rely on the default behavior in this case except for a few 
commands. As far as Emacs windowing woes go, I often get bit by windows 
being resized improperly after quit-window.

>>> Should we force one behavior over another?
>>
>> FWIW, my (moderate) preference is Eshell's behavior because it's also consistent with IELM or Dired. And I use it more.
>>
> 
> Yeah, Im inclined to agree, though I dont think we should force it.

So... use pop-to-buffer-same-window in both cases, then?

It should obey display-buffer-alist anyway, so the user can still 
override it, as long as they know how.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill]

> I generally rely on the default behavior in this case except for a few 
> commands. As far as Emacs windowing woes go, I often get bit by windows 
> being resized improperly after quit-window.
>

Same.

>>>> Should we force one behavior over another?
>>>
>>> FWIW, my (moderate) preference is Eshell's behavior because it's also consistent with IELM or Dired. And I use it more.
>>>
>> 
>> Yeah, Im inclined to agree, though I dont think we should force it.
>
> So... use pop-to-buffer-same-window in both cases, then?
>

Sure - all four cases, then? shell, eshell, project-shell and project-eshell?

> It should obey display-buffer-alist anyway, so the user can still 
> override it, as long as they know how.

Good!

--
Theo

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 22:21, Theodor Thornhill wrote:
>>>>> Should we force one behavior over another?
>>>> FWIW, my (moderate) preference is Eshell's behavior because it's also consistent with IELM or Dired. And I use it more.
>>>>
>>> Yeah, Im inclined to agree, though I dont think we should force it.
>> So... use pop-to-buffer-same-window in both cases, then?
>>
> Sure - all four cases, then? shell, eshell, project-shell and project-eshell?

That would be my choice.

Unless someone here wants to voice a strong disagreement.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill]

>> Sure - all four cases, then? shell, eshell, project-shell and project-eshell?
>
> That would be my choice.
>
> Unless someone here wants to voice a strong disagreement.

Unless you're already on it, then I can prepare a patch for that

--
Theo

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 22:33, Theodor Thornhill wrote:
>>> Sure - all four cases, then? shell, eshell, project-shell and project-eshell?
>> That would be my choice.
>>
>> Unless someone here wants to voice a strong disagreement.
> Unless you're already on it, then I can prepare a patch for that

Please go ahead.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill]

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


>
> Please go ahead.

How about this? Would this change need a news entry?

--
Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-pop-to-buffer-same-window-for-shell.patch --]
[-- Type: text/x-patch, Size: 1583 bytes --]

From 711eaaf2c64b4e5aa9b49d147771a299fc31f81d Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sat, 20 Mar 2021 22:46:42 +0100
Subject: [PATCH] Use pop-to-buffer-same-window for shell

* lisp/progmodes/project.el (project-shell): Behave the same way as
'M-x project-eshell'
* lisp/shell.el (shell): Behave the same way as 'M-x eshell'
---
 lisp/progmodes/project.el | 2 +-
 lisp/shell.el             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index bd552c917a..b6a886f731 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -911,7 +911,7 @@ if one already exists."
                    "-shell*"))
          (shell-buffer (get-buffer default-project-shell-name)))
     (if (and shell-buffer (not current-prefix-arg))
-        (pop-to-buffer shell-buffer)
+        (pop-to-buffer-same-window shell-buffer)
       (shell (generate-new-buffer-name default-project-shell-name)))))
 
 ;;;###autoload
diff --git a/lisp/shell.el b/lisp/shell.el
index 53f5d0b6f1..7f4ca76547 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -743,7 +743,7 @@ Make the shell buffer the current buffer, and return it.
                  (current-buffer)))
   ;; The buffer's window must be correctly set when we call comint
   ;; (so that comint sets the COLUMNS env var properly).
-  (pop-to-buffer buffer)
+  (pop-to-buffer-same-window buffer)
 
   (with-connection-local-variables
    ;; On remote hosts, the local `shell-file-name' might be useless.
-- 
2.24.3 (Apple Git-128)

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Juri Linkov]

>>> So... use pop-to-buffer-same-window in both cases, then?
>>>
>> Sure - all four cases, then? shell, eshell, project-shell and project-eshell?
>
> That would be my choice.
>
> Unless someone here wants to voice a strong disagreement.

+1, I customized the same anyway:

(push `(,(rx bos
             "*"
             ;; Handle both "*shell*" and e.g. "*myproject-shell*"
             ;; generated by ‘project-shell’:
             (and (? (* nonl) "-") "shell")
             "*"
             ;; Uniquifed buffer name with optional suffix in angle brackets
             (? (and "<" (+ (not (any ">"))) ">"))
             eos)
        display-buffer-same-window
        (inhibit-same-window . nil))
      display-buffer-alist)

But it's fine also to change the format of uniquify as was discussed
recently to "myproject/*shell*<3>" or "*shell*<myproject><3>",
I don't remember what was the conclusion.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 20.03.2021 23:51, Theodor Thornhill wrote:
> Would this change need a news entry?

I think so. In the "Incompatible Editing Changes" section.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill via Emacs development discussions.]

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

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 20.03.2021 23:51, Theodor Thornhill wrote:
>> Would this change need a news entry?
>
> I think so. In the "Incompatible Editing Changes" section.

There :)

--
Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-pop-to-buffer-same-window-for-shell.patch --]
[-- Type: text/x-patch, Size: 2149 bytes --]

From c02866f883c6a893617fced03c499f80b4010007 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sat, 20 Mar 2021 23:36:29 +0100
Subject: [PATCH] Use pop-to-buffer-same-window for shell

* lisp/progmodes/project.el (project-shell): Behave the same way as
'M-x project-eshell'.
* lisp/shell.el (shell): Behave the same way as 'M-x eshell'.
* etc/NEWS: Add news entry describing the change.
---
 etc/NEWS                  | 3 +++
 lisp/progmodes/project.el | 2 +-
 lisp/shell.el             | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c602166397..2a87b82454 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2300,6 +2300,9 @@ since the latter uses 'M-s' as a prefix key of the search prefix map.
 ** 'vc-print-branch-log' shows the change log for BRANCH from its root
 directory instead of the default directory.
 
+** 'project-shell' and 'shell' now uses 'pop-to-buffer-same-window'
+This is to keep the same behavior as eshell has.
+
 \f
 * Incompatible Lisp Changes in Emacs 28.1
 
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index bd552c917a..b6a886f731 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -911,7 +911,7 @@ if one already exists."
                    "-shell*"))
          (shell-buffer (get-buffer default-project-shell-name)))
     (if (and shell-buffer (not current-prefix-arg))
-        (pop-to-buffer shell-buffer)
+        (pop-to-buffer-same-window shell-buffer)
       (shell (generate-new-buffer-name default-project-shell-name)))))
 
 ;;;###autoload
diff --git a/lisp/shell.el b/lisp/shell.el
index 53f5d0b6f1..7f4ca76547 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -743,7 +743,7 @@ Make the shell buffer the current buffer, and return it.
                  (current-buffer)))
   ;; The buffer's window must be correctly set when we call comint
   ;; (so that comint sets the COLUMNS env var properly).
-  (pop-to-buffer buffer)
+  (pop-to-buffer-same-window buffer)
 
   (with-connection-local-variables
    ;; On remote hosts, the local `shell-file-name' might be useless.
-- 
2.24.3 (Apple Git-128)

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Eli Zaretskii]

> Cc: Stefan Kangas <stefan@marxist.se>, Simen Heggestøyl
>  <simenheg@gmail.com>, emacs-devel@gnu.org
> Date: Sat, 20 Mar 2021 23:37:40 +0100
> From:  Theodor Thornhill via "Emacs development discussions." <emacs-devel@gnu.org>
> 
> +** 'project-shell' and 'shell' now uses 'pop-to-buffer-same-window'
                                      ^^^^
"use", in plural.  Also, the first line of a NEWS entry should be a
complete sentence, so it should end with a period.

> +This is to keep the same behavior as eshell has.

I believe our convention is to use "Eshell", capitalized.

Thanks.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Eli Zaretskii]

> Date: Sun, 21 Mar 2021 08:10:18 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org, stefan@marxist.se, simenheg@gmail.com,
>  dgutov@yandex.ru
> 
> > Cc: Stefan Kangas <stefan@marxist.se>, Simen Heggestøyl
> >  <simenheg@gmail.com>, emacs-devel@gnu.org
> > Date: Sat, 20 Mar 2021 23:37:40 +0100
> > From:  Theodor Thornhill via "Emacs development discussions." <emacs-devel@gnu.org>
> > 
> > +** 'project-shell' and 'shell' now uses 'pop-to-buffer-same-window'
>                                       ^^^^
> "use", in plural.  Also, the first line of a NEWS entry should be a
> complete sentence, so it should end with a period.

Oh, and one more nit: we generally prefer to update the manuals where
appropriate together with code changes, to avoid the need to tediously
go over all the manuals and update them when the time comes to release
the next Emacs version.  So if this change doesn't need any updates
for the manuals (I think it doesn't, but please make sure we don't say
there anything that could contradict the new behavior), then please
mark the NEWS entry with "---", per the note at the beginning of NEWS.

Thanks.

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Theodor Thornhill via Emacs development discussions.]

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

Hi, Eli!

> "use", in plural.  Also, the first line of a NEWS entry should be a
> complete sentence, so it should end with a period.
>
>> +This is to keep the same behavior as eshell has.
>
> I believe our convention is to use "Eshell", capitalized.
>

New patch with your comments addressed, hopefully sufficiently :)

> Thanks.

Thank you :)

--
Theo

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-pop-to-buffer-same-window-for-shell.patch --]
[-- Type: text/x-patch, Size: 2193 bytes --]

From fed04b8ba437043f30132fe362509ebf14ed0588 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sun, 21 Mar 2021 08:02:28 +0100
Subject: [PATCH] Use pop-to-buffer-same-window for shell

* lisp/progmodes/project.el (project-shell): Behave the same way as
'M-x project-eshell'.
* lisp/shell.el (shell): Behave the same way as 'M-x eshell'.
* etc/NEWS: Add news entry describing the change.
* lisp/tutorial.el: Use lexical-binding.
---
 etc/NEWS                  | 4 ++++
 lisp/progmodes/project.el | 2 +-
 lisp/shell.el             | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c602166397..49a4bb8106 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2300,6 +2300,10 @@ since the latter uses 'M-s' as a prefix key of the search prefix map.
 ** 'vc-print-branch-log' shows the change log for BRANCH from its root
 directory instead of the default directory.
 
+---
+** 'project-shell' and 'shell' now use 'pop-to-buffer-same-window'.
+This is to keep the same behavior as Eshell.
+
 \f
 * Incompatible Lisp Changes in Emacs 28.1
 
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index bd552c917a..b6a886f731 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -911,7 +911,7 @@ if one already exists."
                    "-shell*"))
          (shell-buffer (get-buffer default-project-shell-name)))
     (if (and shell-buffer (not current-prefix-arg))
-        (pop-to-buffer shell-buffer)
+        (pop-to-buffer-same-window shell-buffer)
       (shell (generate-new-buffer-name default-project-shell-name)))))
 
 ;;;###autoload
diff --git a/lisp/shell.el b/lisp/shell.el
index 53f5d0b6f1..7f4ca76547 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -743,7 +743,7 @@ Make the shell buffer the current buffer, and return it.
                  (current-buffer)))
   ;; The buffer's window must be correctly set when we call comint
   ;; (so that comint sets the COLUMNS env var properly).
-  (pop-to-buffer buffer)
+  (pop-to-buffer-same-window buffer)
 
   (with-connection-local-variables
    ;; On remote hosts, the local `shell-file-name' might be useless.
-- 
2.24.3 (Apple Git-128)

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 21.03.2021 09:04, Theodor Thornhill via Emacs development 
discussions. wrote:
> Hi, Eli!
> 
>> "use", in plural.  Also, the first line of a NEWS entry should be a
>> complete sentence, so it should end with a period.
>>
>>> +This is to keep the same behavior as eshell has.
>>
>> I believe our convention is to use "Eshell", capitalized.
>>
> 
> New patch with your comments addressed, hopefully sufficiently :)

Thanks, installed. :)

Re: master 432c1aa: Use `pop-to-buffer-same-window' in `project-eshell'

[Dmitry Gutov]

On 21.03.2021 00:10, Juri Linkov wrote:
> But it's fine also to change the format of uniquify as was discussed
> recently to "myproject/*shell*<3>" or "*shell*<myproject><3>",
> I don't remember what was the conclusion.

The intermediate conclusion is need more work and fixes done in uniquify.

I've posted an update to that thread just now.