bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Ludovic Courtès]

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

Currently M-x guix-edit fails badly (actually ‘guix-package-location’)
pwhen passed the name of a nonexistent package:

--8<---------------cut here---------------start------------->8---
Debugger entered--Lisp error: (error "Error in evaluating guile expression: ERROR: In procedure car:
ERROR: In procedure car: Wrong type (expecting pair): ()

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [3]> ")
  signal(error ("Error in evaluating guile expression: ERROR: In procedure car:\nERROR: In procedure car: Wrong type (expecting pair): ()\n\nEntering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.\nscheme@(guile-user) [3]> "))
  error("Error in evaluating guile expression: %s" "ERROR: In procedure car:\nERROR: In procedure car: Wrong type (expecting pair): ()\n\nEntering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.\nscheme@(guile-user) [3]> ")
  (if (geiser-eval--retort-error res) (error "Error in evaluating guile expression: %s" (geiser-eval--retort-output res)) (cdr (assq (quote result) res)))
  (let ((res (geiser-eval--send/wait (list (quote :eval) (list (quote :scm) str))))) (if (geiser-eval--retort-error res) (error "Error in evaluating guile expression: %s" (geiser-eval--retort-output res)) (cdr (assq (quote result) res))))
  (save-current-buffer (set-buffer (or repl (guix-geiser-repl))) (let ((res (geiser-eval--send/wait (list (quote :eval) (list (quote :scm) str))))) (if (geiser-eval--retort-error res) (error "Error in evaluating guile expression: %s" (geiser-eval--retort-output res)) (cdr (assq (quote result) res)))))
  guix-geiser-eval("(package-location-string \"foo\")" #<buffer *Guix Internal REPL*>)
  (car (guix-geiser-eval str repl))
  (replace-regexp-in-string "#t" "t" (car (guix-geiser-eval str repl)))
  (replace-regexp-in-string "#f\\|#<unspecified>" "nil" (replace-regexp-in-string "#t" "t" (car (guix-geiser-eval str repl))))
  (read (replace-regexp-in-string "#f\\|#<unspecified>" "nil" (replace-regexp-in-string "#t" "t" (car (guix-geiser-eval str repl)))))
  guix-geiser-eval-read("(package-location-string \"foo\")" #<buffer *Guix Internal REPL*>)
  guix-eval-read("(package-location-string \"foo\")")
  guix-package-location("foo")
  eval((guix-package-location "foo") nil)
  eval-expression((guix-package-location "foo") nil)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)
--8<---------------cut here---------------end--------------->8---

I think this patch fixes it:


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

diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 34da6ac..c5d5d75 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -954,10 +954,14 @@ GENERATIONS is a list of generation numbers."
 
 (define (package-location-string id-or-name)
   "Return a location string of a package with ID-OR-NAME."
-  (and-let* ((package  (or (package-by-id id-or-name)
-                           (first (packages-by-name id-or-name))))
-             (location (package-location package)))
-    (location->string location)))
+  (define package
+    (or (package-by-id id-or-name)
+        (match (packages-by-name id-or-name)
+          (() #f)
+          ((first . rest) first))))
+
+  (and package
+       (location->string (package-location package))))
 
 (define (package-source-derivation->store-path derivation)
   "Return a store path of the package source DERIVATION."

[-- Attachment #3: Type: text/plain, Size: 25 bytes --]


Thoughts?

Ludo’.

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Mathieu Lirzin]

ludo@gnu.org (Ludovic Courtès) writes:

> diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
> index 34da6ac..c5d5d75 100644
> --- a/emacs/guix-main.scm
> +++ b/emacs/guix-main.scm
> @@ -954,10 +954,14 @@ GENERATIONS is a list of generation numbers."
>  
>  (define (package-location-string id-or-name)
>    "Return a location string of a package with ID-OR-NAME."
> -  (and-let* ((package  (or (package-by-id id-or-name)
> -                           (first (packages-by-name id-or-name))))
> -             (location (package-location package)))
> -    (location->string location)))
> +  (define package
> +    (or (package-by-id id-or-name)
> +        (match (packages-by-name id-or-name)
> +          (() #f)
> +          ((first . rest) first))))
> +
> +  (and package
> +       (location->string (package-location package))))

Not related to the bug.  but it feels weird to use internal defines for
something else than a procedure.

what about using (not tested):

--8<---------------cut here---------------start------------->8---
  (and=> (or (package-by-id id-or-name)
             (match (packages-by-name id-or-name)
               (()        #f)
               ((pkg ..1) pkg)))
         (compose location->string package-location))
--8<---------------cut here---------------end--------------->8---

I know you love my 'pkg' identifier.  ;)

-- 
Mathieu Lirzin

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Ludovic Courtès]

Mathieu Lirzin <mthl@gnu.org> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
>> index 34da6ac..c5d5d75 100644
>> --- a/emacs/guix-main.scm
>> +++ b/emacs/guix-main.scm
>> @@ -954,10 +954,14 @@ GENERATIONS is a list of generation numbers."
>>  
>>  (define (package-location-string id-or-name)
>>    "Return a location string of a package with ID-OR-NAME."
>> -  (and-let* ((package  (or (package-by-id id-or-name)
>> -                           (first (packages-by-name id-or-name))))
>> -             (location (package-location package)))
>> -    (location->string location)))
>> +  (define package
>> +    (or (package-by-id id-or-name)
>> +        (match (packages-by-name id-or-name)
>> +          (() #f)
>> +          ((first . rest) first))))
>> +
>> +  (and package
>> +       (location->string (package-location package))))
>
> Not related to the bug.  but it feels weird to use internal defines for
> something else than a procedure.

I don’t find it weird.  :-)

> what about using (not tested):
>
>   (and=> (or (package-by-id id-or-name)
>              (match (packages-by-name id-or-name)
>                (()        #f)
>                ((pkg ..1) pkg)))
>          (compose location->string package-location))

I like it too!

> I know you love my 'pkg' identifier.  ;)

No I don’t!  :-)

