bug#65386: [PATCH] ; Refine some 'package-vc' docstrings

[Philip Kaludercic]

Eshel Yaron <me@eshelyaron.com> writes:

>> Also, would it make sense to determine this at compile-time?  On the
>> other hand, if a VC backend is installed later on from ELPA, we would
>> want the custom type to reflect this.
>
> Yes, I couldn't find a way to defer computing the set of candidates to
> "customization type", I'm not sure if that even makes total sense.  I
> think it's not that crucial since someone adding a VC backend and
> immediately trying to customize these options seems to me like a very
> minor edge case, and we had the same issue prior to my patch anyhow.

Likely yes, but if we have two mostly equivalent solutions, I think that
flexibility without having to restart Emacs is preferable to performance.

[...]

> From 8a339d3bffe81f80e7e968505c40855d6598b8ac Mon Sep 17 00:00:00 2001
> From: Eshel Yaron <me@eshelyaron.com>
> Date: Sun, 20 Aug 2023 16:20:54 +0200
> Subject: [PATCH] ; Refine 'defcustom' types in 'package-vc'
>
> Only include VC backends that support cloning in the ':type' of
> 'package-vc-heuristic-alist' and 'package-vc-default-backend'.
>
> * lisp/emacs-lisp/package-vc.el (package-vc--cloning-backend-p)
> (package-vc--backends): New functions.
> (package-vc-heuristic-alist, package-vc-default-backend): Use it.
> ---
>  lisp/emacs-lisp/package-vc.el | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index a3762d252b0..44a4624e49f 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -62,6 +62,15 @@ package-vc
>  (defconst package-vc--elpa-packages-version 1
>    "Version number of the package specification format understood by 
> package-vc.")
>  
> +(defun package-vc--cloning-backend-p (be)
> +  "Return non-nil if the VC backend BE supports cloning repositories."
> +  (or (vc-find-backend-function be 'clone)
> +      (alist-get 'clone (get be 'vc-functions))))
> +
> +(defun package-vc--backends ()
> +  "Return a list of VC backends suitable for cloning package VCS 
> repositories."
> +  (seq-filter #'package-vc--cloning-backend-p vc-handled-backends))

Do you think that these utility functions are useful enough to have in
general, or would it be better to just define a defconst once that
generates the `(choice ...)' object?

> +
>  (defcustom package-vc-heuristic-alist
>    `((,(rx bos "http" (? "s") "://"
>            (or (: (? "www.") "github.com"
> @@ -105,7 +114,7 @@ package-vc-heuristic-alist
>    :type `(alist :key-type (regexp :tag "Regular expression matching URLs")
>                  :value-type (choice :tag "VC Backend"
>                                      ,@(mapcar (lambda (b) `(const ,b))
> -                                              vc-handled-backends)))
> +                                              (package-vc--backends))))
>    :version "29.1")
>  
>  (defcustom package-vc-default-backend 'Git
> @@ -117,7 +126,7 @@ package-vc-default-backend
>  The value must be a member of `vc-handled-backends' that supports
>  the `clone' VC function."
>    :type `(choice ,@(mapcar (lambda (b) (list 'const b))
> -                           vc-handled-backends))
> +                           (package-vc--backends)))
>    :version "29.1")
>  
>  (defcustom package-vc-register-as-project t