bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


Hello!

This patch is directly dependent on https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41868
since I reused one of the functions added by Philip K.

For now I rely on that report to be resolved before this.

However, attached is a patch for project-switch-to-buffer, where only file-bound buffers are included. I believe the function should be kind of self explanatory.

I use this daily, but stole the buffer list function since it was better than my initial one, and seems to be accepted soon :)

I find this useful, and hope others will as well. I bind it myself to 'C-x p b'.

The 'project--list-buffers' will not be part of the final patch, but I wanted to provide it so that you have a working example.

Theodor


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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 218058b195..65f3285705 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -753,6 +753,35 @@ project-compile
          (default-directory (project-root pr)))
     (compile command comint)))
 
+(defun project--list-buffers (pr)
+  "Return a list of all buffers in project PR."
+  (let ((root (project-root pr))
+        bufs)
+    (dolist (buf (buffer-list))
+      (when-let* ((path (or (buffer-file-name buf)
+                            (buffer-local-value 'default-directory buf)))
+                  (true (file-truename path)))
+        (when (file-in-directory-p true root)
+          (push buf bufs))))
+    bufs))
+
+(defun project--buffer-file-names (pr)
+  "Return the buffers in project PR associated with a file."
+  (mapcar #'buffer-name
+          (cl-remove-if-not
+           (lambda (buffer) (buffer-file-name buffer))
+           (project--list-buffers pr))))
+
+;;;###autoload
+(defun project-switch-to-buffer ()
+  "Switch to a buffer in the current project."
+  (interactive)
+  (let ((pr (project-current t)))
+    (switch-to-buffer
+     (project--completing-read-strict
+      "Switch to buffer"
+      (project--buffer-file-names pr)))))
+
 \f
 ;;; Project list
 

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Philip K.]

Cool! Just note that in the following patch I renamed the function to
project--buffer-list, so that it looks and acts more like
buffer-list. Sorry for the inconvenience though, didn't expect anyone
to pick the function up that quickly. 

-- 
	Philip K.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 15.06.2020 20:45, Theodor Thornhill wrote:
> I find this useful, and hope others will as well. I bind it myself to 'C-x p b'.

Sounds good.

> The 'project--list-buffers' will not be part of the final patch, but I wanted to provide it so that you have a working example.

I see that the code uses project--completing-read-strict. I'm not 
entirely happy with that function, yet.

Have you tried calling read-buffer with a PREDICATE argument instead? 
That would require extracting some code from project--list-buffers 
(defined in Philip's patch), but it should be straightforward.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


Hey,
"Dmitry Gutov" <dgutov@yandex.ru> writes:


[...]
> I see that the code uses project--completing-read-strict. I'm not
> entirely happy with that function, yet.
Ok, I just thought I'd use something already written :)

>
> Have you tried calling read-buffer with a PREDICATE argument instead?
> That would require extracting some code from project--list-buffers
> (defined in Philip's patch), but it should be straightforward.

The attached patch works fine, and is also not reliant on Philip's patch. I omitted the DEF argument, since then I had to do the filtering beforehand, and also in the PREDICATE function. Just seems a little redundant to me, but maybe it should be there

The complete patch is a lot smaller now as well!
What do you think?

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr-switch-to-buffer.patch --]
[-- Type: text/x-patch; name=pr-switch-to-buffer.patch, Size: 767 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 218058b195..fa50bbe57b 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -753,6 +753,20 @@ project-compile
          (default-directory (project-root pr)))
     (compile command comint)))
 
+;;;###autoload
+(defun project-switch-to-buffer ()
+  "Switch to a buffer in the current project."
+  (interactive)
+  (let* ((pr (project-current t)))
+    (switch-to-buffer
+     (read-buffer
+      "Switch-to-buffer: " nil nil
+      (lambda (buffer)
+        (when-let* ((path (buffer-file-name (cdr buffer)))
+                    (true (file-truename path)))
+          (when (file-in-directory-p true (project-root pr))
+            'buffer)))))))
+
 \f
 ;;; Project list
 

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

Thank you.

On 16.06.2020 01:05, Theodor Thornhill wrote:
> What do you think?

A few more notes. :-)

 > +      "Switch-to-buffer: " nil nil

The REQUIRE-MATCH argument should be t, I think.

 > +      (lambda (buffer)
 > +        (when-let* ((path (buffer-file-name (cdr buffer)))

The word "path" is reserved for something else (e.g. exec-path) in the 
GNU project.

Call it file or file-name.

 > +                    (true (file-truename path)))

Do we need this conversion? It'll add some runtime overhead, and I'm not 
sure in which conditions the result will be different.

 > +          (when (file-in-directory-p true (project-root pr))
 > +            'buffer)))))))

Why 'buffer and not t?

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


Thanks alot for your comments!

"Dmitry Gutov" <dgutov@yandex.ru> writes:

[...]
> The REQUIRE-MATCH argument should be t, I think.
Done.

>  > +      (lambda (buffer)
>  > +        (when-let* ((path (buffer-file-name (cdr buffer)))

[...]

> Call it file or file-name.
Done.


>  > +                    (true (file-truename path)))
>
> Do we need this conversion? It'll add some runtime overhead, and I'm not
> sure in which conditions the result will be different.
I removed it, seems to work fine :).


>  > +          (when (file-in-directory-p true (project-root pr))
>  > +            'buffer)))))))
>
> Why 'buffer and not t?
I just figured it as readable, but t is there now!

Theo


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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 218058b195..30d1d5a3dd 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -753,6 +753,19 @@ project-compile
          (default-directory (project-root pr)))
     (compile command comint)))
 
+;;;###autoload
+(defun project-switch-to-buffer ()
+  "Switch to a buffer in the current project."
+  (interactive)
+  (let ((root (project-root (project-current t))))
+    (switch-to-buffer
+     (read-buffer
+      "Switch-to-buffer: " nil t
+      (lambda (buffer)
+        (when-let ((file (buffer-file-name (cdr buffer))))
+          (when (file-in-directory-p file root)
+            t)))))))
+
 \f
 ;;; Project list
 

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Philip K.]

Theodor Thornhill <theo@thornhill.no> writes:

> +;;;###autoload
> +(defun project-switch-to-buffer ()
> +  "Switch to a buffer in the current project."
> +  (interactive)
> +  (let ((root (project-root (project-current t))))
> +    (switch-to-buffer
> +     (read-buffer
> +      "Switch-to-buffer: " nil t
> +      (lambda (buffer)
> +        (when-let ((file (buffer-file-name (cdr buffer))))
> +          (when (file-in-directory-p file root)
> +            t)))))))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Shouldn't this just be (file-in-directory-p file root), without the
(when ... t)?

-- 
	Philip K.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


Hi!

"Philip K." <philip@warpmail.net> writes:

[...]

> Shouldn't this just be (file-in-directory-p file root), without the
> (when ... t)?
Yes! Thanks a lot :)


Theo


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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 218058b195..a61b29abe6 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -753,6 +753,18 @@ project-compile
          (default-directory (project-root pr)))
     (compile command comint)))
 
+;;;###autoload
+(defun project-switch-to-buffer ()
+  "Switch to a buffer in the current project."
+  (interactive)
+  (let ((root (project-root (project-current t))))
+    (switch-to-buffer
+     (read-buffer
+      "Switch-to-buffer: " nil t
+      (lambda (buffer)
+        (when-let ((file (buffer-file-name (cdr buffer))))
+          (file-in-directory-p file root)))))))
+
 \f
 ;;; Project list
 

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Basil L. Contovounesios]

Theodor Thornhill <theo@thornhill.no> writes:

> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 218058b195..a61b29abe6 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -753,6 +753,18 @@ project-compile
>           (default-directory (project-root pr)))
>      (compile command comint)))
>  
> +;;;###autoload
> +(defun project-switch-to-buffer ()
> +  "Switch to a buffer in the current project."
> +  (interactive)
> +  (let ((root (project-root (project-current t))))
> +    (switch-to-buffer
> +     (read-buffer
> +      "Switch-to-buffer: " nil t

Nit: For consistency with switch-to-buffer,
this should be "Switch to buffer: " without the hyphens.

> +      (lambda (buffer)
> +        (when-let ((file (buffer-file-name (cdr buffer))))

Nit: Neither the manual nor read-buffer's docstring documents what
(cdr buffer) is here, so a comment mentioning Vbuffer_alist or something
along those lines would be nice.

> +          (file-in-directory-p file root)))))))
> +

