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

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

[Dmitry Gutov]

We can get the addresses from the semi-standard URL header, it's 
mentioned on these two pages, so at least some people are using it:

http://emacswiki.org/emacs/AutomaticFileHeaders
http://marmalade-repo.org/doc-files/package.5.html#SINGLE-FILE-PACKAGES

Plus, when the header's absent, the package repository can try to add 
the field automatically. Melpa can derive it from the recipe settings, 
and GNU Elpa can at least show links to the repository browser.
Or maybe to the pages at elpa.gnu.org. They basically contain the same 
information as the description buffer, though, so probably not.

Also see https://github.com/milkypostman/melpa/issues/424.

Any opposing opinions?

--Dmitry

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

[Stefan Monnier]

I don't see any problem with this idea.

> Elpa can at least show links to the repository browser.
> Or maybe to the pages at elpa.gnu.org.

It should point to the pages at elpa.gnu.org.

> They basically contain the same information as the description buffer,
> though, so probably not.

The are meant to be the "canonical page" for the package.  Admittedly,
they seem to contain the same info as the description buffer, but they
already contain a bit more info (a pointer to the repository browser, a list
of past revisions) and that could grow over time.


        Stefan

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On 12.01.2013 7:28, Stefan Monnier wrote:
> I don't see any problem with this idea.

Terrific. Patch attached.

Note that I haven't tested it with an actual package repository 
supporting this extension, just with modified -pkg.el and 
archive-contents files.

>> Elpa can at least show links to the repository browser.
>> Or maybe to the pages at elpa.gnu.org.
>
> It should point to the pages at elpa.gnu.org.
>
>> They basically contain the same information as the description buffer,
>> though, so probably not.
>
> The are meant to be the "canonical page" for the package.  Admittedly,
> they seem to contain the same info as the description buffer, but they
> already contain a bit more info (a pointer to the repository browser, a list
> of past revisions) and that could grow over time.

Sounds good to me.

Hopefully, the code responsible for it will take care of the case when 
those pages are not in fact canonical, like org, auctex, etc.

[-- Attachment #2: package-homepage-button.diff --]
[-- Type: text/plain, Size: 4516 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-01-11 23:24:52 +0000
+++ lisp/ChangeLog	2013-01-12 07:33:28 +0000
@@ -1,3 +1,16 @@
+2013-01-12  Dmitry Gutov  <gutov@vbx>
+
+	* emacs-lisp/package.el (package-desc-kind): Get the kind from the
+	metadata plist.
+	(package-desc-meta): Return metadata plist.
+	(define-package): Store EXTRA-PROPERTIES as the 4th element of
+	the package data vector.
+	(package--add-to-archive-contents): Instead of just package kind,
+	use the 4th element of the vector for the matadata.  Include kind
+	in the metadata.
+	(describe-package-1): When the package metadata includes
+	`:homepage', display a link button for it (bug#13291).
+
 2013-01-11  Felix H. Dahlke  <fhd@ubercode.de>
 
 	* progmodes/js.el: Fix multiline declarations's indentation (bug#8576).

=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el	2013-01-01 09:11:05 +0000
+++ lisp/emacs-lisp/package.el	2013-01-12 07:23:54 +0000
@@ -302,12 +302,13 @@
 Each element has the form (PKG . DESC), where PKG is a package
 name (a symbol) and DESC is a vector that describes the package.
 
-The vector DESC has the form [VERSION-LIST REQS DOCSTRING].
+The vector DESC has the form [VERSION-LIST REQS DOCSTRING META].
   VERSION-LIST is a version list.
   REQS is a list of packages required by the package, each
    requirement having the form (NAME VL) where NAME is a string
    and VL is a version list.
   DOCSTRING is a brief description of the package.
+  META is a property list mapping metadata keywords to values.
 
 This variable is set automatically by `package-load-descriptor',
 called via `package-initialize'.  To change which packages are
@@ -426,6 +427,10 @@
 
 (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))
 
 (defun package--dir (name version)
@@ -525,7 +530,7 @@
 
 (defun define-package (name-string version-string
 				&optional docstring requirements
-				&rest _extra-properties)
+				&rest extra-properties)
   "Define a new package.
 NAME-STRING is the name of the package, as a string.
 VERSION-STRING is the version of the package, as a string.
@@ -533,8 +538,8 @@
 REQUIREMENTS is a list of dependencies on other packages.
  Each requirement is of the form (OTHER-PACKAGE OTHER-VERSION),
  where OTHER-VERSION is a string.
-
-EXTRA-PROPERTIES is currently unused."
+EXTRA-PROPERTIES is a property list mapping additional metadata
+keywords (e.g. `:homepage') to values."
   (let* ((name (intern name-string))
 	 (version (version-to-list version-string))
 	 (new-pkg-desc
@@ -545,7 +550,8 @@
 			   (list (car elt)
 				 (version-to-list (car (cdr elt)))))
 			 requirements)
-			docstring)))
+			docstring
+			extra-properties)))
 	 (old-pkg (assq name package-alist)))
     (cond
      ;; If there's no old package, just add this to `package-alist'.
@@ -853,8 +859,15 @@
 Also, add the originating archive to the end of the package vector."
   (let* ((name    (car package))
          (version (package-desc-vers (cdr package)))
+         (data    (append (cdr package) nil))
+         (ex-len  (- (length data) 3))
+         (extras  (last data ex-len))
          (entry   (cons name
-			(vconcat (cdr package) (vector archive))))
+                        (vconcat (nbutlast data ex-len)
+                                 ;; Save the kind and any following
+                                 ;; keyword-value pairs as metadata.
+                                 (vector (cons :kind extras)
+                                         archive))))
          (existing-package (assq name package-archive-contents)))
     (cond ((not existing-package)
 	   (add-to-list 'package-archive-contents entry))
@@ -1261,7 +1274,13 @@
 	  (help-insert-xref-button text 'help-package name))
 	(insert "\n")))
     (insert "    " (propertize "Summary" 'font-lock-face 'bold)
-	    ": " (if desc (package-desc-doc desc)) "\n\n")
+	    ": " (if desc (package-desc-doc desc)) "\n")
+    (let ((homepage (plist-get (package-desc-meta desc) :homepage)))
+      (when homepage
+        (insert "   " (propertize "Homepage" 'font-lock-face 'bold) ": ")
+        (help-insert-xref-button homepage 'help-url homepage)
+        (insert "\n")))
+    (insert "\n")
 
     (if built-in
 	;; For built-in packages, insert the commentary.

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

[Dmitry Gutov]

On 12.01.2013 11:41, Dmitry Gutov wrote:
> On 12.01.2013 7:28, Stefan Monnier wrote:
>> I don't see any problem with this idea.
>
> Terrific. Patch attached.
>
> Note that I haven't tested it with an actual package repository
> supporting this extension, just with modified -pkg.el and
> archive-contents files.

To clarify, here's how they look:

smex-pkg.el:

(define-package "smex" "20120915.2041"
"M-x interface with Ido-style fuzzy matching. [source: github]"
(quote nil) :homepage "https://github.com/nonsequitur/smex")

archive-contents:

(1
  (zotelo .
          [(20121024 1114)
           nil "Manage Zotero collections from emacs [source: github]"
           single
           :homepage "https://github.com/vitoshka/zotelo"])
  ...)

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 56 bytes --]

Here's a rough patch for elpa/admin/archive-contents.el

[-- Attachment #2: archive-contents-homepage.diff --]
[-- Type: text/plain, Size: 4642 bytes --]

=== modified file 'admin/archive-contents.el' (properties changed: -x to +x)
--- admin/archive-contents.el	2012-11-29 17:21:45 +0000
+++ admin/archive-contents.el	2013-01-13 06:17:37 +0000
@@ -122,15 +122,16 @@
 
 (defun archive--simple-package-p (dir pkg)
   "Test whether DIR contains a simple package named PKG.
-If so, return a list (VERSION DESCRIPTION REQ COMMENTARY), where
-VERSION is the version string of the simple package, DESCRIPTION
-is the brief description of the package, REQ is a list of
-requirements, and COMMENTARY is the package commentary.
+If so, return a list (VERSION DESCRIPTION REQ COMMENTARY
+METADATA...), where VERSION is the version string of the simple
+package, DESCRIPTION is the brief description of the package, REQ
+is a list of requirements, COMMENTARY is the package commentary,
+and METADATA is a property list with additional metadata.
 Otherwise, return nil."
   (let* ((pkg-file (expand-file-name (concat pkg "-pkg.el") dir))
 	 (mainfile (expand-file-name (concat pkg ".el") dir))
          (files (directory-files dir nil archive-re-no-dot))
-	 version description req commentary)
+	 version description req commentary homepage)
     (dolist (file (prog1 files (setq files ())))
       (unless (string-match "\\.elc\\'" file)
         (push file files)))
@@ -157,11 +158,15 @@
                 (setq req (mapcar 'archive--convert-require
                                   (car (read-from-string requires-str))))))
           (setq commentary (lm-commentary))
-          (list version description req commentary))))
+          (setq homepage (or (lm-homepage)
+                             (format "http://elpa.gnu.org/packages/%s.html" pkg)))
+          (list version description req commentary
+                :homepage homepage))))
      ((not (file-exists-p pkg-file))
       (error "Can find single file nor package desc file in %s" dir)))))
 
