[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#13291: The package description buffer needs an URL button
From: |
Dmitry Gutov |
Subject: |
bug#13291: The package description buffer needs an URL button |
Date: |
Tue, 12 Mar 2013 15:49:36 +0400 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130307 Thunderbird/17.0.4 |
On 11.03.2013 21:40, Stefan Monnier wrote:
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.
Not a problem, I rather expected some other developers familiar with
this package to chime in.
+(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.
Thanks, good to know. I wasn't aware that `nthcdr' is implemented in C.
-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.
Sounds good.
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. ]
Hmm, I kinda assumed that the simpler structure would result in better,
not worse, theoretical performance. Food for thought.
But I'm not really sure if we're ever going to want to iterate over the
extra properties. Right now, where it's accessed, the change to alist
will mostly mean going from
(plist-get (package-desc-meta desc) :homepage)
to
(cdr (assoc :homepage (package-desc-meta desc)))
which is not as nice.
(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?
Yes.
Why not add a 5th field instead?
It actually has a 5th field already (containing the name of the
repository). This is a bit complicated, because we also have
`package-alist', its entries don't have the last two elements, and we
want the homepage url to be available in both. But I guess we could push
the new element before `kind' instead.
This may be moot now, now that we have the updated Daniel's
defstruct-based rewrite, which uses struct accessor functions here, so
adding another slot would be the way to go.
(Since it has a test suite, merging this patch on top of it should be
easier than going the other way around).
(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.
One of the things you haven't commented on is that the vectors in the
archive-contents file are of variable length with this proposal.
Should
(cl-lib . [(0 2) nil "Prefixed!" single :homepage "http://foo"])
turn into
(cl-lib . [(0 2) nil "Prefixed!" single (:homepage "http://foo")])
or maybe
(cl-lib . [(0 2) nil "Prefixed!" single (:homepage . "http://foo")])
?