diff mbox

[2/2] emacs: Add 'guix-package-from-file' command.

Message ID 1462704662-18972-3-git-send-email-alezost@gmail.com
State Committed
Headers show

Commit Message

Alex Kost May 8, 2016, 10:51 a.m. UTC
* emacs/guix-main.scm (register-package, packages-from-file): New procedures.
(%patterns-makers): Add 'from-file' search type.
* emacs/guix-messages.el (guix-messages): Add messages for it.
* emacs/guix-ui-package.el (guix-package-from-file): New command.
(guix-package-info-insert-location): Adjust for 'from-file' type.
* doc/emacs.texi (Emacs Commands): Document it.
---
 doc/emacs.texi           |  5 +++++
 emacs/guix-main.scm      | 47 ++++++++++++++++++++++++++++++++++++-----------
 emacs/guix-messages.el   |  7 +++++++
 emacs/guix-ui-package.el | 33 ++++++++++++++++++++++++---------
 4 files changed, 72 insertions(+), 20 deletions(-)

Comments

Ludovic Courtès May 8, 2016, 6:40 p.m. UTC | #1
Alex Kost <alezost@gmail.com> skribis:

> * emacs/guix-main.scm (register-package, packages-from-file): New procedures.
> (%patterns-makers): Add 'from-file' search type.
> * emacs/guix-messages.el (guix-messages): Add messages for it.
> * emacs/guix-ui-package.el (guix-package-from-file): New command.
> (guix-package-info-insert-location): Adjust for 'from-file' type.
> * doc/emacs.texi (Emacs Commands): Document it.

[...]

> +@item M-x guix-package-from-file
> +Display package that the code within the specified file evaluates to.
> +@xref{Invoking guix package, @code{--install-from-file}}, for an example
> +of how a file may look like.

s/how a file/what such a file/