-(defun archive--process-simple-package (dir pkg vers desc req commentary)
+(defun archive--process-simple-package (dir pkg vers desc req commentary
+                                            &rest extra-properties)
   "Deploy the contents of DIR into the archive as a simple package.
 Rename DIR/PKG.el to PKG-VERS.el, delete DIR, and write the
 package commentary to PKG-readme.txt.  Return the descriptor."
@@ -205,7 +210,9 @@
       (save-buffer)
       (kill-buffer)))
   (delete-directory dir t)
-  (cons (intern pkg) (vector (version-to-list vers) req desc 'single)))
+  (cons (intern pkg) (vconcat
+                      (vector (version-to-list vers) req desc 'single)
+                      extra-properties)))
 
 (defun archive--make-changelog (dir)
   "Export Bzr log info of DIR into a ChangeLog file."
@@ -239,7 +246,9 @@
     (when (file-exists-p readme)
       (copy-file readme (concat pkg "-readme.txt") 'ok-if-already-exists))
     (rename-file dir (concat pkg "-" vers))
-    (cons (intern pkg) (vector (version-to-list vers) req (nth 3 exp) 'tar))))
+    (cons (intern pkg) (vconcat
+                        (vector (version-to-list vers) req (nth 3 exp) 'tar)
+                        (cdr (cddddr exp))))))
 
 (defun archive--multi-file-package-def (dir pkg)
   "Reurn the `define-package' form in the file DIR/PKG-pkg.el."
@@ -294,7 +303,8 @@
       ;; FIXME: Don't compile the -pkg.el files!
       (byte-recompile-directory dir 0))))
 
-(defun archive--write-pkg-file (pkg-dir name version desc requires &rest ignored)
+(defun archive--write-pkg-file (pkg-dir name version desc requires
+                                        &rest extra-properties)
   (let ((pkg-file (expand-file-name (concat name "-pkg.el") pkg-dir))
 	(print-level nil)
 	(print-length nil))
@@ -302,17 +312,19 @@
      (concat (format ";; Generated package description from %s.el\n"
 		     name)
 	     (prin1-to-string
-	      (list 'define-package
-		    name
-		    version
-		    desc
-		    (list 'quote
-			  ;; Turn version lists into string form.
-			  (mapcar
-			   (lambda (elt)
-			     (list (car elt)
-				   (package-version-join (cadr elt))))
-			   requires))))
+              (nconc
+               (list 'define-package
+                     name
+                     version
+                     desc
+                     (list 'quote
+                           ;; Turn version lists into string form.
+                           (mapcar
+                            (lambda (elt)
+                              (list (car elt)
+                                    (package-version-join (cadr elt))))
+                            requires)))
+               extra-properties))
 	     "\n")
      nil
      pkg-file)))

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 238 bytes --]

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'.

Again, probably less tested that it should be.

[-- Attachment #2: package-homepage-button.diff --]
[-- Type: text/plain, Size: 10283 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-01-12 19:24:27 +0000
+++ lisp/ChangeLog	2013-01-13 07:54:01 +0000
@@ -1,3 +1,27 @@
+2013-01-13  Dmitry Gutov  <dgutov@yandex.ru>
+
+	* emacs-lisp/package.el (package-desc-kind): Get the kind from the
+	metadata plist.
+	(package-desc-meta): Return metadata plist.
+	(define-package): Store EXTRA-PROPERTIES as the 4th element of
+	the package data vector.
+	(package--add-to-archive-contents): Instead of just package kind,
+	use the 4th element of the vector for the matadata.  Include kind
+	in the metadata.
+	(describe-package-1): When the package metadata includes
+	`:homepage', display a link button for it (bug#13291).
+	(package-unpack-single): Accept a 5th argument, with metadata.
+	Appent it to the `define-package' form.
+	(package-download-single): Accept and pass on the META argument.
+	(package-download-transaction): Pass the package metadata to
+	`package-download-single'.
+	(package-buffer-info): Return the package metadata (currently with
+	just homepage) as the 6th vector argument.
+	(package-tar-file-info): Same.  Like most of the elements of the
+	returned vector, though, it won't be used by the caller.
+	(package-install-from-buffer): Get package metadata from PKG-INFO
+	and pass it to `package-unpack-single'.
+
 2013-01-12  Michael Albinus  <michael.albinus@gmx.de>
 
 	* autorevert.el (auto-revert-notify-watch-descriptor): Give it

=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el	2013-01-01 09:11:05 +0000
+++ lisp/emacs-lisp/package.el	2013-01-13 07:44:34 +0000
@@ -170,6 +170,7 @@
 ;;; Code:
 
 (require 'tabulated-list)
+(require 'cl-lib)
 
 (defgroup package nil
   "Manager for Emacs Lisp packages."
@@ -302,12 +303,13 @@
 Each element has the form (PKG . DESC), where PKG is a package
 name (a symbol) and DESC is a vector that describes the package.
 
-The vector DESC has the form [VERSION-LIST REQS DOCSTRING].
+The vector DESC has the form [VERSION-LIST REQS DOCSTRING META].
   VERSION-LIST is a version list.
   REQS is a list of packages required by the package, each
    requirement having the form (NAME VL) where NAME is a string
    and VL is a version list.
   DOCSTRING is a brief description of the package.
+  META is a property list mapping metadata keywords to values.
 
 This variable is set automatically by `package-load-descriptor',
 called via `package-initialize'.  To change which packages are
@@ -426,6 +428,10 @@
 
 (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))
 
 (defun package--dir (name version)
@@ -525,7 +531,7 @@
 
 (defun define-package (name-string version-string
 				&optional docstring requirements
-				&rest _extra-properties)
+				&rest extra-properties)
   "Define a new package.
 NAME-STRING is the name of the package, as a string.
 VERSION-STRING is the version of the package, as a string.
@@ -533,8 +539,8 @@
 REQUIREMENTS is a list of dependencies on other packages.
  Each requirement is of the form (OTHER-PACKAGE OTHER-VERSION),
  where OTHER-VERSION is a string.
-
-EXTRA-PROPERTIES is currently unused."
+EXTRA-PROPERTIES is a property list mapping additional metadata
+keywords (e.g. `:homepage') to values."
   (let* ((name (intern name-string))
 	 (version (version-to-list version-string))
 	 (new-pkg-desc
@@ -545,7 +551,8 @@
 			   (list (car elt)
 				 (version-to-list (car (cdr elt)))))
 			 requirements)
-			docstring)))
+			docstring
+			extra-properties)))
 	 (old-pkg (assq name package-alist)))
     (cond
      ;; If there's no old package, just add this to `package-alist'.
@@ -642,7 +649,7 @@
   (let ((buffer-file-coding-system 'no-conversion))
     (write-region (point-min) (point-max) file-name)))
 
-(defun package-unpack-single (file-name version desc requires)
+(defun package-unpack-single (file-name version desc requires meta)
   "Install the contents of the current buffer as a package."
   ;; Special case "package".
   (if (string= file-name "package")
@@ -661,17 +668,19 @@
 	(write-region
 	 (concat
 	  (prin1-to-string
-	   (list 'define-package
-		 file-name
-		 version
-		 desc
-		 (list 'quote
-		       ;; Turn version lists into string form.
-		       (mapcar
-			(lambda (elt)
-			  (list (car elt)
-				(package-version-join (cadr elt))))
-			requires))))
+           (nconc
+            (list 'define-package
+                  file-name
+                  version
+                  desc
+                  (list 'quote
+                        ;; Turn version lists into string form.
+                        (mapcar
+                         (lambda (elt)
+                           (list (car elt)
+                                 (package-version-join (cadr elt))))
+                         requires)))
+            meta))
 	  "\n")
 	 nil
 	 pkg-file
@@ -721,12 +730,12 @@
 						       (end-of-line)
 						       (point)))))))
 
-(defun package-download-single (name version desc requires)
+(defun package-download-single (name version desc requires meta)
   "Download and install a single-file package."
   (let ((location (package-archive-base name))
 	(file (concat (symbol-name name) "-" version ".el")))
     (package--with-work-buffer location file
-      (package-unpack-single (symbol-name name) version desc requires))))
+      (package-unpack-single (symbol-name name) version desc requires meta))))
 
 (defun package-download-tar (name version)
   "Download and install a tar package."
@@ -853,8 +862,15 @@
 Also, add the originating archive to the end of the package vector."
   (let* ((name    (car package))
          (version (package-desc-vers (cdr package)))
+         (data    (append (cdr package) nil))
+         (ex-len  (- (length data) 3))
+         (extras  (last data ex-len))
          (entry   (cons name
-			(vconcat (cdr package) (vector archive))))
+                        (vconcat (nbutlast data ex-len)
+                                 ;; Save the kind and any following
+                                 ;; keyword-value pairs as metadata.
+                                 (vector (cons :kind extras)
+                                         archive))))
          (existing-package (assq name package-archive-contents)))
     (cond ((not existing-package)
 	   (add-to-list 'package-archive-contents entry))
@@ -886,7 +902,8 @@
        ((eq kind 'single)
 	(package-download-single elt v-string
 				 (package-desc-doc desc)
-				 (package-desc-reqs desc)))
+				 (package-desc-reqs desc)
+                                 (package-desc-meta desc)))
        (t
 	(error "Unknown package kind: %s" (symbol-name kind))))
       ;; If package A depends on package B, then A may `require' B
@@ -942,7 +959,7 @@
   "Return a vector describing the package in the current buffer.
 The vector has the form
 
-   [FILENAME REQUIRES DESCRIPTION VERSION COMMENTARY]
+   [FILENAME REQUIRES DESCRIPTION VERSION COMMENTARY META]
 
 FILENAME is the file name, a string, sans the \".el\" extension.
 REQUIRES is a list of requirements, each requirement having the
@@ -950,6 +967,7 @@
 DESCRIPTION is the package description, a string.
 VERSION is the version, a string.
 COMMENTARY is the commentary section, a string, or nil if none.
+META is a property list with additional metadata.
 
 If the buffer does not contain a conforming package, signal an
 error.  If there is a package, narrow the buffer to the file's
@@ -975,7 +993,8 @@
 	   (pkg-version
 	    (or (package-strip-rcs-id (lm-header "package-version"))
 		(package-strip-rcs-id (lm-header "version"))))
-	   (commentary (lm-commentary)))
+	   (commentary (lm-commentary))
+           (homepage (lm-homepage)))
       (unless pkg-version
 	(error
 	 "Package lacks a \"Version\" or \"Package-Version\" header"))
@@ -986,7 +1005,8 @@
 	       (list (car elt)
 		     (version-to-list (car (cdr elt)))))
 	     requires))
-      (vector file-name requires desc pkg-version commentary))))
+      (vector file-name requires desc pkg-version commentary
+              (list :homepage homepage)))))
 
 (defun package-tar-file-info (file)
   "Find package information for a tar file.
@@ -1013,6 +1033,7 @@
 	    (version-string (nth 2 pkg-def-parsed))
 	    (docstring      (nth 3 pkg-def-parsed))
 	    (requires       (nth 4 pkg-def-parsed))
+            (meta           (cdr (cl-cddddr pkg-def-parsed)))
 	    (readme (shell-command-to-string
 		     ;; Requires GNU tar.
 		     (concat "tar -xOf " file " "
@@ -1032,7 +1053,7 @@
 			(list (car elt)
 			      (version-to-list (cadr elt))))
 		      requires))
-	(vector pkg-name requires docstring version-string readme)))))
+	(vector pkg-name requires docstring version-string readme meta)))))
 
 ;;;###autoload
 (defun package-install-from-buffer (pkg-info type)
@@ -1052,14 +1073,15 @@
 	     (desc (if (string= (aref pkg-info 2) "")
 		       "No description available."
 		     (aref pkg-info 2)))
-	     (pkg-version (aref pkg-info 3)))
+	     (pkg-version (aref pkg-info 3))
+             (meta (aref pkg-info 5)))
 	;; Download and install the dependencies.
 	(let ((transaction (package-compute-transaction nil requires)))
 	  (package-download-transaction transaction))
 	;; Install the package itself.
 	(cond
 	 ((eq type 'single)
-	  (package-unpack-single file-name pkg-version desc requires))
+	  (package-unpack-single file-name pkg-version desc requires meta))
 	 ((eq type 'tar)
 	  (package-unpack (intern file-name) pkg-version))
 	 (t
@@ -1261,7 +1283,13 @@
 	  (help-insert-xref-button text 'help-package name))
 	(insert "\n")))
     (insert "    " (propertize "Summary" 'font-lock-face 'bold)
-	    ": " (if desc (package-desc-doc desc)) "\n\n")
+	    ": " (if desc (package-desc-doc desc)) "\n")
+    (let ((homepage (plist-get (package-desc-meta desc) :homepage)))
+      (when homepage
+        (insert "   " (propertize "Homepage" 'font-lock-face 'bold) ": ")
+        (help-insert-xref-button homepage 'help-url homepage)
+        (insert "\n")))
+    (insert "\n")
 
     (if built-in
 	;; For built-in packages, insert the commentary.

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

[Dmitry Gutov]

Hey all,

Could someone take a look at package-homepage-button.diff and 
package-homepage-button.diff I submitted previously? I'd like to install 
them sometime soon.

I'll test them again before installing and will deal with any fallout, 
so the main issue I'd like an additional opinion on is the changes in 
the formats of pkg.el and archive-contents files 
(http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13291#17).

For example, is it okay that the definition vectors in archive-contents 
can be of variable length, or would it be better to keep all extra 
metadata in one plist, in 4th element?
IIRC, I chose the former because the package type (single or not) ends 
up in the same metadata structure, but changing the format of the 3rd 
element would break backward compatibility. So it makes some sense to 
keep the rest of the metadata as separate elements, too.

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

[Stefan Monnier]

> 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

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

[Dmitry Gutov]

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")])

?

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

Here's an updated patch for package, package-x and tests.

Please comment, I'd like to install it soon-ish.

Notes:

* Converting from plist to alist and back is a hassle, but it gives us 
an opportunity to clear out keys with nil values in 
`package-desc-from-define'.
* Not passing :homepage to `package-desc-from-define' in 
`package-buffer-info' when its value is nil seems hard.
* `package--add-to-archive-contents' tries to retain backward 
compatibility by checking the given vector's length.

Now we just need a package archive that would include homepage information.

[-- Attachment #2: package-homepage-button-new.diff --]
[-- Type: text/x-patch, Size: 16895 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-08-06 12:18:43 +0000
+++ lisp/ChangeLog	2013-08-07 09:14:53 +0000
@@ -1,3 +1,22 @@
+2013-08-07  Dmitry Gutov  <dgutov@yandex.ru>
+
+	* emacs-lisp/package.el (package-desc-from-define): Accept
+	additional arguments as plist, convert it to an alist and store it
+	in the `extras' slot.
+	(package-generate-description-file): Convert extras alist back to
+	plist and append to the `define-package' form arguments.
+	(package--alist-to-plist): New function.
+	(package--ac-desc): Add `extras' slot.
+	(package--add-to-archive-contents): Check if the archive-contents
+	vector is long enough, and if it is, pass its `extras' slot value
+	to `package-desc-create'.
+	(package-buffer-info): Call `lm-homepage', pass the returned value
+	to `package-desc-from-define'.
+	(describe-package-1): Render the homepage button.
+
+	* emacs-lisp/package-x.el (package-upload-buffer-internal): Pass
+	`extras' slot from `package-desc' to `package-make-ac-desc'.
+
 2013-08-06  Juanma Barranquero  <lekktu@gmail.com>
 
 	* frameset.el (frameset, frameset-filter-alist)

=== modified file 'lisp/emacs-lisp/package-x.el'
--- lisp/emacs-lisp/package-x.el	2013-06-27 09:26:54 +0000
+++ lisp/emacs-lisp/package-x.el	2013-08-07 08:31:50 +0000
@@ -209,6 +209,7 @@
                 (pcase file-type
                   (`single (lm-commentary))
                   (`tar nil))) ;; FIXME: Get it from the README file.
+               (extras (package-desc-extras pkg-desc))
 	       (pkg-version (package-version-join split-version))
 	       (pkg-buffer (current-buffer)))
 
@@ -217,7 +218,7 @@
 	  (let ((contents (or (package--archive-contents-from-url archive-url)
 			      (package--archive-contents-from-file)))
 		(new-desc (package-make-ac-desc
-                           split-version requires desc file-type)))
+                           split-version requires desc file-type extras)))
 	    (if (> (car contents) package-archive-version)
 		(error "Unrecognized archive version %d" (car contents)))
 	    (let ((elt (assq pkg-name (cdr contents))))

=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el	2013-08-03 02:34:22 +0000
+++ lisp/emacs-lisp/package.el	2013-08-07 08:51:42 +0000
@@ -296,7 +296,7 @@
                (:constructor
                 package-desc-from-define
                 (name-string version-string &optional summary requirements
-                 &key kind archive &allow-other-keys
+                 &rest rest-plist
                  &aux
                  (name (intern name-string))
                  (version (version-to-list version-string))
@@ -305,7 +305,19 @@
                                          (version-to-list (cadr elt))))
                                (if (eq 'quote (car requirements))
                                    (nth 1 requirements)
-                                 requirements))))))
+                                 requirements)))
+                 (kind (plist-get rest-plist :kind))
+                 (archive (plist-get rest-plist :archive))
+                 (extras (let (alist)
+                           (cl-remf rest-plist :kind)
+                           (cl-remf rest-plist :archive)
+                           (while rest-plist
+                             (let ((value (cadr rest-plist)))
+                               (when value
+                                 (push (cons (car rest-plist) value)
+                                       alist)))
+                             (setq rest-plist (cddr rest-plist)))
+                           alist)))))
   "Structure containing information about an individual package.
 Slots:
 
