Subject: Re: bug#13291: The package description buffer needs an URL button

From: Stefan Monnier <[email-masked]>
Date: Mon, 11 Mar 2013 13:40:06 -0400

> And here's the updated patch for package.el, with saving the new metadata
> to -pkg.el file when a single-file package is being installed, and with
> support for it in `package-install-file'.

Sorry for taking so long.  Here are some comments.

> +(require 'cl-lib)

AFAICT, you only do that for cl-cddddr, but:
- cl-cddddr only requires cl-lib at compile-time.
- "nthcdr 4" is both shorter and faster.

> -The vector DESC has the form [VERSION-LIST REQS DOCSTRING].
> +The vector DESC has the form [VERSION-LIST REQS DOCSTRING META].
[...]
> +  META is a property list mapping metadata keywords to values.

VERSION-LIST, REQS, and DOCSTRING are also metadata, so I'd call the new
entry something like EXTRA or EXTRA-PROPS rather than META.

Also, I'd personally use an alist rather than a plist, tho it's largely
a question of taste (I prefer alist because they have a bit more
structure, which in turn lets you use things like mapcar, memq, dolist,
... on them).
[ From a theoretical efficiency viewpoint they are also half as deep as
  plists, so in an "ideal" PRAM world, they'd be about twice as fast,
  although in reality I doubt there is any measurable difference.  ]

>  (defsubst package-desc-kind (desc)
>    "Extract the kind of download from an archive package description vector."
> +  (plist-get (package-desc-meta desc) :kind))
> +
> +(defsubst package-desc-meta (desc)
> +  "Extract the metadata property list from a package description vector."
>    (aref desc 3))

Hmm... does that mean that the 4th field of each vector in
`archive-contents' is changed from holding either `tar' or `single' to
holding a plist?
Why not add a 5th field instead?
 
>  (defun define-package (name-string version-string
>  				&optional docstring requirements
> -				&rest _extra-properties)
> +				&rest extra-properties)
[...]
> -EXTRA-PROPERTIES is currently unused."
> +EXTRA-PROPERTIES is a property list mapping additional metadata
> +keywords (e.g. `:homepage') to values."

I guess here I agree that a plist is more convenient for the
package maintainer.

        Stefan