> +(define-values (package-by-address
> +                register-package)
> +  (let* ((table (delay (fold-packages
> +                        (lambda (package table)
> +                          (vhash-consq (object-address package)
> +                                       package table))
> +                        vlist-null)))
> +         (table* (lambda ()
> +                   (if (promise? table)
> +                       (force table)
> +                       table))))

It may be easier to always make ‘table’ a promise…

> +    (values
> +     (lambda (address)
> +       "Return package by its object ADDRESS."
> +       (match (vhash-assq address (table*))
> +         ((_ . package) package)
> +         (_ #f)))
> +     (lambda (package)
> +       "Register PACKAGE by its 'object-address', so that later
> +'package-by-address' can be used to access it."
> +       (set! table
> +             (vhash-consq (object-address package)
> +                          package (table*)))))))

… by wrapping ‘vhash-consq’ in ‘delay’.

I think this approach is OK.

The rest LGTM!

To avoid ‘set!’ above, the options that come to mind would be:

  1. To not provide M-x guix-package-from-file and instead provide, say,
     M-x guix-install-package-from-file.  That way, we wouldn’t need to
     remember the package.

  2. To thread the state, consisting mainly of lookup tables/procedures,
     through the state monad, and to change the state in this particular
     case.

     Converting to this new style would be quite a bit of work, for just
     this one special case.

Thanks,
Ludo’.
Alex Kost May 9, 2016, 8:25 a.m. UTC | #2
Ludovic Courtès (2016-05-08 21:40 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> * emacs/guix-main.scm (register-package, packages-from-file): New procedures.
>> (%patterns-makers): Add 'from-file' search type.
>> * emacs/guix-messages.el (guix-messages): Add messages for it.
>> * emacs/guix-ui-package.el (guix-package-from-file): New command.
>> (guix-package-info-insert-location): Adjust for 'from-file' type.
>> * doc/emacs.texi (Emacs Commands): Document it.
>
> [...]
>
>> +@item M-x guix-package-from-file
>> +Display package that the code within the specified file evaluates to.
>> +@xref{Invoking guix package, @code{--install-from-file}}, for an example
>> +of how a file may look like.
>
> s/how a file/what such a file/

Fixed.

>> +(define-values (package-by-address
>> +                register-package)
>> +  (let* ((table (delay (fold-packages
>> +                        (lambda (package table)
>> +                          (vhash-consq (object-address package)
>> +                                       package table))
>> +                        vlist-null)))
>> +         (table* (lambda ()
>> +                   (if (promise? table)
>> +                       (force table)
>> +                       table))))
>
> It may be easier to always make ‘table’ a promise…

I don't like this 'table*' procedure (especially its name), and I would
like to get rid of it, but I couldn't make your suggestion work…

>> +    (values
>> +     (lambda (address)
>> +       "Return package by its object ADDRESS."
>> +       (match (vhash-assq address (table*))
>> +         ((_ . package) package)
>> +         (_ #f)))
>> +     (lambda (package)
>> +       "Register PACKAGE by its 'object-address', so that later
>> +'package-by-address' can be used to access it."
>> +       (set! table
>> +             (vhash-consq (object-address package)
>> +                          package (table*)))))))
>
> … by wrapping ‘vhash-consq’ in ‘delay’.

… I tried it, but I got an error I don't know what to do with:

  Throw to key `vm-error' with args `(vm-run "VM: Stack overflow" ())'.

I attach the file that illustrates this problem.  Here is the recipe to
reproduce it using geiser:

1. M-x run-guile
2. Open the attached file
3. Evaluate it: C-c C-b

And the last expression leads to an error (at least for me).  Is there
some problem with combining force/delay or did I do something wrong?

> To avoid ‘set!’ above, the options that come to mind would be:
>
>   1. To not provide M-x guix-package-from-file and instead provide, say,
>      M-x guix-install-package-from-file.  That way, we wouldn’t need to
>      remember the package.

I don't like this solution.  With "M-x guix-package-from-file" you get a
full-featured *Guix Package Info* buffer, where you can not only install
the package, but download its source, look at its license, inputs,
etc. (and soon it will be possible just to build it without installing).

>   2. To thread the state, consisting mainly of lookup tables/procedures,
>      through the state monad, and to change the state in this particular
>      case.
>
>      Converting to this new style would be quite a bit of work, for just
>      this one special case.

Ouch, this looks scary to me, so I stay on the current solution, but
thanks for the pointers!
Ludovic Courtès May 9, 2016, 8:29 p.m. UTC | #3
Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2016-05-08 21:40 +0300) wrote:

[...]

>> … by wrapping ‘vhash-consq’ in ‘delay’.
>
> … I tried it, but I got an error I don't know what to do with:
>
>   Throw to key `vm-error' with args `(vm-run "VM: Stack overflow" ())'.

I needs to be changed like this:

--8<---------------cut here---------------start------------->8---
(define-values (package-by-address
                register-package)
  (let ((table (delay (fold-packages
                       (lambda (package table)
                         (vhash-consq (object-address package)
                                      package table))
                       vlist-null))))
    (values
     (lambda (address)
       "Return package by its object ADDRESS."
       (match (vhash-assq address (force table))
         ((_ . package) package)
         (_ #f)))
     (lambda (package)
       "Register PACKAGE by its 'object-address', so that later
'package-by-address' can be used to access it."
       (let ((table (force table)))
         (set! table
           (delay (vhash-consq (object-address package)
                               package table))))))))
--8<---------------cut here---------------end--------------->8---

In the example you posted, ‘register-package’ would turn ‘table’ into a
self-reference, hence the stack overflow.

>> To avoid ‘set!’ above, the options that come to mind would be:
>>
>>   1. To not provide M-x guix-package-from-file and instead provide, say,
>>      M-x guix-install-package-from-file.  That way, we wouldn’t need to
>>      remember the package.
>
> I don't like this solution.  With "M-x guix-package-from-file" you get a
> full-featured *Guix Package Info* buffer, where you can not only install
> the package, but download its source, look at its license, inputs,
> etc. (and soon it will be possible just to build it without installing).

Yeah, I mentioned it for completeness but I don’t like it either.

Thanks!

Ludo’.
Alex Kost May 10, 2016, 9:09 a.m. UTC | #4
Ludovic Courtès (2016-05-09 23:29 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2016-05-08 21:40 +0300) wrote:
>
> [...]
>
>>> … by wrapping ‘vhash-consq’ in ‘delay’.
>>
>> … I tried it, but I got an error I don't know what to do with:
>>
>>   Throw to key `vm-error' with args `(vm-run "VM: Stack overflow" ())'.
>
> I needs to be changed like this:
>
> (define-values (package-by-address
>                 register-package)
>   (let ((table (delay (fold-packages
>                        (lambda (package table)
>                          (vhash-consq (object-address package)
>                                       package table))
>                        vlist-null))))
>     (values
>      (lambda (address)
>        "Return package by its object ADDRESS."
>        (match (vhash-assq address (force table))
>          ((_ . package) package)
>          (_ #f)))
>      (lambda (package)
>        "Register PACKAGE by its 'object-address', so that later
> 'package-by-address' can be used to access it."
>        (let ((table (force table)))
>          (set! table
>            (delay (vhash-consq (object-address package)
>                                package table))))))))
>
> In the example you posted, ‘register-package’ would turn ‘table’ into a
> self-reference, hence the stack overflow.

Heh, so simple, thanks a lot!  I used this variant and committed this
patch, thanks again.
diff mbox

Patch

diff --git a/doc/emacs.texi b/doc/emacs.texi
index 575e87c..1962ffc 100644
--- a/doc/emacs.texi
+++ b/doc/emacs.texi
@@ -166,6 +166,11 @@  Display package(s) located in the specified file.  These files usually
 have the following form: @file{gnu/packages/emacs.scm}, but don't type
 them manually!  Press @key{TAB} to complete the file name.
 
+@item M-x guix-package-from-file
+Display package that the code within the specified file evaluates to.
+@xref{Invoking guix package, @code{--install-from-file}}, for an example
+of how a file may look like.
+
 @item M-x guix-search-by-regexp
 Search for packages by a specified regexp.  By default ``name'',
 ``synopsis'' and ``description'' of the packages will be searched.  This
diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 5d7df2a..5068ba1 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -300,17 +300,29 @@  Example:
 
 ;;; Finding packages.
 
-(define package-by-address
-  (let ((table (delay (fold-packages
-                       (lambda (package table)
-                         (vhash-consq (object-address package)
-                                      package table))
-                       vlist-null))))
-    (lambda (address)
-      "Return package by its object ADDRESS."
-      (match (vhash-assq address (force table))
-        ((_ . package) package)
-        (_ #f)))))
+(define-values (package-by-address
+                register-package)
+  (let* ((table (delay (fold-packages
+                        (lambda (package table)
+                          (vhash-consq (object-address package)
+                                       package table))
+                        vlist-null)))
+         (table* (lambda ()
+                   (if (promise? table)
+                       (force table)
+                       table))))
+    (values
+     (lambda (address)
+       "Return package by its object ADDRESS."
+       (match (vhash-assq address (table*))
+         ((_ . package) package)
+         (_ #f)))
+     (lambda (package)
+       "Register PACKAGE by its 'object-address', so that later
+'package-by-address' can be used to access it."
+       (set! table
+             (vhash-consq (object-address package)
+                          package (table*)))))))
 
 (define packages-by-name+version
   (let ((table (delay (fold-packages
@@ -410,6 +422,15 @@  MATCH-PARAMS is a list of parameters that REGEXP can match."
               '()
               (find-newest-available-packages)))
 
+(define (packages-from-file file)
+  "Return a list of packages from FILE."
+  (let ((package (load (canonicalize-path file))))
+    (if (package? package)
+        (begin
+          (register-package package)
+          (list package))
+        '())))
+
 
 ;;; Making package/output patterns.
 
@@ -662,6 +683,8 @@  ENTRIES is a list of installed manifest entries."
                                    (lookup-license license-name))))
          (location-proc         (lambda (_ location)
                                   (packages-by-location-file location)))
+         (file-proc             (lambda (_ file)
+                                  (packages-from-file file)))
          (all-proc              (lambda _ (all-available-packages)))
          (newest-proc           (lambda _ (newest-available-packages))))
     `((package
@@ -672,6 +695,7 @@  ENTRIES is a list of installed manifest entries."
        (regexp           . ,regexp-proc)
        (license          . ,license-proc)
        (location         . ,location-proc)
+       (from-file        . ,file-proc)
        (all-available    . ,all-proc)
        (newest-available . ,newest-proc))
       (output
@@ -682,6 +706,7 @@  ENTRIES is a list of installed manifest entries."
        (regexp           . ,regexp-proc)
        (license          . ,license-proc)
        (location         . ,location-proc)
+       (from-file        . ,file-proc)
        (all-available    . ,all-proc)
        (newest-available . ,newest-proc)))))
 
diff --git a/emacs/guix-messages.el b/emacs/guix-messages.el
index 7ebe7e8..52436af 100644
--- a/emacs/guix-messages.el
+++ b/emacs/guix-messages.el
@@ -44,6 +44,9 @@ 
       ,(lambda (_ entries locations)
          (apply #'guix-message-packages-by-location
                 entries 'package locations)))
+     (from-file
+      (0 "No package in file '%s'." val)
+      (1 "Package from file '%s'." val))
      (regexp
       (0 "No packages matching '%s'." val)
       (1 "A single package matching '%s'." val)
@@ -80,6 +83,10 @@ 
       ,(lambda (_ entries locations)
          (apply #'guix-message-packages-by-location
                 entries 'output locations)))
+     (from-file
+      (0 "No package in file '%s'." val)
+      (1 "Package from file '%s'." val)
+      (many "Package outputs from file '%s'." val))
      (regexp
       (0 "No package outputs matching '%s'." val)
       (1 "A single package output matching '%s'." val)
diff --git a/emacs/guix-ui-package.el b/emacs/guix-ui-package.el
index 38f0c08..edc3648 100644
--- a/emacs/guix-ui-package.el
+++ b/emacs/guix-ui-package.el
@@ -393,15 +393,17 @@  formatted with this string, an action button is inserted.")
       (guix-format-insert nil)
     (let ((location-file (car (split-string location ":"))))
       (guix-info-insert-value-indent location 'guix-package-location)
-      (guix-info-insert-indent)
-      (guix-info-insert-action-button
-       "Packages"
-       (lambda (btn)
-         (guix-package-get-display (guix-ui-current-profile)
-                                   'location
-                                   (button-get btn 'location)))
-       (format "Display packages from location '%s'" location-file)
-       'location location-file))))
+      ;; Do not show "Packages" button if a package 'from file' is displayed.
+      (unless (eq (guix-ui-current-search-type) 'from-file)
+        (guix-info-insert-indent)
+        (guix-info-insert-action-button
+         "Packages"
+         (lambda (btn)
+           (guix-package-get-display (guix-ui-current-profile)
+                                     'location
+                                     (button-get btn 'location)))
+         (format "Display packages from location '%s'" location-file)
+         'location location-file)))))
 
 (defun guix-package-info-insert-systems (systems entry)
   "Insert supported package SYSTEMS at point."
@@ -1001,6 +1003,19 @@  Interactively with prefix, prompt for PROFILE."
   (guix-package-get-display profile 'location location))
 
 ;;;###autoload
+(defun guix-package-from-file (file &optional profile)
+  "Display Guix package that the code from FILE evaluates to.
+If PROFILE is nil, use `guix-current-profile'.
+Interactively with prefix, prompt for PROFILE."
+  (interactive
+   (list (read-file-name "File with package: ")
+         (guix-ui-read-profile)))
+  (guix-buffer-get-display-entries
+   'info 'package
+   (list (or profile guix-current-profile) 'from-file file)
+   'add))
+
+;;;###autoload
 (defun guix-search-by-regexp (regexp &optional params profile)
   "Search for Guix packages by REGEXP.
 PARAMS are package parameters that should be searched.