all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values.
@ 2021-09-30  9:32 Maxime Devos
  2021-10-04 14:26 ` Ludovic Courtès
  2021-10-05  2:02 ` Jack Hill
  0 siblings, 2 replies; 4+ messages in thread
From: Maxime Devos @ 2021-09-30  9:32 UTC (permalink / raw)
  To: 50914; +Cc: Maxime Devos

* guix/records.scm (report-invalid-field-specifier): If
  'weird' is something like (field (record ...) extra ...), hint that 'extra
  ...' should probably be moved inside (record ...).
---
 guix/records.scm | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/guix/records.scm b/guix/records.scm
index ed94c83dac..db0c0a7ca0 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 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,10 +22,13 @@
   #: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)
   #:autoload (system base target) (target-most-positive-fixnum)
+  #:autoload (guix diagnostics) (&fix-hint)
   #:export (define-record-type*
             this-record
 
@@ -83,10 +87,35 @@ 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"
+         (let ((forms
                 (if parent-form
                     (list parent-form #'weird)
-                    (list #'weird)))))))
+                    (list #'weird))))
+           (syntax-case #'weird ()
+             ;; common mistake
+             ((field (record-name fields ...) extra-value extra-value* ...)
+              (raise-exception
+               (condition
+                (&origin (origin name))
+                (&message (message
+                           (format #f "field ‘~a’ should only have one \
+value, but an extra value ‘~a’ was passed as well.  Perhaps this extra \
+value was supposed to be a field specifier, and needs to be moved inside \
+the record ‘~a’?"
+                                   (syntax->datum #'field)
+                                   (syntax->datum #'extra-value)
+                                   (syntax->datum #'(record-name fields ...)))))
+                (&syntax (form (car forms))
+                         (subform (and (not (null? (cdr forms)))
+                                       (cadr forms))))
+                (&fix-hint (hint (object->string
+                                  (syntax->datum
+                                   #'(field
+                                      (record-name fields ... extra-value
+                                                   extra-value* ...)))))))))
+             (_
+              (apply syntax-violation name
+                     "invalid field specifier" forms))))))))
 
   (define (report-duplicate-field-specifier name ctor)
     "Report the first duplicate identifier among the bindings in CTOR."

base-commit: 808f9ffbd3106da4c92d2367b118b98196c9e81e
prerequisite-patch-id: 7fdac44e8681baaf419cbf8da78cdebb8b9f9757
prerequisite-patch-id: 1f7f1597b9c85b2b1f9db1044d193bcf6ec8650e
prerequisite-patch-id: 588ca94b9c4603424094a9cc2854c4f9bc83c7e4
prerequisite-patch-id: 82b4951463e8979d1c4cd15e1ca6a36308b21b51
prerequisite-patch-id: 75cdb9eb6b038adfb605253163b94efd51e0276c
prerequisite-patch-id: 35140f4f2873d0b9f4fc8caca6ec2e013ecb830a
prerequisite-patch-id: ed97d14afd166e7b6cac37e3aa87a85246f7e320
prerequisite-patch-id: 3f3d43f5583dce32af7d4e9925771e581c3cc5ee
prerequisite-patch-id: e8f735697c0535afe9335448b16e3e1f308de362
prerequisite-patch-id: eaf1f67c4c07482fb4da81525cbd5dcb1ea2194e
prerequisite-patch-id: 9a15aa08fbbbf110ba76409dcc2a3ab5e0764806
prerequisite-patch-id: 675a3c516f47dfcbaf61d5ad41ca7f3babdd3f20
prerequisite-patch-id: ac188cb61957c9639d0ac125c941950afbdba9c7
prerequisite-patch-id: f3f1f02944a4aa9635ce7094bfda254315d990b3
prerequisite-patch-id: 5eee450b2221d67fbda1e6581d16628394c912a7
prerequisite-patch-id: bad535152857928abf624dc49dc2b27718d3885e
prerequisite-patch-id: 623edc835c2c5dfd8c83dcf32e650cfebea42aa0
prerequisite-patch-id: c0f50259e7fc09455f77dca31113b0b55e955220
prerequisite-patch-id: 9c7c0929a48b103b6b69626dbc86d7744f2f40ad
prerequisite-patch-id: 8f8c2af0b856f56c7798a3b79ba90b073bbb382f
prerequisite-patch-id: 7d88402829a8967c23650dffe115a94ae26433da
prerequisite-patch-id: ffb3d6215a89195f6e0f274f2f119c1f7b65259a
prerequisite-patch-id: 54eec153e523b58c3670c48afda9ef50ec44eb8e
prerequisite-patch-id: bc5dfc06e9d67d10a37fbd7ba61939907d93ca7c
prerequisite-patch-id: f85cc1c9eac0d44f40afe2a547ac3d866769f685
prerequisite-patch-id: 0db9692e872bf73242cfec6f8aa390abe14d08f1
prerequisite-patch-id: 36431a656d29e90e8eb218730c64807e2477c9d7
prerequisite-patch-id: 6817d73f5e42ccd33b19642ea4ef62814ad2b10e
prerequisite-patch-id: 9146aec4a40f7da60a4c64643a9aa0e405567b04
prerequisite-patch-id: ff2daf978d58ec12c25dcbce4e7ee010d337cd54
prerequisite-patch-id: c88d74073fcd5d1ff19a4a9338b0df63b648d98a
prerequisite-patch-id: 3032f2193b95d5cb625e33daec015f3354f01903
-- 
2.33.0





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

* [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values.
  2021-09-30  9:32 [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values Maxime Devos
@ 2021-10-04 14:26 ` Ludovic Courtès
  2021-10-05  2:02 ` Jack Hill
  1 sibling, 0 replies; 4+ messages in thread
From: Ludovic Courtès @ 2021-10-04 14:26 UTC (permalink / raw)
  To: Maxime Devos; +Cc: 49671, 50914

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> * guix/records.scm (report-invalid-field-specifier): If
>   'weird' is something like (field (record ...) extra ...), hint that 'extra
>   ...' should probably be moved inside (record ...).

Please see also <https://issues.guix.gnu.org/49671>.

> +               (condition
> +                (&origin (origin name))
> +                (&message (message
> +                           (format #f "field ‘~a’ should only have one \
> +value, but an extra value ‘~a’ was passed as well.  Perhaps this extra \
> +value was supposed to be a field specifier, and needs to be moved inside \
> +the record ‘~a’?"

We’ll need i18n here and straight quotes.

> +                (&syntax (form (car forms))
> +                         (subform (and (not (null? (cdr forms)))
> +                                       (cadr forms))))

No cadrcdr please.  :-)

> +                (&fix-hint (hint (object->string
> +                                  (syntax->datum
> +                                   #'(field
> +                                      (record-name fields ... extra-value
> +                                                   extra-value* ...)))))))))

The ‘hint’ field must be a string, typically the message you had above;
see other examples in the code.

TIA!

Ludo’.




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

* [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values.
  2021-09-30  9:32 [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values Maxime Devos
  2021-10-04 14:26 ` Ludovic Courtès
@ 2021-10-05  2:02 ` Jack Hill
  2021-10-05  4:06   ` Jack Hill
  1 sibling, 1 reply; 4+ messages in thread
From: Jack Hill @ 2021-10-05  2:02 UTC (permalink / raw)
  To: Maxime Devos; +Cc: 50914

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

Wow, thanks for picking up on an annoyance I mentioned on IRC an providing 
a patch!

On Thu, 30 Sep 2021, Maxime Devos wrote:

> * guix/records.scm (report-invalid-field-specifier): If
>  'weird' is something like (field (record ...) extra ...), hint that 'extra
>  ...' should probably be moved inside (record ...).
> ---

I don't have any comments on the code directly, but while the error 
message is much improved, and would have helped me find my mistake 
quicker, it's still a bit verbose and doesn't seem to be printing your 
nice error message. The error I now get is,

```
gnu/packages/mail.scm:2884:2: error: (package (name "msgconvert") (version "0.920") (source (origin (method git-fetch) (uri (git-reference (url "https://github.com/mvz/email-outlook-message-perl") (commit "dd382f47fd112032bf91cb673178a27142d23e38"))))) (sha256 (base32 "0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4")) (file-name (git-file-name name version)) (build-system perl-build-system) (native-inputs (quasiquote (("perl-module-build" (unquote perl-module-build))))) (inputs (quasiquote (("perl-email-address" (unquote perl-email-address)) ("perl-email-mime" (unquote perl-email-mime)) ("perl-email-mime-contenttype" (unquote perl-email-mime-contenttype)) ("perl-email-sender" (unquote perl-email-sender)) ("perl-email-simple" (unquote perl-email-simple)) ("perl-io-all" (unquote perl-io-all)) ("perl-io-string" (unquote perl-io-string)) ("perl-ole-storage-lite" (unquote perl-ole-storage-lite))))) (home-page "https://www.matijs.net/software/msgconv") (synopsis "Foo") (description "
 Foo.") (license license:gpl1+)): extraneous field initializers (sha256 file-name)
```

For reference, the faulty package definition I used to test:

```
(define-public msgconvert
   (package
     (name "msgconvert")
     (version "0.920")
     (source
      (origin
        (method git-fetch)
        (uri
         (git-reference
          (url "https://github.com/mvz/email-outlook-message-perl")
          (commit "dd382f47fd112032bf91cb673178a27142d23e38" ;; (string-append "v" version)
                  )))
        ))
     (sha256
         (base32 "0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4"))
        (file-name (git-file-name name version))
     (build-system perl-build-system)
     (native-inputs
      `(("perl-module-build" ,perl-module-build)))
     (inputs
      `(("perl-email-address" ,perl-email-address)
        ("perl-email-mime" ,perl-email-mime)
        ("perl-email-mime-contenttype" ,perl-email-mime-contenttype)
        ("perl-email-sender" ,perl-email-sender)
        ("perl-email-simple" ,perl-email-simple)
        ("perl-io-all" ,perl-io-all)
        ("perl-io-string" ,perl-io-string)
        ("perl-ole-storage-lite" ,perl-ole-storage-lite)))
     (home-page "https://www.matijs.net/software/msgconv")
     (synopsis "Foo")
     (description "Foo.")
     (license license:gpl1+)))
```

Best,
Jack

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

* [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values.
  2021-10-05  2:02 ` Jack Hill
@ 2021-10-05  4:06   ` Jack Hill
  0 siblings, 0 replies; 4+ messages in thread
From: Jack Hill @ 2021-10-05  4:06 UTC (permalink / raw)
  To: Maxime Devos; +Cc: 50914

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

On Mon, 4 Oct 2021, Jack Hill wrote:

> Wow, thanks for picking up on an annoyance I mentioned on IRC an providing a 
> patch!
>
> On Thu, 30 Sep 2021, Maxime Devos wrote:
>
>> * guix/records.scm (report-invalid-field-specifier): If
>>  'weird' is something like (field (record ...) extra ...), hint that 'extra
>>  ...' should probably be moved inside (record ...).
>> ---
>
> I don't have any comments on the code directly, but while the error message 
> is much improved, and would have helped me find my mistake quicker, it's 
> still a bit verbose and doesn't seem to be printing your nice error message.

I did perturb the package definition in different ways and got your error 
message

```
error: field ‘uri’ should only have one value, but an extra value ‘(sha256 (base32 0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4))’ was passed as well.  Perhaps this extra value was supposed to be a field specifier, and needs to be moved inside the record ‘(git-reference (url https://github.com/mvz/email-outlook-message-perl) (commit dd382f47fd112032bf91cb673178a27142d23e38))’?
hint: (uri (git-reference (url "https://github.com/mvz/email-outlook-message-perl") (commit
"dd382f47fd112032bf91cb673178a27142d23e38") (sha256 (base32
"0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4")) (file-name (git-file-name
name version))))
```

with

```
(define-public msgconvert
   (package
     (name "msgconvert")
     (version "0.920")
     (source
      (origin
        (method git-fetch)
        (uri
         (git-reference
          (url "https://github.com/mvz/email-outlook-message-perl")
          (commit "dd382f47fd112032bf91cb673178a27142d23e38" ;; (string-append "v" version)
))
         (sha256
          (base32 "0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4"))
         (file-name (git-file-name name version)))))
     (build-system perl-build-system)
     (native-inputs
      `(("perl-module-build" ,perl-module-build)))
     (inputs
      `(("perl-email-address" ,perl-email-address)
        ("perl-email-mime" ,perl-email-mime)
        ("perl-email-mime-contenttype" ,perl-email-mime-contenttype)
        ("perl-email-sender" ,perl-email-sender)
        ("perl-email-simple" ,perl-email-simple)
        ("perl-io-all" ,perl-io-all)
        ("perl-io-string" ,perl-io-string)
        ("perl-ole-storage-lite" ,perl-ole-storage-lite)))
     (home-page "https://www.matijs.net/software/msgconv")
     (synopsis "Foo")
     (description "Foo.")
     (license license:gpl1+)))
```

Best,
Jack

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

end of thread, other threads:[~2021-10-05  4:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-30  9:32 [bug#50914] [PATCH] records: Raise a &fix-hint if a field has multiple values Maxime Devos
2021-10-04 14:26 ` Ludovic Courtès
2021-10-05  2:02 ` Jack Hill
2021-10-05  4:06   ` Jack Hill

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.