[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#49671] [PATCH] guix: records: Improve error reporting.
From: |
Sarah Morgensen |
Subject: |
[bug#49671] [PATCH] guix: records: Improve error reporting. |
Date: |
Wed, 21 Jul 2021 12:21:23 -0700 |
Hi Julien,
Julien Lepiller <julien@lepiller.eu> writes:
> Hi Guix!
>
> This patch improves error reporting a bit when making mistakes in guix
> records. This is motivated by a user getting "invalid field specifier"
> for their whole services field in their os record. With this patches,
> they would have seen:
After applying your patch, I get:
--8<---------------cut here---------------start------------->8---
guix/records.scm:108:19: warning: "multiple values in field specifier. Got ~a
values associated with key ~a. Values are:~%~{~a~%~}": unsupported format
option ~{, use (ice-9 format) instead
--8<---------------cut here---------------end--------------->8---
After adding `(ice-9 format)` to imports it works as expected. I see
this also applies to package records! This will be great for those
starting to package in Guix.
>
> multiple values in field specifier. Got 2 values associated with key
> services. Values are:
> (append (list (service ...) ...))
> (modify-services %desktop-services ...)
>
> Which would have hinted them at fixing the parenthesis. Or at least, it
> would have saved us some time trying to count them :)
>
> Here are the cases that are handled and the associated message:
>
> (operating-system
> services)
> guix system: error: services: invalid field specifier. The format of a
> field is `(services value)'
>
> (operating-system
> (services))
> test.scm:2:2: error: (services): Value missing in field specifier. The
> format of a field is `(services value)'.
>
> (operating-system
> (services 1 2 3))
> test.scm:2:2: error: (services 1 2 3): multiple values in field
> specifier. Got 3 values associated with key services. Values are:
^ Wrap in `'?
> 1
> 2
> 3
>
> (operating-system
> ())
> guix system: error: (): invalid field specifier. The format of a field
> is `(field value)'
>
> (operating-system
> ((services %desktop-services)))
> test.scm:2:2: error: ((services %desktop-services)): invalid field
> specifier. (services %desktop-services) is not a valid field name.
^ Should this also be wrapped in `'?
Why do some of these messages lose their context and come from `guix
system` instead?
>
> Of course, we can improve these error messages, and internationalize
> them.
>
> WDYT?
[...]
> - (apply syntax-violation name "invalid field specifier"
> - (if parent-form
> + (syntax-case #'weird ()
> + (() ;the empty list
> + (apply syntax-violation name
> + "invalid field specifier. The format of a field is
> `(field value)'"
> (list parent-form #'weird)
> - (list #'weird)))))))
> + (list #'weird)))
Why the extra `(list #'weird')`? AFAICT right now this is providing
`(list parent-form #:'weird)` as the parent form. And since parent-form
is optional, shouldn't this be
--8<---------------cut here---------------start------------->8---
(apply syntax-violation name
"invalid field specifier. The format of a field is `(field value)'"
(if parent-form (list parent-form #:'weird) (list weird)))
--8<---------------cut here---------------end--------------->8---
(and similar for the others)?
--
Sarah