GNU bug report logs - #65386
[PATCH] ; Refine some 'package-vc' docstrings

Previous Next

Full log

Message #41 received at 65386 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 65386 <at> debbugs.gnu.org
Subject: Re: bug#65386: [PATCH] ; Refine some 'package-vc' docstrings
Date: Sun, 20 Aug 2023 18:22:28 +0200

Philip Kaludercic <philipk <at> posteo.net> writes:

> Eshel Yaron <me <at> 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.
>

Hmm I'm not sure I completely follow.  Which two solutions do you have
in mind?  I agree that it would be nice have the `:type` updated if the
user adds a relevant VC backend, but I don't think `defcustom` supports
something like that.  How do you propose to achieve that flexibility?

> [...]
>
>> From 8a339d3bffe81f80e7e968505c40855d6598b8ac Mon Sep 17 00:00:00 2001
>> From: Eshel Yaron <me <at> 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?
>

I think these are nice to have, but I don't mind removing them in favor
of a `defconst` (or a `define-widget` for that matter) if you think
that's better.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.