Otherwise LGTM, thanks,

-- 
Basil

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

Hello!

"Basil L. Contovounesios" <contovob@tcd.ie> writes:


[...]
> Nit: For consistency with switch-to-buffer,
> this should be "Switch to buffer: " without the hyphens.

Yes, absolutely!

>> +      (lambda (buffer)
>> +        (when-let ((file (buffer-file-name (cdr buffer))))
>
> Nit: Neither the manual nor read-buffer's docstring documents what
> (cdr buffer) is here, so a comment mentioning Vbuffer_alist or something
> along those lines would be nice.
It kind of does, though indirectly? But sure, I can add a small comment.

Documentation states that:

 "It will be called with each potential candidate, in the form of either a
string or a cons cell whose ‘car’ is a string, and should return non-nil to
accept the candidate for completion, nil otherwise."

[...]
> Otherwise LGTM, thanks,
Nice

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Basil L. Contovounesios]

Theodor Thornhill <theo@thornhill.no> writes:

>>> +      (lambda (buffer)
>>> +        (when-let ((file (buffer-file-name (cdr buffer))))
>>
>> Nit: Neither the manual nor read-buffer's docstring documents what
>> (cdr buffer) is here, so a comment mentioning Vbuffer_alist or something
>> along those lines would be nice.
> It kind of does, though indirectly? But sure, I can add a small comment.
>
> Documentation states that:
>
>  "It will be called with each potential candidate, in the form of either a
> string or a cons cell whose ‘car’ is a string, and should return non-nil to
> accept the candidate for completion, nil otherwise."

I know, but this says nothing about the cdr.

-- 
Basil

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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

Hello,

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> I know, but this says nothing about the cdr.

Maybe like this?

Theo


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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f3df44fa7b..0361a3ecdd 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -744,6 +744,19 @@ project-compile
          (default-directory (project-root pr)))
     (compile command comint)))
 