@@ -327,14 +339,17 @@
 	package came.
 
 `dir'	The directory where the package is installed (if installed),
-	`builtin' if it is built-in, or nil otherwise."
+	`builtin' if it is built-in, or nil otherwise.
+
+`extras' Optional alist of additional keyword-value pairs."
   name
   version
   (summary package--default-summary)
   reqs
   kind
   archive
-  dir)
+  dir
+  extras)
 
 ;; Pseudo fields.
 (defun package-desc-full-name (pkg-desc)
@@ -635,22 +650,28 @@
       (write-region
        (concat
         (prin1-to-string
-         (list 'define-package
-               (symbol-name name)
-               (package-version-join (package-desc-version pkg-desc))
-               (package-desc-summary pkg-desc)
-               (let ((requires (package-desc-reqs pkg-desc)))
-                 (list 'quote
-                       ;; Turn version lists into string form.
-                       (mapcar
-                        (lambda (elt)
-                          (list (car elt)
-                                (package-version-join (cadr elt))))
-                        requires)))))
+         (nconc
+          (list 'define-package
+                (symbol-name name)
+                (package-version-join (package-desc-version pkg-desc))
+                (package-desc-summary pkg-desc)
+                (let ((requires (package-desc-reqs pkg-desc)))
+                  (list 'quote
+                        ;; Turn version lists into string form.
+                        (mapcar
+                         (lambda (elt)
+                           (list (car elt)
+                                 (package-version-join (cadr elt))))
+                         requires))))
+          (package--alist-to-plist
+           (package-desc-extras pkg-desc))))
         "\n")
        nil
        pkg-file))))
 
