bug#73357: [PATCH] Make vc-clone interactive

bug#73357: [PATCH] Make vc-clone interactive

[Aleksandr Vityazev via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi,

Cloning is used quite often, so I would like to have an interactive
command. A patch is attached to the email. WDYT?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Make vc-clone interactive --]
[-- Type: text/x-patch, Size: 3031 bytes --]

From 1fd3aa4b3afe4064bcc78787b99f4fa336e4031d Mon Sep 17 00:00:00 2001
Message-ID: <1fd3aa4b3afe4064bcc78787b99f4fa336e4031d.1726751543.git.avityazev@disroot.org>
From: Aleksandr Vityazev <avityazev@disroot.org>
Date: Thu, 19 Sep 2024 16:11:31 +0300
Subject: [PATCH] Make vc-clone interactive

* lisp/vc/vc.el (vc-clone): Make interactive.
(vc--remotes-history): New defvar.
---
 lisp/vc/vc.el | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 597a1622f5a..d3d3a302d45 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -3804,6 +3804,8 @@ vc-check-headers
   (interactive)
   (vc-call-backend (vc-backend buffer-file-name) 'check-headers))
 
+(defvar vc--remotes-history)
+
 (defun vc-clone (remote &optional backend directory rev)
   "Clone repository REMOTE using version-control BACKEND, into DIRECTORY.
 If successful, return the string with the directory of the checkout;
@@ -3815,19 +3817,31 @@ vc-clone
 backend in `vc-handled-backends' until one succeeds to clone REMOTE.
 If REV is non-nil, it indicates a specific revision to check out after
 cloning; the syntax of REV depends on what BACKEND accepts."
-  (setq directory (expand-file-name (or directory default-directory)))
-  (if backend
-      (progn
-        (unless (memq backend vc-handled-backends)
-          (error "Unknown VC backend %s" backend))
-        (vc-call-backend backend 'clone remote directory rev))
-    (catch 'ok
-      (dolist (backend vc-handled-backends)
-        (ignore-error vc-not-supported
-          (when-let ((res (vc-call-backend
-                           backend 'clone
-                           remote directory rev)))
-            (throw 'ok res)))))))
+  (interactive
+   (list (read-string "Remote: " nil 'vc--remotes-history)
+         (intern (completing-read "Backend: " vc-handled-backends nil t))
+         (expand-file-name
+          (read-directory-name "Clone dir: "))
+         (read-string "Revision (RET if not needed): ")))
+  (let ((directory (expand-file-name (or directory default-directory)))
+        (rev (unless (string-empty-p rev) rev)))
+    (setq directory
+          (if backend
+              (progn
+                (unless (memq backend vc-handled-backends)
+                  (error "Unknown VC backend %s" backend))
+                (vc-call-backend backend 'clone remote directory rev))
+            (catch 'ok
+              (dolist (backend vc-handled-backends)
+                (ignore-error vc-not-supported
+                  (when-let ((res (vc-call-backend
+                                   backend 'clone
+                                   remote directory rev)))
+                    (throw 'ok res)))))))
+    (when (file-directory-p directory)
+      (if (called-interactively-p 'interactive)
+          (find-file directory)
+        directory))))
 
 (declare-function log-view-current-tag "log-view" (&optional pos))
 (defun vc-default-last-change (_backend file line)
-- 
2.46.0


[-- Attachment #3: Type: text/plain, Size: 39 bytes --]



-- 
Best regards,
Aleksandr Vityazev

bug#73357: [PATCH] Make vc-clone interactive

[Eli Zaretskii]

> Date: Thu, 19 Sep 2024 16:18:16 +0300
> From:  Aleksandr Vityazev via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Cloning is used quite often, so I would like to have an interactive
> command. A patch is attached to the email. WDYT?

Thanks, but making this function a command will take more than just
adding the interactive form.  I think we need at least:

  . mention that in interactive usage the command prompts for the
    argument (why do we have to prompt for REV, btw?)
  . announce the change in NEWS
  . maybe update the user manual
  . maybe make the command fall back to 'checkout' method if 'clone'
    is not supported

> +    (when (file-directory-p directory)
> +      (if (called-interactively-p 'interactive)
> +          (find-file directory)
> +        directory))))

This changes the value returned by the function from what it did
before, no?

bug#73357: [PATCH] Make vc-clone interactive

[Aleksandr Vityazev via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On 2024-09-19 16:36, Eli Zaretskii wrote:

>> Date: Thu, 19 Sep 2024 16:18:16 +0300
>> From:  Aleksandr Vityazev via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Cloning is used quite often, so I would like to have an interactive
>> command. A patch is attached to the email. WDYT?
>
> Thanks, but making this function a command will take more than just
> adding the interactive form.  I think we need at least:
>
>   . mention that in interactive usage the command prompts for the
>     argument (why do we have to prompt for REV, btw?)

I clarified in the docstring. We can agree that we shouldn't prompt for REV
when cloning interactively, removed.

>   . announce the change in NEWS
Announced but did not mark the news with +++ or ---.
>   . maybe update the user manual
maybe if I have to, I'll do it.
>   . maybe make the command fall back to 'checkout' method if 'clone'
>     is not supported

it's worth thinking about, because I can't say for sure right now if
it's worth it. And how to do this when vc-checkout requires a file as an
argument.

>> +    (when (file-directory-p directory)
>> +      (if (called-interactively-p 'interactive)
>> +          (find-file directory)
>> +        directory))))
>
> This changes the value returned by the function from what it did
> before, no?

If the function is called from the code, it returns nil or the
directory, just like in the previous version. Or am I missing
something?

V2 patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Make vc-clone interactive --]
[-- Type: text/x-patch, Size: 3811 bytes --]

From b302b5c42e01fe0b6f7607128ed660babf55e35a Mon Sep 17 00:00:00 2001
Message-ID: <b302b5c42e01fe0b6f7607128ed660babf55e35a.1726762586.git.avityazev@disroot.org>
From: Aleksandr Vityazev <avityazev@disroot.org>
Date: Thu, 19 Sep 2024 16:11:31 +0300
Subject: [PATCH] Make vc-clone interactive

* lisp/vc/vc.el (vc-clone): Make interactive.
Mention this in the doc string.
(vc--remotes-history): New defvar.
* etc/NEWS: Announce it.
---
 etc/NEWS      |  7 +++++++
 lisp/vc/vc.el | 42 ++++++++++++++++++++++++++++--------------
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b52ad001a2e..db5f05c823c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -359,6 +359,13 @@ functionality of the standard 'xref' commands in TeX buffers.  You can
 restore the standard 'etags' backend with the 'M-x xref-etags-mode'
 toggle.
 
+** VC
+
+*** 'vc-clone' is now an interactive command.
+When called interactively, 'vc-clone' now prompts for the address of the
+remote repository, the backend that will be used for cloning, as well as
+the directory where the repository will be cloned.
+
 \f
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 597a1622f5a..fc964803a02 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -3804,6 +3804,8 @@ vc-check-headers
   (interactive)
   (vc-call-backend (vc-backend buffer-file-name) 'check-headers))
 
+(defvar vc--remotes-history)
+
 (defun vc-clone (remote &optional backend directory rev)
   "Clone repository REMOTE using version-control BACKEND, into DIRECTORY.
 If successful, return the string with the directory of the checkout;
@@ -3814,20 +3816,32 @@ vc-clone
 If BACKEND is nil or omitted, the function iterates through every known
 backend in `vc-handled-backends' until one succeeds to clone REMOTE.
 If REV is non-nil, it indicates a specific revision to check out after
-cloning; the syntax of REV depends on what BACKEND accepts."
-  (setq directory (expand-file-name (or directory default-directory)))
-  (if backend
-      (progn
-        (unless (memq backend vc-handled-backends)
-          (error "Unknown VC backend %s" backend))
-        (vc-call-backend backend 'clone remote directory rev))
-    (catch 'ok
-      (dolist (backend vc-handled-backends)
-        (ignore-error vc-not-supported
-          (when-let ((res (vc-call-backend
-                           backend 'clone
-                           remote directory rev)))
-            (throw 'ok res)))))))
+cloning; the syntax of REV depends on what BACKEND accepts.
+If called interactively, prompt for REMOTE, BACKEND and DIRECTORY in
+the minibuffer."
+  (interactive
+   (list (read-string "Remote: " nil 'vc--remotes-history)
+         (intern (completing-read "Backend: " vc-handled-backends nil t))
+         (expand-file-name
+          (read-directory-name "Clone dir: "))))
+  (let ((directory (expand-file-name (or directory default-directory))))
+    (setq directory
+          (if backend
+              (progn
+                (unless (memq backend vc-handled-backends)
+                  (error "Unknown VC backend %s" backend))
+                (vc-call-backend backend 'clone remote directory rev))
+            (catch 'ok
+              (dolist (backend vc-handled-backends)
+                (ignore-error vc-not-supported
+                  (when-let ((res (vc-call-backend
+                                   backend 'clone
+                                   remote directory rev)))
+                    (throw 'ok res)))))))
+    (when (file-directory-p directory)
+      (if (called-interactively-p 'interactive)
+          (find-file directory)
+        directory))))
 
 (declare-function log-view-current-tag "log-view" (&optional pos))
 (defun vc-default-last-change (_backend file line)
-- 
2.46.0


[-- Attachment #3: Type: text/plain, Size: 38 bytes --]


-- 
Best regards,
Aleksandr Vityazev