I think we should stick to one identifier style, which is to always use
full words (I’m often tempted to use abbreviations and I force myself
not to, as silly as I am!)

Ludo’.

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Alex Kost]

Ludovic Courtès (2016-03-07 19:28 +0300) wrote:

> Currently M-x guix-edit fails badly (actually ‘guix-package-location’)
> pwhen passed the name of a nonexistent package:
[...]
> I think this patch fixes it:
>
>
> diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
> index 34da6ac..c5d5d75 100644
> --- a/emacs/guix-main.scm
> +++ b/emacs/guix-main.scm
> @@ -954,10 +954,14 @@ GENERATIONS is a list of generation numbers."
>  
>  (define (package-location-string id-or-name)
>    "Return a location string of a package with ID-OR-NAME."
> -  (and-let* ((package  (or (package-by-id id-or-name)
> -                           (first (packages-by-name id-or-name))))
> -             (location (package-location package)))
> -    (location->string location)))
> +  (define package
> +    (or (package-by-id id-or-name)
> +        (match (packages-by-name id-or-name)
> +          (() #f)
> +          ((first . rest) first))))
> +
> +  (and package
> +       (location->string (package-location package))))
>  
>  (define (package-source-derivation->store-path derivation)
>    "Return a store path of the package source DERIVATION."
>
> Thoughts?

Great!  Thanks for fixing it!  Feel free (you or Mathieu) to push any of
your patches.

-- 
Alex

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Alex Kost]

Mathieu Lirzin (2016-03-07 22:58 +0300) wrote:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
>> index 34da6ac..c5d5d75 100644
>> --- a/emacs/guix-main.scm
>> +++ b/emacs/guix-main.scm
>> @@ -954,10 +954,14 @@ GENERATIONS is a list of generation numbers."
>>  
>>  (define (package-location-string id-or-name)
>>    "Return a location string of a package with ID-OR-NAME."
>> -  (and-let* ((package  (or (package-by-id id-or-name)
>> -                           (first (packages-by-name id-or-name))))
>> -             (location (package-location package)))
>> -    (location->string location)))
>> +  (define package
>> +    (or (package-by-id id-or-name)
>> +        (match (packages-by-name id-or-name)
>> +          (() #f)
>> +          ((first . rest) first))))
>> +
>> +  (and package
>> +       (location->string (package-location package))))
>
> Not related to the bug.  but it feels weird to use internal defines for
> something else than a procedure.

I have the same feeling.

> what about using (not tested):
>
>
>   (and=> (or (package-by-id id-or-name)
>              (match (packages-by-name id-or-name)
>                (()        #f)
>                ((pkg ..1) pkg)))
>          (compose location->string package-location))

I like this variant!

-- 
Alex

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Alex Kost]

Ludovic Courtès (2016-03-08 00:03 +0300) wrote:

[...]
>> what about using (not tested):
>>
>>   (and=> (or (package-by-id id-or-name)
>>              (match (packages-by-name id-or-name)
>>                (()        #f)
>>                ((pkg ..1) pkg)))
>>          (compose location->string package-location))
>
> I like it too!
>
>> I know you love my 'pkg' identifier.  ;)
>
> No I don’t!  :-)
>
> I think we should stick to one identifier style, which is to always use
> full words (I’m often tempted to use abbreviations and I force myself
> not to, as silly as I am!)

Good to know, I always thought that things like "pkg" or "drv" are OK in
a local scope.