+(defun package--alist-to-plist (alist)
+  (apply #'nconc (mapcar (lambda (pair) (list (car pair) (cdr pair))) alist)))
+
 (defun package-unpack (pkg-desc)
   "Install the contents of the current buffer as a package."
   (let* ((name (package-desc-name pkg-desc))
@@ -886,10 +907,10 @@
 ;; Changing this defstruct implies changing the format of the
 ;; "archive-contents" files.
 (cl-defstruct (package--ac-desc
-               (:constructor package-make-ac-desc (version reqs summary kind))
+               (:constructor package-make-ac-desc (version reqs summary kind extras))
                (:copier nil)
                (:type vector))
-  version reqs summary kind)
+  version reqs summary kind extras)
 
 (defun package--add-to-archive-contents (package archive)
   "Add the PACKAGE from the given ARCHIVE if necessary.
@@ -904,7 +925,11 @@
            :reqs (package--ac-desc-reqs (cdr package))
            :summary (package--ac-desc-summary (cdr package))
            :kind (package--ac-desc-kind (cdr package))
-           :archive archive))
+           :archive archive
+           :extras (and (> (length (cdr package)) 4)
+                        ;; Older archive-contents files have only 4
+                        ;; elements here.
+                        (package--ac-desc-extras (cdr package)))))
          (existing-packages (assq name package-archive-contents))
          (pinned-to-archive (assoc name package-pinned-packages)))
     (cond
@@ -997,14 +1022,16 @@
 	   ;; probably wants us to use it.  Otherwise try Version.
 	   (pkg-version
 	    (or (package-strip-rcs-id (lm-header "package-version"))
-		(package-strip-rcs-id (lm-header "version")))))
+		(package-strip-rcs-id (lm-header "version"))))
+           (homepage (lm-homepage)))
       (unless pkg-version
 	(error
 	 "Package lacks a \"Version\" or \"Package-Version\" header"))
       (package-desc-from-define
        file-name pkg-version desc
        (if requires-str (package-read-from-string requires-str))
-       :kind 'single))))
+       :kind 'single
+       :homepage homepage))))
 
 (declare-function tar-get-file-descriptor "tar-mode" (file))
 (declare-function tar--extract "tar-mode" (descriptor))