+;;;###autoload
+(defun project-switch-to-buffer ()
+  "Switch to a buffer in the current project."
+  (interactive)
+  (let ((root (project-root (project-current t))))
+    (switch-to-buffer
+     (read-buffer
+      "Switch to buffer: " nil t
+      (lambda (buffer)
+        ;; buffer is a alist of ("filename" . #<buffer filename>)
+        (when-let ((file (buffer-file-name (cdr buffer))))
+          (file-in-directory-p file root)))))))
+
 \f
 ;;; Project list
 

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Basil L. Contovounesios]

Theodor Thornhill <theo@thornhill.no> writes:

> +;;;###autoload
> +(defun project-switch-to-buffer ()
> +  "Switch to a buffer in the current project."
> +  (interactive)
> +  (let ((root (project-root (project-current t))))
> +    (switch-to-buffer
> +     (read-buffer
> +      "Switch to buffer: " nil t
> +      (lambda (buffer)
> +        ;; buffer is a alist of ("filename" . #<buffer filename>)

That's not right: BUFFER is a flat cons cell, not an alist.  Please also
start sentences with a capital letter and end them with a full stop.
I'd write something like this:

  ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.

> +        (when-let ((file (buffer-file-name (cdr buffer))))
> +          (file-in-directory-p file root)))))))

Thanks,

-- 
Basil

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


Hi!
Thanks for your keen eye :)

> I'd write something like this:
>
>   ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
>

Got it. I stole your comment - I hope that is OK?


Theo

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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f3df44fa7b..52e8ef4ff9 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -744,6 +744,19 @@ project-compile
          (default-directory (project-root pr)))
     (compile command comint)))
 
+;;;###autoload
+(defun project-switch-to-buffer ()
+  "Switch to a buffer in the current project."
+  (interactive)
+  (let ((root (project-root (project-current t))))
+    (switch-to-buffer
+     (read-buffer
+      "Switch to buffer: " nil t
+      (lambda (buffer)
+        ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
+        (when-let ((file (buffer-file-name (cdr buffer))))
+          (file-in-directory-p file root)))))))
+
 \f
 ;;; Project list
 

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Basil L. Contovounesios]

Theodor Thornhill <theo@thornhill.no> writes:

> Thanks for your keen eye :)

Thanks for all your useful contributions!

>> I'd write something like this:
>>
>>   ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
>>
>
> Got it. I stole your comment - I hope that is OK?

Of course.
In decreasing order of preference: improved > stolen > ignored. ;)

> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index f3df44fa7b..52e8ef4ff9 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -744,6 +744,19 @@ project-compile
>           (default-directory (project-root pr)))
>      (compile command comint)))
>  
> +;;;###autoload
> +(defun project-switch-to-buffer ()
> +  "Switch to a buffer in the current project."
> +  (interactive)
> +  (let ((root (project-root (project-current t))))
> +    (switch-to-buffer
> +     (read-buffer
> +      "Switch to buffer: " nil t
> +      (lambda (buffer)
> +        ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
> +        (when-let ((file (buffer-file-name (cdr buffer))))
> +          (file-in-directory-p file root)))))))

LGTM, thanks.

-- 
Basil

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 16.06.2020 21:58, Theodor Thornhill wrote:
> Got it. I stole your comment - I hope that is OK?

Looking good, thanks.

I'll install it tomorrow or so.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 16.06.2020 21:58, Theodor Thornhill wrote:
> Got it. I stole your comment - I hope that is OK?

Also installed. Thanks!

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 16.06.2020 21:19, Basil L. Contovounesios wrote:
>    ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.

I don't think we can/should refer to C level variables from Lisp 
documentation.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Basil L. Contovounesios]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 16.06.2020 21:19, Basil L. Contovounesios wrote:
>>    ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
>
> I don't think we can/should refer to C level variables from Lisp documentation.

This is a source comment, not public documentation, and the relevant
code relies on an undocumented C-level implementation detail (the use of
Vbuffer_alist as a completion collection in read-buffer) so I think the
comment (or something along those lines; feel free to suggest
improvements) is appropriate.

-- 
Basil

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 17.06.2020 00:58, Basil L. Contovounesios wrote:
> This is a source comment, not public documentation, and the relevant
> code relies on an undocumented C-level implementation detail (the use of
> Vbuffer_alist as a completion collection in read-buffer) so I think the
> comment (or something along those lines; feel free to suggest
> improvements) is appropriate.

I see. All right then.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Juri Linkov]

> However, attached is a patch for project-switch-to-buffer, where only
> file-bound buffers are included.  I believe the function should be
> kind of self explanatory.
>
> I find this useful, and hope others will as well. I bind it myself to 'C-x p b'.

Nice, I consider your new project-switch-to-buffer as an accompanying
command with the existing project-find-file (that could get a keybinding too,
the most natural would be 'C-x p f').

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 16.06.2020 02:10, Juri Linkov wrote:
> Nice, I consider your new project-switch-to-buffer as an accompanying
> command with the existing project-find-file (that could get a keybinding too,
> the most natural would be 'C-x p f')

Would you like to create a patch that adds a global keymap for project 
related commands?

I've been putting it off myself. The question I've been considering:

Do we keep it in project.el or somewhere outside? Maybe the latter, 
since project.el is also an ELPA package, and we generally don't want 
packages to alter Emacs' key bindings right after installation.

OTOH, if the prefix is customizable, that can also be fine.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Hi Theodor,

> However, attached is a patch for project-switch-to-buffer, where only
> file-bound buffers are included. I believe the function should be kind
> of self explanatory.

Can you please tell why only file-bound buffers are included?  I have
some buffers with inferior python process in each project, would be nice
to switch to them with project-switch-to-buffer.

Also consider to provide default value like:

    (buffer-name (other-buffer (current-buffer)))

This way one can switch to recent project buffer with just RET, same way
as switch-to-buffer.

Thanks for your work!

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

> Hi Theodor,
Hi!

> Can you please tell why only file-bound buffers are included?  I have
> some buffers with inferior python process in each project, would be nice
> to switch to them with project-switch-to-buffer.

Well, I tried a few different ways of including other buffers as well, but emacs seemed to want to include earlier used minibuffers and all sorts of 'junk'.  I agree that these inferior processes should be interesting to include, though.  I can try to find some way to filter out the cruft a bit better!  Do you have a specific idea in mind to go from?  

> Also consider to provide default value like:
>
>     (buffer-name (other-buffer (current-buffer)))
>
> This way one can switch to recent project buffer with just RET, same way
> as switch-to-buffer.

This seems smart! I'll look into that as well :) Thanks!

Theo

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


On the other hand, what do you think about this one?

Now you will get buffers such as '*xref*' and I assume your inferior buffers as well. Could you try this? I added your 'other-buffer' thing also for good measure :)

