| Message ID | 20210930093229.4730-1-maximedevos@telenet.be |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#50914] records: Raise a &fix-hint if a field has multiple values. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
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’.
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
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
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."