@@ -1173,6 +1200,8 @@
          (reqs (if desc (package-desc-reqs desc)))
          (version (if desc (package-desc-version desc)))
          (archive (if desc (package-desc-archive desc)))
+         (homepage (if desc (cdr (assoc :homepage
+                                        (package-desc-extras desc)))))
          (built-in (eq pkg-dir 'builtin))
          (installable (and archive (not built-in)))
          (status (if desc (package-desc-status desc) "orphan")))
@@ -1241,7 +1270,10 @@
 	(insert "\n")))
     (insert "    " (propertize "Summary" 'font-lock-face 'bold)
 	    ": " (if desc (package-desc-summary desc)) "\n")
-
+    (when homepage
+      (insert "   " (propertize "Homepage" 'font-lock-face 'bold) ": ")
+      (help-insert-xref-button homepage 'help-url homepage)
+      (insert "\n"))
     (let* ((all-pkgs (append (cdr (assq name package-alist))
                              (cdr (assq name package-archive-contents))
                              (let ((bi (assq name package--builtins)))

=== modified file 'test/ChangeLog'
--- test/ChangeLog	2013-08-05 01:32:00 +0000
+++ test/ChangeLog	2013-08-07 09:43:50 +0000
@@ -1,3 +1,29 @@
+2013-08-07  Dmitry Gutov  <dgutov@yandex.ru>
+
+	* automated/package-test.el (simple-single-desc-1-4): Remove, it
+	was unused.
+	(simple-single-desc): Expect :homepage property.
+	(multi-file-desc): Same.
+	(with-package-test): Do not save previous `default-directory'
+	value, let-bind the var instead.
+	(package-test-install-single): Expect :homepage property in the
+	generated pkg file.
+	(package-test-describe-package): Expect Homepage button.
+	(package-test-describe-non-installed-package)
+	(package-test-describe-non-installed-multi-file-package): Same.
+	(package-test-describe-not-installed-package): Remove, it was a
+	duplicate.
+
+	* automated/package-x-test.el
+	(package-x-test--single-archive-entry-1-3): Expect :homepage
+	property.
+	(package-x-test--single-archive-entry-1-4): Expect nil extras slot.
+
+	* automated/data/package/simple-single-1.3.el: Add URL header.
+
+	* automated/data/package/archive-contents: Add :homepage
+	properties to `simple-single' and `multi-file'.
+
 2013-08-05  Glenn Morris  <rgm@gnu.org>
 
 	* automated/mule-util.el: New file, with tests extracted from

=== modified file 'test/automated/data/package/archive-contents'
--- test/automated/data/package/archive-contents	2013-06-27 09:26:54 +0000
+++ test/automated/data/package/archive-contents	2013-08-07 08:37:09 +0000
@@ -1,10 +1,12 @@
 (1
  (simple-single .
                 [(1 3)
-                 nil "A single-file package with no dependencies" single])
+                 nil "A single-file package with no dependencies" single
+                 ((:homepage . "http://doodles.au"))])
  (simple-depend .
                 [(1 0)
                  ((simple-single (1 3))) "A single-file package with a dependency." single])
  (multi-file .
              [(0 2 3)
-              nil "Example of a multi-file tar package" tar]))
+              nil "Example of a multi-file tar package" tar
+              ((:homepage . "http://puddles.li"))]))

=== modified file 'test/automated/data/package/multi-file-0.2.3.tar'
Binary files test/automated/data/package/multi-file-0.2.3.tar	2013-06-27 09:26:54 +0000 and test/automated/data/package/multi-file-0.2.3.tar	2013-08-06 22:11:14 +0000 differ

=== modified file 'test/automated/data/package/simple-single-1.3.el'
--- test/automated/data/package/simple-single-1.3.el	2013-06-27 09:26:54 +0000
+++ test/automated/data/package/simple-single-1.3.el	2013-08-07 08:36:44 +0000
@@ -3,6 +3,7 @@
 ;; Author: J. R. Hacker <jrh@example.com>
 ;; Version: 1.3
 ;; Keywords: frobnicate
+;; URL: http://doodles.au
 
 ;;; Commentary:
 

=== modified file 'test/automated/package-test.el'
--- test/automated/package-test.el	2013-07-11 16:01:26 +0000
+++ test/automated/package-test.el	2013-08-07 09:44:09 +0000
@@ -47,16 +47,10 @@
   (package-desc-create :name 'simple-single
                        :version '(1 3)
                        :summary "A single-file package with no dependencies"
-                       :kind 'single)
+                       :kind 'single
+                       :extras '((:homepage . "http://doodles.au")))
   "Expected `package-desc' parsed from simple-single-1.3.el.")
 
-(defvar simple-single-desc-1-4
-  (package-desc-create :name 'simple-single
-                       :version '(1 4)
-                       :summary "A single-file package with no dependencies"
-                       :kind 'single)
-  "Expected `package-desc' parsed from simple-single-1.4.el.")
-
 (defvar simple-depend-desc
   (package-desc-create :name 'simple-depend
                        :version '(1 0)
@@ -69,7 +63,8 @@
   (package-desc-create :name 'multi-file
                        :version '(0 2 3)
                        :summary "Example of a multi-file tar package"
-                       :kind 'tar)
+                       :kind 'tar
+                       :extras '((:homepage . "http://puddles.li")))
   "Expected `package-desc' from \"multi-file-0.2.3.tar\".")
 
 (defvar new-pkg-desc
@@ -100,7 +95,7 @@
           (package-user-dir package-test-user-dir)
           (package-archives `(("gnu" . ,package-test-data-dir)))
           (old-yes-no-defn (symbol-function 'yes-or-no-p))
-          (old-pwd default-directory)
+          (default-directory package-test-file-dir)
           package--initialized
           package-alist
           ,@(if update-news
@@ -131,8 +126,7 @@
        (when (and (boundp 'package-test-archive-upload-base)
                   (file-directory-p package-test-archive-upload-base))
          (delete-directory package-test-archive-upload-base t))
-       (setf (symbol-function 'yes-or-no-p) old-yes-no-defn)
-       (cd old-pwd))))
+       (setf (symbol-function 'yes-or-no-p) old-yes-no-defn))))
 
 (defmacro with-fake-help-buffer (&rest body)
   "Execute BODY in a temp buffer which is treated as the \"*Help*\" buffer."
@@ -232,7 +226,9 @@
         (should (string= (buffer-string)
                          (concat "(define-package \"simple-single\" \"1.3\" "
                                  "\"A single-file package "
-                                 "with no dependencies\" 'nil)\n"))))
+                                 "with no dependencies\" 'nil "
+                                 ":homepage \"http://doodles.au\""
+                                 ")\n"))))
       (should (file-exists-p autoloads-file))
       (should-not (get-file-buffer autoloads-file)))))
 
@@ -357,23 +353,12 @@
      (should (search-forward "Version: 1.3" nil t))
      (should (search-forward "Summary: A single-file package with no dependencies"
                              nil t))
+     (should (search-forward "Homepage: http://doodles.au" nil t))
      ;; No description, though. Because at this point we don't know
      ;; what archive the package originated from, and we don't have
      ;; its readme file saved.
      )))
 
-(ert-deftest package-test-describe-not-installed-package ()
-  "Test displaying of the readme for not-installed package."
-
-  (with-package-test ()
-    (package-initialize)
-    (package-refresh-contents)
-    (with-fake-help-buffer
-     (describe-package 'simple-single)
-     (goto-char (point-min))
-     (should (search-forward "This package provides a minor mode to frobnicate"
-                             nil t)))))
-
 (ert-deftest package-test-describe-non-installed-package ()
   "Test displaying of the readme for non-installed package."
 
@@ -383,6 +368,7 @@
     (with-fake-help-buffer
      (describe-package 'simple-single)
      (goto-char (point-min))
+     (should (search-forward "Homepage: http://doodles.au" nil t))
      (should (search-forward "This package provides a minor mode to frobnicate"
                              nil t)))))
 
@@ -395,6 +381,7 @@
     (with-fake-help-buffer
      (describe-package 'multi-file)
      (goto-char (point-min))
+     (should (search-forward "Homepage: http://puddles.li" nil t))
      (should (search-forward "This is a bare-bones readme file for the multi-file"
                              nil t)))))
 

=== modified file 'test/automated/package-x-test.el'
--- test/automated/package-x-test.el	2013-07-09 07:11:50 +0000
+++ test/automated/package-x-test.el	2013-08-07 08:34:21 +0000
@@ -48,14 +48,16 @@
   (cons 'simple-single
         (package-make-ac-desc '(1 3) nil
                               "A single-file package with no dependencies"
-                              'single))
+                              'single
+                              '((:homepage . "http://doodles.au"))))
   "Expected contents of the archive entry from the \"simple-single\" package.")
 
 (defvar package-x-test--single-archive-entry-1-4
   (cons 'simple-single
         (package-make-ac-desc '(1 4) nil
                               "A single-file package with no dependencies"
-                              'single))
+                              'single
+                              nil))
   "Expected contents of the archive entry from the updated \"simple-single\" package.")
 
 (ert-deftest package-x-test-upload-buffer ()

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

[Dmitry Gutov]

Patch installed in revision 114484.

Guess what's left is to add ELPA support.

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

And here's the updated patch for admin/archive-contents.el.

Does the ELPA server use the stable version of Emacs, or the current
trunk? The attached code uses `package-desc-from-define' and
`package--alist-to-plist', requiring a very recent version.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: archive-contents url patch --]
[-- Type: text/x-diff, Size: 8710 bytes --]

diff --git a/admin/archive-contents.el b/admin/archive-contents.el
index 499728e..17a4e17 100644
--- a/admin/archive-contents.el
+++ b/admin/archive-contents.el
@@ -158,11 +158,12 @@ Currently only refreshes the ChangeLog files."
 
 (defun archive--simple-package-p (dir pkg)
   "Test whether DIR contains a simple package named PKG.
-Return a list (SIMPLE VERSION DESCRIPTION REQ), where
+Return a list (SIMPLE VERSION DESCRIPTION REQ EXTRAS), where
 SIMPLE is non-nil if the package is indeed simple;
 VERSION is the version string of the simple package;
 DESCRIPTION is the brief description of the package;
-REQ is a list of requirements.
+REQ is a list of requirements;
+EXTRAS is an alist with additional metadata.
 Otherwise, return nil."
   (let* ((pkg-file (expand-file-name (concat pkg "-pkg.el") dir))
 	 (mainfile (expand-file-name (concat pkg ".el") dir))
@@ -186,15 +187,17 @@ Otherwise, return nil."
                  (requires-str (lm-header "package-requires"))
                  (pt (lm-header "package-type"))
                  (simple (if pt (equal pt "simple") (= (length files) 1)))
+                 (url (or (lm-homepage)
+                          (format "http://elpa.gnu.org/packages/%s.html" pkg)))
                  (req
                   (if requires-str
                       (mapcar 'archive--convert-require
                               (car (read-from-string requires-str))))))
-            (list simple version description req)))))
+            (list simple version description req (list (cons :url url)))))))
      ((not (file-exists-p pkg-file))
       (error "Can find single file nor package desc file in %s" dir)))))
 
-(defun archive--process-simple-package (dir pkg vers desc req)
+(defun archive--process-simple-package (dir pkg vers desc req extras)
   "Deploy the contents of DIR into the archive as a simple package.
 Rename DIR/PKG.el to PKG-VERS.el, delete DIR, and return the descriptor."
   ;; Write DIR/foo.el to foo-VERS.el and delete DIR
@@ -220,7 +223,7 @@ Rename DIR/PKG.el to PKG-VERS.el, delete DIR, and return the descriptor."
       (kill-buffer)))
   (delete-directory dir t)
   (cons (intern pkg) (vector (archive--version-to-list vers)
-                             req desc 'single)))
+                             req desc 'single extras)))
 
 (defun archive--make-changelog (dir srcdir)
   "Export Git log info of DIR into a ChangeLog file."
@@ -251,19 +254,18 @@ Rename DIR/PKG.el to PKG-VERS.el, delete DIR, and return the descriptor."
   "Deploy the contents of DIR into the archive as a multi-file package.
 Rename DIR/ to PKG-VERS/, and return the descriptor."
   (let* ((exp (archive--multi-file-package-def dir pkg))
-	 (vers (nth 2 exp))
-         (req-exp (nth 4 exp))
-	 (req (mapcar 'archive--convert-require
-                      (if (eq 'quote (car-safe req-exp)) (nth 1 req-exp)
-                        (when req-exp
-                          (error "REQ should be a quoted constant: %S"
-                                 req-exp))))))
-    (unless (equal (nth 1 exp) pkg)
+         (pkg-desc (apply #'package-desc-from-define (cdr exp)))
+         (pkg-name (package-desc-name pkg-desc)))
+    (unless (string= pkg-name pkg)
       (error (format "Package name %s doesn't match file name %s"
-		     (nth 1 exp) pkg)))
-    (rename-file dir (concat pkg "-" vers))
-    (cons (intern pkg) (vector (archive--version-to-list vers)
-                               req (nth 3 exp) 'tar))))
+		     pkg-name pkg)))
+    (rename-file dir (concat pkg "-" (package-version-join
+                                      (package-desc-version pkg-desc))))
+    (cons (intern pkg) (vector (package-desc-version pkg-desc)
+                               (package-desc-reqs pkg-desc)
+                               (package-desc-summary pkg-desc)
+                               'tar
+                               (package-desc-extras pkg-desc)))))
 
 (defun archive--multi-file-package-def (dir pkg)
   "Return the `define-package' form in the file DIR/PKG-pkg.el."
@@ -286,7 +288,7 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
       ;; (message "Not refreshing pkg description of %s" pkg)
       )))
 
-(defun archive--write-pkg-file (pkg-dir name version desc requires &rest ignored)
+(defun archive--write-pkg-file (pkg-dir name version desc requires extras)
   (let ((pkg-file (expand-file-name (concat name "-pkg.el") pkg-dir))
 	(print-level nil)
         (print-quoted t)
@@ -295,17 +297,19 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
      (concat (format ";; Generated package description from %s.el\n"
 		     name)
 	     (prin1-to-string
-	      (list 'define-package
-		    name
-		    version
-		    desc
-		    (list 'quote
-			  ;; Turn version lists into string form.
-			  (mapcar
-			   (lambda (elt)
-			     (list (car elt)
-				   (package-version-join (cadr elt))))
-			   requires))))
+              (nconc
+               (list 'define-package
+                     name
+                     version
+                     desc
+                     (list 'quote
+                           ;; Turn version lists into string form.
+                           (mapcar
+                            (lambda (elt)
+                              (list (car elt)
+                                    (package-version-join (cadr elt))))
+                            requires)))
+               (package--alist-to-plist extras)))
 	     "\n")
      nil
      pkg-file)))
@@ -388,30 +392,29 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
   (replace-regexp-in-string "<" "&lt;"
                             (replace-regexp-in-string "&" "&amp;" txt)))
 
-(defun archive--insert-repolinks (name srcdir mainsrcfile)
-  (let ((url (archive--get-prop "URL" name srcdir mainsrcfile)))
-    (if url
-        (insert (format "<p>Origin: <a href=%S>%s</a></p>\n"
-                        url (archive--quote url)))
-      (let* ((externals
-              (with-temp-buffer
-                (insert-file-contents
-                 (expand-file-name "../../../elpa/externals-list" srcdir))
-                (read (current-buffer))))
-             (external (eq :external (nth 1 (assoc name externals))))
-             (git-sv "http://git.savannah.gnu.org/")
-             (urls (if external
-                       '("cgit/emacs/elpa.git/?h=externals/"
-                         "gitweb/?p=emacs/elpa.git;a=shortlog;h=refs/heads/externals/")
-                     '("cgit/emacs/elpa.git/tree/packages/"
-                       "gitweb/?p=emacs/elpa.git;a=tree;f=packages/"))))
-        (insert (format
-                 (concat "<p>Browse repository: <a href=%S>%s</a>"
-                         " or <a href=%S>%s</a></p>\n")
-                 (concat git-sv (nth 0 urls) name)
-                 'CGit
-                 (concat git-sv (nth 1 urls) name)
-                 'Gitweb))))))
+(defun archive--insert-repolinks (name srcdir mainsrcfile url)
+  (if url
+      (insert (format "<p>Origin: <a href=%S>%s</a></p>\n"
+                      url (archive--quote url)))
+    (let* ((externals
+            (with-temp-buffer
+              (insert-file-contents
+               (expand-file-name "../../../elpa/externals-list" srcdir))
+              (read (current-buffer))))
+           (external (eq :external (nth 1 (assoc name externals))))
+           (git-sv "http://git.savannah.gnu.org/")
+           (urls (if external
+                     '("cgit/emacs/elpa.git/?h=externals/"
+                       "gitweb/?p=emacs/elpa.git;a=shortlog;h=refs/heads/externals/")
+                   '("cgit/emacs/elpa.git/tree/packages/"
+                     "gitweb/?p=emacs/elpa.git;a=tree;f=packages/"))))
+      (insert (format
+               (concat "<p>Browse repository: <a href=%S>%s</a>"
+                       " or <a href=%S>%s</a></p>\n")
+               (concat git-sv (nth 0 urls) name)
+               'CGit
+               (concat git-sv (nth 1 urls) name)
+               'Gitweb)))))
 
 (defun archive--html-make-pkg (pkg files)
   (let* ((name (symbol-name (car pkg)))
@@ -431,7 +434,8 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
       (let ((maint (archive--get-prop "Maintainer" name srcdir mainsrcfile)))
         (when maint
           (insert (format "<p>Maintainer: %s</p>\n" (archive--quote maint)))))
-      (archive--insert-repolinks name srcdir mainsrcfile)
+      (archive--insert-repolinks name srcdir mainsrcfile
+                                 (cdr (assoc :url (aref (cdr pkg) 4))))
       (let ((rm (archive--get-section
                  "Commentary" '("README" "README.rst" "README.md" "README.org")
                  srcdir mainsrcfile)))

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

[Stefan Monnier]

> Does the ELPA server use the stable version of Emacs, or the current
> trunk?

It uses the emacs24 package from Debian, IIRC.


        Stefan

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

[Dmitry Gutov]

On 02.10.2013 06:09, Stefan Monnier wrote:
>> Does the ELPA server use the stable version of Emacs, or the current
>> trunk?
>
> It uses the emacs24 package from Debian, IIRC.

That's too bad, I'd rather not reimplement the alist-to-plist and 
plist-to-alist code from package.el. And we could use the new struct 
types more.

There are some snapshot packages here: http://emacs.naquadah.org/
Can we use it?

I understand using auto-updateable snapshot package will be dangerous, 
but if it's only updated manually, it could be fine.

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

[Stefan Monnier]

> That's too bad, I'd rather not reimplement the alist-to-plist and
> plist-to-alist code from package.el.

You can copy&paste it.

> There are some snapshot packages here: http://emacs.naquadah.org/
> Can we use it?

I'd rather not bother.


        Stefan

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

[Dmitry Gutov]

On 03.10.2013 16:46, Stefan Monnier wrote:
>> That's too bad, I'd rather not reimplement the alist-to-plist and
>> plist-to-alist code from package.el.
>
> You can copy&paste it.

Okay, done, thanks for the reality check.

Patch installed, now let's see how it works live.

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

[Stefan Monnier]

> Okay, done, thanks for the reality check.
> Patch installed, now let's see how it works live.

I had to replace lm-homepage since it's not in Emacs-24.3, but other
than that it seems to work.  Thanks,


        Stefan