Theo


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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index e24d81c1b4..e935cae280 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 ()
@@ -779,10 +788,12 @@ project-switch-to-buffer
   (let ((root (project-root (project-current t))))
     (switch-to-buffer
      (read-buffer
-      "Switch to buffer: " nil t
+      "Switch to buffer: "
+      (buffer-name (other-buffer (current-buffer)))
+      t
       (lambda (buffer)
         ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
-        (when-let ((file (buffer-file-name (cdr buffer))))
+        (when-let ((file (buffer-local-value 'default-directory (cdr buffer))))
           (file-in-directory-p file root)))))))
 
 (defcustom project-kill-buffers-skip-conditions

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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


That was a lousy diff, sorry...

Theo


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

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index e24d81c1b4..d4a2b7707e 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -779,10 +779,12 @@ project-switch-to-buffer
   (let ((root (project-root (project-current t))))
     (switch-to-buffer
      (read-buffer
-      "Switch to buffer: " nil t
+      "Switch to buffer: "
+      (buffer-name (other-buffer (current-buffer)))
+      t
       (lambda (buffer)
         ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
-        (when-let ((file (buffer-file-name (cdr buffer))))
+        (when-let ((file (buffer-local-value 'default-directory (cdr buffer))))
           (file-in-directory-p file root)))))))
 
 (defcustom project-kill-buffers-skip-conditions

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Theodor Thornhill <theo@thornhill.no> writes:

> That was a lousy diff, sorry...

Much better!

Few more things:

- Check if other-buffer is passes the predicate function.  The buffer
  from another project must not be the default.
- Exclude the current buffer from the buffers list to switch to.

Thanks!

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 18.06.2020 13:48, Andrii Kolomoiets wrote:
> Much better!
> 
> Few more things:
> 
> - Check if other-buffer is passes the predicate function.  The buffer
>    from another project must not be the default.
> - Exclude the current buffer from the buffers list to switch to.

Perhaps a good idea is to follow the example of 
internal-complete-buffer-except (called from read-buffer-to-switch).

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 18.06.2020 13:48, Andrii Kolomoiets wrote:
>> Much better!
>> Few more things:
>> - Check if other-buffer is passes the predicate function.  The
>> buffer
>>    from another project must not be the default.
>> - Exclude the current buffer from the buffers list to switch to.
>
> Perhaps a good idea is to follow the example of
> internal-complete-buffer-except (called from read-buffer-to-switch).

Isn't it enough to check `(not (eq (cdr buffer) (current-buffer)))` in
the predicate function?

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 18.06.2020 14:30, Andrii Kolomoiets wrote:
> Isn't it enough to check `(not (eq (cdr buffer) (current-buffer)))` in
> the predicate function?

It should also filter out the buffers where the names start with 
whitespace. Unless there are no other completions.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 18.06.2020 14:30, Andrii Kolomoiets wrote:
>> Isn't it enough to check `(not (eq (cdr buffer) (current-buffer)))` in
>> the predicate function?
>
> It should also filter out the buffers where the names start with
> whitespace. Unless there are no other completions.

No, it shouldn't.  read-buffer already shows normal buffers as
completions on TAB and shows hidden buffers as completions after SPC
is entered.  See internal-complete-buffer.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 18.06.2020 14:47, Andrii Kolomoiets wrote:
> No, it shouldn't.  read-buffer already shows normal buffers as
> completions on TAB and shows hidden buffers as completions after SPC
> is entered.  See internal-complete-buffer.

Ah, you're right. internal-complete-buffer is used either way.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

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

Hi Theodor,

Please take a look at the attached patch.  It's based on your patch and
resolves this:

> - Check if other-buffer is passes the predicate function.  The buffer
>   from another project must not be the default.
> - Exclude the current buffer from the buffers list to switch to.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-switch-to-buffer.patch --]
[-- Type: text/x-patch, Size: 1378 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 7a41df614b..b8d5731a71 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -777,14 +777,23 @@ project-compile
 (defun project-switch-to-buffer ()
   "Switch to a buffer in the current project."
   (interactive)
-  (let ((root (project-root (project-current t))))
+  (let* ((root (project-root (project-current t)))
+         (current-buffer (current-buffer))
+         (other-buffer (other-buffer current-buffer))
+         (other-name (buffer-name other-buffer))
+         (predicate
+          (lambda (buffer)
+            ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
+            (and (not (eq (cdr buffer) current-buffer))
+                 (when-let ((file (buffer-local-value 'default-directory (cdr buffer))))
+                   (file-in-directory-p file root))))))
     (switch-to-buffer
      (read-buffer
-      "Switch to buffer: " nil t
-      (lambda (buffer)
-        ;; BUFFER is an entry (BUF-NAME . BUF-OBJ) of Vbuffer_alist.
-        (when-let ((file (buffer-file-name (cdr buffer))))
-          (file-in-directory-p file root)))))))
+      "Switch to buffer: "
+      (when (funcall predicate (cons other-name other-buffer))
+        other-name)
+      t
+      predicate))))
 
 (defcustom project-kill-buffers-skip-conditions
   '("\\*Help\\*")

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

> Hi Theodor,
Hi!

> Please take a look at the attached patch.  
That's very cool! I was sitting right now doing something similar, but you beat me to it.  Sorry for not getting to it faster - life happened :)

I was struggling a little with the default buffer part, which you solved by kinda not solving it.  I was trying to determine in some way what buffer would be reasonable to offer as default value if that predicate fails.  Maybe it is best to just return nil there as you do. 

Maybe just one thing.  Eli addressed the docstring in one of the other functions as being a little ambiguous.  Maybe update it to something like:

"Switch to an open buffer in the current project."

In addition, maybe adding a small comment above the default value predicate, explaining the relation to the undocumentet structure used in this function.  Or not, maybe it is clear enough given the preceeding comment. 

Thank you for bringing this up - and fixing it!

Theo

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Basil L. Contovounesios]

Theodor Thornhill <theo@thornhill.no> writes:

> Maybe just one thing.  Eli addressed the docstring in one of the other functions
> as being a little ambiguous.  Maybe update it to something like:
>
> "Switch to an open buffer in the current project."

FWIW, it is unclear what "open" means in this context, as it is not a
term commonly used to describe buffers.  Possible alternatives include
whether it's displayed somewhere or buried, whether it's live or killed,
etc.

-- 
Basil

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 18.06.2020 22:35, Basil L. Contovounesios wrote:
> FWIW, it is unclear what "open" means in this context, as it is not a
> term commonly used to describe buffers.  Possible alternatives include
> whether it's displayed somewhere or buried, whether it's live or killed,
> etc.

And it's not like any command is going to help you switch to a killed 
buffer.

"Open" sounds like a tautology indeed.

Here's a option from me: "Switch to another buffer belonging to the 
current project".

But personally, I think the current docstring is easy to understand already.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Theodor Thornhill <theo@thornhill.no> writes:

> you beat me to it.  Sorry for not getting to it faster - life happened
> :)

No problem at all.

> I was struggling a little with the default buffer part, which you
> solved by kinda not solving it.  I was trying to determine in some way
> what buffer would be reasonable to offer as default value if that
> predicate fails.  Maybe it is best to just return nil there as you do.

Or we can pass the predicate function to the `other-buffer` function so
it will return only buffer from the current project too!  We just need
to make `other-buffer` accept predicate first :)

> Thank you for bringing this up - and fixing it!

And thank you for the original command! One local function less ;)

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 18.06.2020 21:03, Andrii Kolomoiets wrote:
> Please take a look at the attached patch.  It's based on your patch and
> resolves this:
> 
>> - Check if other-buffer is passes the predicate function.  The buffer
>>    from another project must not be the default.
>> - Exclude the current buffer from the buffers list to switch to.
> Thanks!

Thanks! Pushed.

Something to consider: switch-to-buffer still allows you to choose the 
current buffer. And the new command doesn't.

I'm not sure of the original logic behind this, though.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Something to consider: switch-to-buffer still allows you to choose the
> current buffer. And the new command doesn't.

Furthermore, by not requiring a match, switch-to-buffer allows to create
buffers.

> I'm not sure of the original logic behind this, though.

Theodor, can you please tell the reason why is REQUIRE-MATCH set to t in
the read-buffer call?

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Theodor Thornhill]

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

> Theodor, can you please tell the reason why is REQUIRE-MATCH set to t in
> the read-buffer call?

I believe Dmitry told me to, actually. I can see if I can find the mail. I’m fine with it being nil, as it was in the first patch.

Theo

>

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

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 19.06.2020 07:59, Andrii Kolomoiets wrote:
> Furthermore, by not requiring a match, switch-to-buffer allows to create
> buffers.

Right.

Do you need this ability? I would imagine switch-to-buffer is always 
available for that.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 19.06.2020 07:59, Andrii Kolomoiets wrote:
>> Furthermore, by not requiring a match, switch-to-buffer allows to create
>> buffers.
>
> Right.
>
> Do you need this ability? I would imagine switch-to-buffer is always
> available for that.

Well, I rarely use this ability of switch-to-buffer.  I can't say I
really need this in project-switch-to-buffer.

But to keep consistency with switch-to-buffer, project-switch-to-buffer
should behave the same.

Also, in my understanding, match is should be required on e.g. killing
buffers when existing buffer is required.  Because switch-to-buffer is
able to create buffers, I can't see the reason why
project-switch-to-buffer should require the match.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 20.06.2020 11:35, Andrii Kolomoiets wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 19.06.2020 07:59, Andrii Kolomoiets wrote:
>>> Furthermore, by not requiring a match, switch-to-buffer allows to create
>>> buffers.
>>
>> Right.
>>
>> Do you need this ability? I would imagine switch-to-buffer is always
>> available for that.
> 
> Well, I rarely use this ability of switch-to-buffer.  I can't say I
> really need this in project-switch-to-buffer.
> 
> But to keep consistency with switch-to-buffer, project-switch-to-buffer
> should behave the same.

There are other differences, too. The arglist, for example.

> Also, in my understanding, match is should be required on e.g. killing
> buffers when existing buffer is required.  Because switch-to-buffer is
> able to create buffers, I can't see the reason why
> project-switch-to-buffer should require the match.

To better protect from typos? It's not a very strong argument, of 
course. But "just for consistency" isn't one either.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> But to keep consistency with switch-to-buffer,
>> project-switch-to-buffer
>> should behave the same.
>
> There are other differences, too. The arglist, for example.

Well, NORECORD and FORCE-SAME-WINDOW can be passed to switch-to-buffer
easily if there is the need to.

>> buffers when existing buffer is required.  Because switch-to-buffer is
>> able to create buffers, I can't see the reason why
>> project-switch-to-buffer should require the match.
>
> To better protect from typos? It's not a very strong argument, of
> course. But "just for consistency" isn't one either.

How about "To be able to create buffers"? ;)  This way there are no need
to use switch-to-buffer for those who will use project-switch-to-buffer
solely.
Imagine project-switch-to-buffer is in your muscle memory. You need to
perform additional steps to create the buffer (C-g C-x b
type-buffer-name-again) just because the project-switch-to-buffer can't
create buffers with no reason.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

On 22.06.2020 12:30, Andrii Kolomoiets wrote:

>>> But to keep consistency with switch-to-buffer,
>>> project-switch-to-buffer
>>> should behave the same.
>>
>> There are other differences, too. The arglist, for example.
> 
> Well, NORECORD and FORCE-SAME-WINDOW can be passed to switch-to-buffer
> easily if there is the need to.

Everything can be added later. Adding new function arguments could be a 
bit more problematic than a slight change in semantics like 
REQUIRE-MATCH=nil, though.

>>> buffers when existing buffer is required.  Because switch-to-buffer is
>>> able to create buffers, I can't see the reason why
>>> project-switch-to-buffer should require the match.
>>
>> To better protect from typos? It's not a very strong argument, of
>> course. But "just for consistency" isn't one either.
> 
> How about "To be able to create buffers"? ;)  This way there are no need
> to use switch-to-buffer for those who will use project-switch-to-buffer
> solely.
> Imagine project-switch-to-buffer is in your muscle memory. You need to
> perform additional steps to create the buffer (C-g C-x b
> type-buffer-name-again) just because the project-switch-to-buffer can't
> create buffers with no reason.

All right. I'll change it to nil now.

Not sure I understand the passion behind your argument, though, since 
you said you're not going to need this yourself. But perhaps I 
misunderstood.

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> Imagine project-switch-to-buffer is in your muscle memory. You need to
>> perform additional steps to create the buffer (C-g C-x b
>> type-buffer-name-again) just because the project-switch-to-buffer can't
>> create buffers with no reason.
>
> All right. I'll change it to nil now.
>
> Not sure I understand the passion behind your argument, though, since
> you said you're not going to need this yourself. But perhaps I 
> misunderstood.

I'm rarely use this feature, but when I'll need it it's great that
`project-switch-to-buffer` support it.

Thank you!

bug#41879: 28.0.50; [Patch]: Add project-switch-to-buffer in project.el

[Dmitry Gutov]

[-- Attachment #1: Type: text/html, Size: 360 bytes --]