Re: feature/package-vc has been merged

[Philip Kaludercic]

Rudolf Adamkovič <salutis@me.com> writes:

> Rudolf Adamkovič <salutis@me.com> writes:
>
>> OMG, the time has come!  Thank you.
>
> I would like to share first-time user experience with you.  Perhaps you
> will find it interesting.  Here it goes:
>
> I found `package-vc-selected-packages' in seconds thanks to its familiar
> name, nicely matching `package-selected-packages'.  Well done!  It
> surprised me, though, that the variable does not accept bare package
> names, such as `modus-themes' instead of `(modus-themes . nil)'.

It would be trivial to implement, the main issue is that I wanted to use
`alist' as the user option type.  If instead the type were to be
replaced with some

    (repeat (choice symbol (cons symbol string) ...))

it might work.

But keep in mind that (modus-themes . nil) is the same as (modus-themes).

> Then I looked for `package-vc-install-selected-packages`, and troubles
> started.  I found no such function, so I decided to consult the
> documentation for `package-vc-selected-packages' which says:
>
>   You can also use the function ‘package-vc-selected-packages’ to apply
>   the changes.

> Confused, I could not find any such function.  I also tried `M-x
> package-vc TAB' but that did not help either.  So, I read the source
> code, I found `package-vc-ensure-packages', and that worked.  Phew!

Right, that was a mistake in the documentation that Eli also pointed
out.  If you think the name-symmetry between package and package-vc is
helpful, we could rename that one to
`package-vc-install-selected-packages'.

> I then added `package-vc-ensure-packages' to my Emacs init file, but
> Emacs did not launch cleanly, for it could not find the function.  So, I
> added an explicit `require' for `package-vc'.

What I had in mind was for `package-vc-selected-packages' to be used as
is.  It is an autoloaded option with a custom setter that installs all
"selected packages" as a side effect.

As the manual says, all you need to do is write

    (setopt package-vc-selected-packages '((modus-themes)))

Should that be highlighted more explicitly in the documentation?  I had
actually initially hesitated in exposing `package-vc-ensure-packages' as
a public function at all, let alone auto-loading it.

> Finally, it all worked.  I installed Modus Themes!
>
> Next, I looked for `package-vc-update', but found nothing.  I then found
> `package-vc-refresh' but that sounded like `package-refresh-contents'
> which does not update packages.  Confusing!

Hmm, I forgot about `package-refresh-contents'.  What `package-vc-refresh'
does is package installing dependencies, generating autoloads, building
the documentation, byte compiling, etc.  It exists so that if the user
changes something, they can "activate" their changes without having to
do all those things automatically.

> Still, I ran the `package-vc-refresh' function, and it prompted me with
> "Refresh package:".  I hit RET, just to try something, and it failed
> with
>
>   funcall-interactively: Wrong number of arguments: #<subr
>   package-vc-refresh>, 0

Interesting...

> So, I tried again with "modus-themes", which I installed with
> `package-vc-ensure-packages', and got
>
>   execute-extended-command: Wrong type argument: listp,
> #s(package-desc modus-themes (3 0 0) "Elegant, highly legible and
> customizable themes" ((emacs (27 1))) vc nil
> "/Users/salutis/.emacs.d/elpa/modus-themes" ((:url
> . "https://git.sr.ht/~protesilaos/modus-themes";) (:keywords "faces"
> "theme" "accessibility") (:maintainer "Modus-Themes Development"
> . "~protesilaos/modus-themes@lists.sr.ht") (:authors ("Protesilaos
> Stavrou" . "info@protesilaos.com")) (:commit
> . "f1149c18005d31638b3701f9a89819b133b4360e")) nil)

Ah, the interactive spec for `package-vc-refresh' is missing a (list
...).  Is trying to apply a package description as an argument list,
which doesn't make sense.  This will fix the issue:

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index a0b4b03118..127a7e073f 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -692,7 +692,7 @@ package-vc-install-from-checkout
 (defun package-vc-refresh (pkg-desc)
   "Refresh the installation for package given by PKG-DESC.
 Interactively, prompt for the name of the package to refresh."
-  (interactive (package-vc--read-pkg "Refresh package: "))
+  (interactive (list (package-vc--read-pkg "Refresh package: ")))
   (package-vc--unpack-1 pkg-desc (package-desc-dir pkg-desc)))
 
 (defun package-vc--read-pkg (prompt)

Perhaps the function should be called `-reinstall', or `-reactivate',
but I am uncertain about all these names too.

> Ignoring the refresh woes, I suggest the following design changes:
>
>   - allow strings in `-selected-packages' if possible

Can be done.

>   - make `-ensure-packages' interactive for `M-x'

Can be done.

>   - rename `-ensure-' to `-install-selected-' for consistency

Can be done.

>   - auto-load `-install-selected-packages' for common use

Not a fan of that, but I guess it can be done.

>   - rename `-refresh' to `-update' for consistency

That would be wrong, because `package-vc-refresh' doesn't update the
package.  There is `package-vc-update' and `package-update' (that do the
same thing for source packages).  Maybe `package-vc-update' can be made
interactive too and made to only prompt for source packages.

>   - add `-update-all' for consistency

That already exists, in `package-update-all'.  It will also update
source packages.  What you have in mind would be a
`package-vc-update-all' that would only update all the source packages.

>   - fix the nonexistent function reference in the documentation

Right, we are on that already.

> Then, the existing users of `package.el' will know how to use a large
> part of `package-vc.el' without needing to learn that "ensure" means
> "install-selected", "refresh" means "update", and so on.

I agree on these points.

> Rudy