-- 
Alex

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2016-03-08 00:03 +0300) wrote:
>
> [...]
>>> what about using (not tested):
>>>
>>>   (and=> (or (package-by-id id-or-name)
>>>              (match (packages-by-name id-or-name)
>>>                (()        #f)
>>>                ((pkg ..1) pkg)))
>>>          (compose location->string package-location))
>>
>> I like it too!
>>
>>> I know you love my 'pkg' identifier.  ;)
>>
>> No I don’t!  :-)
>>
>> I think we should stick to one identifier style, which is to always use
>> full words (I’m often tempted to use abbreviations and I force myself
>> not to, as silly as I am!)
>
> Good to know, I always thought that things like "pkg" or "drv" are OK in
> a local scope.

Well, ahem, ‘drv’ is an exception…

Otherwise I think the rationale of the “Naming” section at
<http://mumble.net/~campbell/scheme/style.txt> is a good one.

Ludo’.

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Ludovic Courtès]

Mathieu Lirzin <mthl@gnu.org> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
>> index 34da6ac..c5d5d75 100644
>> --- a/emacs/guix-main.scm
>> +++ b/emacs/guix-main.scm
>> @@ -954,10 +954,14 @@ GENERATIONS is a list of generation numbers."
>>  
>>  (define (package-location-string id-or-name)
>>    "Return a location string of a package with ID-OR-NAME."
>> -  (and-let* ((package  (or (package-by-id id-or-name)
>> -                           (first (packages-by-name id-or-name))))
>> -             (location (package-location package)))
>> -    (location->string location)))
>> +  (define package
>> +    (or (package-by-id id-or-name)
>> +        (match (packages-by-name id-or-name)
>> +          (() #f)
>> +          ((first . rest) first))))
>> +
>> +  (and package
>> +       (location->string (package-location package))))
>
> Not related to the bug.  but it feels weird to use internal defines for
> something else than a procedure.
>
> what about using (not tested):
>
>   (and=> (or (package-by-id id-or-name)
>              (match (packages-by-name id-or-name)
>                (()        #f)
>                ((pkg ..1) pkg)))
>          (compose location->string package-location))
>
> I know you love my 'pkg' identifier.  ;)

Fixed along these lines in commit
16f4acbddbb38275a52554caf693017465586ac6.

(Note that ..1 matches a list of one or more element, not the first
element of a list.)

Ludo’.

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Mathieu Lirzin]

ludo@gnu.org (Ludovic Courtès) writes:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2016-03-08 00:03 +0300) wrote:
>>>
>>> I think we should stick to one identifier style, which is to always use
>>> full words (I’m often tempted to use abbreviations and I force myself
>>> not to, as silly as I am!)
>>
>> Good to know, I always thought that things like "pkg" or "drv" are OK in
>> a local scope.
>
> Well, ahem, ‘drv’ is an exception…
>
> Otherwise I think the rationale of the “Naming” section at
> <http://mumble.net/~campbell/scheme/style.txt> is a good one.

I totally agree with the suggestions made in this section.  However
these conventions don't talk about "bound variables" which is IMO a
different context than the global name space.

I used to think that full words everywhere were a good thing, by
opposition of the unhelpful variables 'i' and 'x'.  Nonetheless, with
the experience (short I admit) I tend to think that an abbreviation is
still helpful for bound variables because it helps distinguishing them
from free variables.  It is even more true when the meaning of this
abbrevation is made explicit by the doc-string.

-- 
Mathieu Lirzin

bug#22933: M-x guix-edit fails gracelessly when passed an nonexistent package name

[Ludovic Courtès]

Mathieu Lirzin <mthl@gnu.org> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Alex Kost <alezost@gmail.com> skribis:
>>
>>> Ludovic Courtès (2016-03-08 00:03 +0300) wrote:
>>>>
>>>> I think we should stick to one identifier style, which is to always use
>>>> full words (I’m often tempted to use abbreviations and I force myself
>>>> not to, as silly as I am!)
>>>
>>> Good to know, I always thought that things like "pkg" or "drv" are OK in
>>> a local scope.
>>
>> Well, ahem, ‘drv’ is an exception…
>>
>> Otherwise I think the rationale of the “Naming” section at
>> <http://mumble.net/~campbell/scheme/style.txt> is a good one.
>
> I totally agree with the suggestions made in this section.  However
> these conventions don't talk about "bound variables" which is IMO a
> different context than the global name space.
>
> I used to think that full words everywhere were a good thing, by
> opposition of the unhelpful variables 'i' and 'x'.  Nonetheless, with
> the experience (short I admit) I tend to think that an abbreviation is
> still helpful for bound variables because it helps distinguishing them
> from free variables.  It is even more true when the meaning of this
> abbrevation is made explicit by the doc-string.

Yeah I was thinking along these lines at some point (as is apparent in
older parts of Guix ;-)), but my preference du jour is different.
Anyway, we’ll see next time such an issue comes up!

Ludo’.