unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#49671] [PATCH] guix: records: Improve error reporting.
@ 2021-07-20 23:40 Julien Lepiller
  2021-07-21 19:21 ` Sarah Morgensen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Julien Lepiller @ 2021-07-20 23:40 UTC (permalink / raw)
  To: 49671

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

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:

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


Of course, we can improve these error messages, and internationalize
them.

WDYT?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-guix-records-Improve-error-reporting.patch --]
[-- Type: text/x-patch, Size: 3335 bytes --]

From 4cde5555682ca6a3c78cb274cc308087e51ea0e9 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Wed, 21 Jul 2021 01:30:04 +0200
Subject: [PATCH] guix: records: Improve error reporting.

* guix/records.scm (report-invalid-field-specifier): Handle various
invalidity causes separately.
---
 guix/records.scm | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/guix/records.scm b/guix/records.scm
index 3d54a51956..3c3dd478a5 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2021 Julien Lepiller <julien@lepiller.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -83,10 +84,40 @@ error-reporting purposes."
          ;; WEIRD may be an identifier, thus lacking source location info, and
          ;; BINDINGS is a list, also lacking source location info.  Hopefully
          ;; PARENT-FORM provides source location info.
-         (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)))
+           (((field ...) _ ...)            ;a list whose first element is a list
+            (apply syntax-violation name
+                   (format #f "invalid field specifier. ~a is not a valid field name."
+                           (map syntax->datum #'(field ...)))
+                   (list parent-form #'weird)
+                   (list #'weird)))
+           ((field)                        ;a list with one element
+            (apply syntax-violation name
+                   (format #f "Value missing in field specifier. The format of \
+a field is `(~a value)'."
+                           (syntax->datum #'field))
+                   (list parent-form #'weird)
+                   (list #'weird)))
+           ((field value ...)              ;any other list
+            (apply syntax-violation name
+                   (format #f "multiple values in field specifier. Got ~a values \
+associated with key ~a. Values are:~%~{~a~%~}"
+                           (length #'(value ...)) (syntax->datum #'field)
+                           (map syntax->datum #'(value ...)))
+                    (list parent-form #'weird)
+                    (list #'weird)))
+           (field                          ;not a list
+             (apply syntax-violation name
+                    (format #f "invalid field specifier. The format of a field \
+is `(~a value)'"
+                            (syntax->datum #'field))
+                    (list parent-form #'weird)
+                    (list #'weird))))))))
 
   (define (report-duplicate-field-specifier name ctor)
     "Report the first duplicate identifier among the bindings in CTOR."
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [bug#49671] [PATCH] guix: records: Improve error reporting.
  2021-07-20 23:40 [bug#49671] [PATCH] guix: records: Improve error reporting Julien Lepiller
@ 2021-07-21 19:21 ` Sarah Morgensen
  2021-08-04 15:19 ` Ludovic Courtès
  2021-10-31  2:06 ` Julien Lepiller
  2 siblings, 0 replies; 6+ messages in thread
From: Sarah Morgensen @ 2021-07-21 19:21 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: 49671

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#49671] [PATCH] guix: records: Improve error reporting.
  2021-07-20 23:40 [bug#49671] [PATCH] guix: records: Improve error reporting Julien Lepiller
  2021-07-21 19:21 ` Sarah Morgensen
@ 2021-08-04 15:19 ` Ludovic Courtès
  2021-10-31  2:06 ` Julien Lepiller
  2 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2021-08-04 15:19 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: 49671

Hello!

Julien Lepiller <julien@lepiller.eu> skribis:

> 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:
> 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.
>
>
> Of course, we can improve these error messages, and internationalize
> them.

I like the idea!

I would prefer for error messages to be plain errors, without hints, so:

  test.scm:2:2: error: (services 1 2 3): multiple values in field specifier

(Not a sentence, no period.)

But I’d also like to have hints, and ideally all hints should be
reported consistently, via ‘&fix-hint’ or similar.

Thankfully, we can do that via (guix diagnostics) + SRFI-35/34:

  (raise (condition
           (&syntax (form 'x) (subform 'y))
           (&fix-hint (hint "Consider fixing this."))))

‘call-with-error-handling’ in (guix ui) might need to be adjusted.

The only downside is that (guix records) would now depend on another
Guix modules, which I tried to avoid so far so it could be more easily
reused elsewhere.  But that’s the price to pay to get consistent error
reports, and I think it’s okay.

Note that ‘tests/guix-system.sh’ and a couple of other files test exact
error reports, so we’ll have to make sure it still works and possibly
augment the tests.

WDYT?

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#49671] [PATCH] guix: records: Improve error reporting.
  2021-07-20 23:40 [bug#49671] [PATCH] guix: records: Improve error reporting Julien Lepiller
  2021-07-21 19:21 ` Sarah Morgensen
  2021-08-04 15:19 ` Ludovic Courtès
@ 2021-10-31  2:06 ` Julien Lepiller
  2021-11-22  2:40   ` [bug#49671] [PATCH v3] " Julien Lepiller
  2 siblings, 1 reply; 6+ messages in thread
From: Julien Lepiller @ 2021-10-31  2:06 UTC (permalink / raw)
  To: 49671

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

Hi Guix!

Here is a new patch that attempts to better catch various causes of
syntax errors in records. I think I fixed all the concerns Ludo had,
and I draw some inspiration from https://issues.guix.gnu.org/50914 for
using conditions.

Here are examples of what you get:

(operating-system
  ())

test.scm:1:0: error: invalid field specifier: ()
hint: The format of a field is `(field value)'



(operating-system
  ((services %base-services)))

test.scm:1:0: error: invalid field specifier: (services %base-services)
is not a valid field name



(operating-system
  (services))

test.scm:1:0: error: missing value in field specifier
hint: The field is missing a value: `(services <value>)'.



(operating-system
  (services (service tor-service-type) %base-services))

test.scm:1:0: error: multiple values in field specifier
hint: 2 values were associated with field `services'.  We got the
following values:

     (service tor-service-type)
     %base-services
     

Perhaps the additional values were intended to be other field
specifiers.  This usually indicates missing or misplaced parenthesis.



(operating-system
  services %base-services)

test.scm:1:0: error: invalid field specifier: #<syntax services>
hint: The format of a field is `(field value)'



(not sure why the last one is wrapped in syntax...)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-guix-records-Improve-error-reporting.patch --]
[-- Type: text/x-patch, Size: 5059 bytes --]

From 9e08a887a08da3f0cc132d148b748eb2ce7db135 Mon Sep 17 00:00:00 2001
Message-Id: <9e08a887a08da3f0cc132d148b748eb2ce7db135.1635645523.git.julien@lepiller.eu>
From: Julien Lepiller <julien@lepiller.eu>
Date: Sun, 31 Oct 2021 02:58:14 +0100
Subject: [PATCH] guix: records: Improve error reporting.

* guix/records.scm (report-invalid-field-specifier): Handle various
invalidity causes separately.
---
 guix/records.scm | 56 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/guix/records.scm b/guix/records.scm
index ed94c83dac..9baa2c2f1d 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2021 Julien Lepiller <julien@lepiller.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,9 +22,13 @@ (define-module (guix records)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-35)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 rdelim)
+  #:use-module (guix diagnostics)
+  #:use-module (guix ui)
   #:autoload (system base target) (target-most-positive-fixnum)
   #:export (define-record-type*
             this-record
@@ -83,10 +88,53 @@ (define-syntax record-error
          ;; WEIRD may be an identifier, thus lacking source location info, and
          ;; BINDINGS is a list, also lacking source location info.  Hopefully
          ;; PARENT-FORM provides source location info.
-         (apply syntax-violation name "invalid field specifier"
-                (if parent-form
-                    (list parent-form #'weird)
-                    (list #'weird)))))))
+         (let* ((weird-properties (source-properties #'weird))
+                (parent-properties (and parent-form (syntax-source parent-form)))
+                (location (source-properties->location
+                            (or (and (not (null? weird-properties)) weird-properties)
+                                (and (not (null? parent-properties)) parent-properties)
+                                '()))))
+           (syntax-case #'weird ()
+             (()                             ;the empty list
+              (raise-exception
+                (condition
+                  (&message (message (format #f (G_ "invalid field specifier: ~a")
+                                             #'weird)))
+                  (&error-location (location location))
+                  (&fix-hint (hint (G_ "The format of a field is `(field value)'"))))))
+             (((field ...) _ ...)            ;a list whose first element is a list
+              (raise-exception
+                (condition
+                  (&message (message (format #f (G_ "invalid field specifier: ~a \
+is not a valid field name")
+                                             (map syntax->datum #'(field ...)))))
+                  (&error-location (location location)))))
+             ((field)                        ;a list with one element
+              (raise-exception
+                (condition
+                  (&message (message (G_ "missing value in field specifier")))
+                  (&error-location (location location))
+                  (&fix-hint (hint (format #f (G_ "The field is missing a value: `(~a <value>)'.")
+                                           (syntax->datum #'field)))))))
+             ((field value ...)              ;any other list
+              (raise-exception
+                (condition
+                  (&message (message (G_ "multiple values in field specifier")))
+                  (&error-location (location location))
+                  (&fix-hint (hint (format #f (G_ "~a values were associated with \
+field `~a'.  We got the following values:~%@lisp~%~{~a~%~}~%@end lisp~%Perhaps the additional values
+were intended to be other field specifiers.  This usually indicates missing or \
+misplaced parenthesis.")
+                                           (length #'(value ...))
+                                           (syntax->datum #'field)
+                                           (map syntax->datum #'(value ...))))))))
+             (field                          ;not a list
+              (raise-exception
+                (condition
+                  (&message (message (format #f (G_ "invalid field specifier: ~a")
+                                             #'weird)))
+                  (&error-location (location location))
+                  (&fix-hint (hint (G_ "The format of a field is `(field value)'"))))))))))))
 
   (define (report-duplicate-field-specifier name ctor)
     "Report the first duplicate identifier among the bindings in CTOR."
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [bug#49671] [PATCH v3] guix: records: Improve error reporting.
  2021-10-31  2:06 ` Julien Lepiller
@ 2021-11-22  2:40   ` Julien Lepiller
  2022-07-17  6:23     ` Julien Lepiller
  0 siblings, 1 reply; 6+ messages in thread
From: Julien Lepiller @ 2021-11-22  2:40 UTC (permalink / raw)
  To: 49671

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

Here is another improvement compared to v2. This time there are two
patches: the first adds support for &syntax in (guix ui), and will
print something like

in form: <pretty-printed-form>

where "in form" is in green.

The second patch is very similar to v2, but will now also raise a
&syntax condition, so it can be pretty-printed. The previous issue
where I printed #<syntax ...> is fixed, I simply forgot a syntax->datum.

WDYT?

[-- Attachment #2: 0001-guix-ui-Print-syntax-errors.patch --]
[-- Type: text/x-patch, Size: 3252 bytes --]

From d1d857ea0ca76f1a917382777f57871e50026df3 Mon Sep 17 00:00:00 2001
Message-Id: <d1d857ea0ca76f1a917382777f57871e50026df3.1637548566.git.julien@lepiller.eu>
From: Julien Lepiller <julien@lepiller.eu>
Date: Mon, 22 Nov 2021 02:54:06 +0100
Subject: [PATCH 1/2] guix: ui: Print syntax errors.

* guix/ui.scm (display-syntax): New procedure.
(call-with-error-handling, report-load-error): Print syntax errors.
---
 guix/ui.scm | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/guix/ui.scm b/guix/ui.scm
index bd999103ff..79aab2db84 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -69,9 +69,11 @@ (define-module (guix ui)
   #:use-module (srfi srfi-31)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (ice-9 exceptions)
   #:autoload   (ice-9 ftw)  (scandir)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 pretty-print)
   #:use-module (ice-9 regex)
   #:autoload   (ice-9 popen) (open-pipe* close-pipe)
   #:autoload   (system repl repl)  (start-repl)
@@ -315,6 +317,19 @@ (define* (display-hint message #:optional (port (current-error-port)))
      (texi->plain-text message))
    port))
 
+(define %syntax-color (color BOLD GREEN))
+
+(define* (display-syntax form #:optional (port (current-error-port)))
+  "Display FORM, an sexp, to PORT"
+  (define colorize
+    (if (color-output? port)
+        (lambda (str)
+          (colorize-string str %syntax-color))
+        identity))
+
+  (display (colorize (G_ "in form: ")) port)
+  (pretty-print form port))
+
 (define* (report-unbound-variable-error args #:key frame)
   "Return the given unbound-variable error, where ARGS is the list of 'throw'
 arguments."
@@ -398,6 +413,9 @@ (define* (report-load-error file args #:optional frame)
                    (formatted-message-arguments obj)))
            (else
             (report-error (G_ "exception thrown: ~s~%") obj)))
+     (when (syntax-error? obj)
+       (let ((form (or (syntax-error-subform obj) (syntax-error-form obj))))
+         (display-syntax form)))
      (when (fix-hint? obj)
        (display-hint (condition-fix-hint obj))))
     ((key args ...)
@@ -801,6 +819,9 @@ (define (call-with-error-handling thunk)
                      (and (error-location? c) (error-location c))
                      (gettext (formatted-message-string c) %gettext-domain)
                      (formatted-message-arguments c))
+              (when (syntax-error? c)
+                (let ((form (or (syntax-error-subform c) (syntax-error-form c))))
+                  (display-syntax form)))
               (when (fix-hint? c)
                 (display-hint (condition-fix-hint c)))
               (exit 1))
@@ -826,6 +847,9 @@ (define (call-with-error-handling thunk)
               (report-error (and (error-location? c) (error-location c))
                             (G_ "~a~%")
                             (gettext (condition-message c) %gettext-domain))
+              (when (syntax-error? c)
+                (let ((form (or (syntax-error-subform c) (syntax-error-form c))))
+                  (display-syntax form)))
               (when (fix-hint? c)
                 (display-hint (condition-fix-hint c)))
               (exit 1)))
-- 
2.33.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-guix-records-Improve-error-reporting.patch --]
[-- Type: text/x-patch, Size: 5267 bytes --]

From 2eebd1113a40fc6e7018975d3696546de584e4a0 Mon Sep 17 00:00:00 2001
Message-Id: <2eebd1113a40fc6e7018975d3696546de584e4a0.1637548566.git.julien@lepiller.eu>
In-Reply-To: <d1d857ea0ca76f1a917382777f57871e50026df3.1637548566.git.julien@lepiller.eu>
References: <d1d857ea0ca76f1a917382777f57871e50026df3.1637548566.git.julien@lepiller.eu>
From: Julien Lepiller <julien@lepiller.eu>
Date: Sun, 31 Oct 2021 02:58:14 +0100
Subject: [PATCH 2/2] guix: records: Improve error reporting.

* guix/records.scm (report-invalid-field-specifier): Handle various
invalidity causes separately.
---
 guix/records.scm | 56 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/guix/records.scm b/guix/records.scm
index ed94c83dac..eeb5908844 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2021 Julien Lepiller <julien@lepiller.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,9 +22,13 @@ (define-module (guix records)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-35)
+  #:use-module (ice-9 exceptions)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 rdelim)
+  #:use-module (guix diagnostics)
+  #:use-module (guix ui)
   #:autoload (system base target) (target-most-positive-fixnum)
   #:export (define-record-type*
             this-record
@@ -83,10 +88,53 @@ (define-syntax record-error
          ;; WEIRD may be an identifier, thus lacking source location info, and
          ;; BINDINGS is a list, also lacking source location info.  Hopefully
          ;; PARENT-FORM provides source location info.
-         (apply syntax-violation name "invalid field specifier"
-                (if parent-form
-                    (list parent-form #'weird)
-                    (list #'weird)))))))
+         (let* ((weird-properties (source-properties #'weird))
+                (parent-properties (and parent-form (syntax-source parent-form)))
+                (form parent-form)
+                (location (source-properties->location
+                            (or (and (not (null? weird-properties)) weird-properties)
+                                (and (not (null? parent-properties)) parent-properties)
+                                '()))))
+           (syntax-case #'weird ()
+             (()                             ;the empty list
+              (raise-exception
+                (condition
+                  (&message (message (G_ "invalid field specifier.")))
+                  (&syntax (form form) (subform (syntax->datum #'weird)))
+                  (&error-location (location location))
+                  (&fix-hint (hint (G_ "The format of a field is `(field value)'"))))))
+             (((field ...) _ ...)            ;a list whose first element is a list
+              (raise-exception
+                (condition
+                  (&message (message (G_ "invalid field name.")))
+                  (&syntax (form form) (subform (map syntax->datum #'(field ...))))
+                  (&error-location (location location)))))
+             ((field)                        ;a list with one element
+              (raise-exception
+                (condition
+                  (&message (message (G_ "missing value in field specifier.")))
+                  (&syntax (form form) (subform (syntax->datum #'weird)))
+                  (&error-location (location location))
+                  (&fix-hint (hint (format #f (G_ "The field is missing a value: `(~a <value>)'.")
+                                           (syntax->datum #'field)))))))
+             ((field value ...)              ;any other list
+              (raise-exception
+                (condition
+                  (&message (message (G_ "multiple values in field specifier.")))
+                  (&syntax (form form) (subform (syntax->datum #'weird)))
+                  (&error-location (location location))
+                  (&fix-hint (hint (format #f (G_ "~a values were associated with \
+field `~a'.  Perhaps the additional values were intended to be other field \
+specifiers.  This usually indicates missing or misplaced parenthesis.")
+                                           (length #'(value ...))
+                                           (syntax->datum #'field)))))))
+             (field                          ;not a list
+              (raise-exception
+                (condition
+                  (&message (message (G_ "invalid field specifier.")))
+                  (&syntax (form form) (subform (syntax->datum #'weird)))
+                  (&error-location (location location))
+                  (&fix-hint (hint (G_ "The format of a field is `(field value)'"))))))))))))
 
   (define (report-duplicate-field-specifier name ctor)
     "Report the first duplicate identifier among the bindings in CTOR."
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [bug#49671] [PATCH v3] guix: records: Improve error reporting.
  2021-11-22  2:40   ` [bug#49671] [PATCH v3] " Julien Lepiller
@ 2022-07-17  6:23     ` Julien Lepiller
  0 siblings, 0 replies; 6+ messages in thread
From: Julien Lepiller @ 2022-07-17  6:23 UTC (permalink / raw)
  To: 49671

Hi Guix!

A few months have passed without comment. I don't feel confident enough
to change Guix internals without comments.

What are your thoughts on this?

Le Mon, 22 Nov 2021 03:40:22 +0100,
Julien Lepiller <julien@lepiller.eu> a écrit :

> Here is another improvement compared to v2. This time there are two
> patches: the first adds support for &syntax in (guix ui), and will
> print something like
> 
> in form: <pretty-printed-form>
> 
> where "in form" is in green.
> 
> The second patch is very similar to v2, but will now also raise a
> &syntax condition, so it can be pretty-printed. The previous issue
> where I printed #<syntax ...> is fixed, I simply forgot a
> syntax->datum.
> 
> WDYT?





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-17  6:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-20 23:40 [bug#49671] [PATCH] guix: records: Improve error reporting Julien Lepiller
2021-07-21 19:21 ` Sarah Morgensen
2021-08-04 15:19 ` Ludovic Courtès
2021-10-31  2:06 ` Julien Lepiller
2021-11-22  2:40   ` [bug#49671] [PATCH v3] " Julien Lepiller
2022-07-17  6:23     ` Julien Lepiller

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).