| Message ID | 20201029231000.14568-5-ludo@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Adding a (guix transformations) module | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/submitting builds | success | |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
Hi, On Fri, 30 Oct 2020 at 00:09, Ludovic Courtès <ludo@gnu.org> wrote: > (transform-package-toolchain): Use 'raise' and 'formatted-message' > instead of 'leave'. Naive question: what is the difference? Other said why? And so why not replaces ’leave’ here and there by ’raise’+’formatted-message’? All the best, simon
Hi simon and Ludo, zimoun <zimon.toutoune@gmail.com> writes: > Hi, > > On Fri, 30 Oct 2020 at 00:09, Ludovic Courtès <ludo@gnu.org> wrote: > >> (transform-package-toolchain): Use 'raise' and 'formatted-message' >> instead of 'leave'. > > Naive question: what is the difference? Other said why? And so why not > replaces ’leave’ here and there by ’raise’+’formatted-message’? Probably Ludo can explain better, but the main difference I see is that leave (guix diagnostics) calls exit, so the client code now can handle these errors and it isn't forced to exit. The patch LGTM. Just one extra comment, this changes some format strings, so we shouldn't forget to send the latest pot files to TP as soon as we generate the branch for the release. Happy hacking! Miguel
Hi, Miguel Ángel Arruga Vivas <rosen644835@gmail.com> skribis: > zimoun <zimon.toutoune@gmail.com> writes: > >> Hi, >> >> On Fri, 30 Oct 2020 at 00:09, Ludovic Courtès <ludo@gnu.org> wrote: >> >>> (transform-package-toolchain): Use 'raise' and 'formatted-message' >>> instead of 'leave'. >> >> Naive question: what is the difference? Other said why? And so why not >> replaces ’leave’ here and there by ’raise’+’formatted-message’? > > Probably Ludo can explain better, but the main difference I see is that > leave (guix diagnostics) calls exit, so the client code now can handle > these errors and it isn't forced to exit. Exactly. In fact, ‘exit’ throws the 'quit exception, which one could also catch, but it’s still nicer to throw &message or &formatted-message within a module meant to be used as a library. > The patch LGTM. Just one extra comment, this changes some format > strings, so we shouldn't forget to send the latest pot files to TP as > soon as we generate the branch for the release. Yes. Thanks, Ludo’.
Pushed as 95460da83b6ffd2bf3b96b3ab7cd302ab76be38a, thanks! Ludo’.
Hi, Thanks for the explanation. On Sat, 31 Oct 2020 at 11:06, Ludovic Courtès <ludo@gnu.org> wrote: > >> Naive question: what is the difference? Other said why? And so why not > >> replaces ’leave’ here and there by ’raise’+’formatted-message’? > > > > Probably Ludo can explain better, but the main difference I see is that > > leave (guix diagnostics) calls exit, so the client code now can handle > > these errors and it isn't forced to exit. > > Exactly. In fact, ‘exit’ throws the 'quit exception, which one could > also catch, but it’s still nicer to throw &message or &formatted-message > within a module meant to be used as a library. And so why not replace the 'leave' appearing here and there in the code by 'raise'+'formatted-message'? Cheers, simon
zimoun <zimon.toutoune@gmail.com> skribis: > On Sat, 31 Oct 2020 at 11:06, Ludovic Courtès <ludo@gnu.org> wrote: > >> >> Naive question: what is the difference? Other said why? And so why not >> >> replaces ’leave’ here and there by ’raise’+’formatted-message’? >> > >> > Probably Ludo can explain better, but the main difference I see is that >> > leave (guix diagnostics) calls exit, so the client code now can handle >> > these errors and it isn't forced to exit. >> >> Exactly. In fact, ‘exit’ throws the 'quit exception, which one could >> also catch, but it’s still nicer to throw &message or &formatted-message >> within a module meant to be used as a library. > > And so why not replace the 'leave' appearing here and there in the > code by 'raise'+'formatted-message'? In (guix scripts …), using ‘leave’ is appropriate. Ludo’.
diff --git a/guix/transformations.scm b/guix/transformations.scm index 126a9a69d3..30142dd059 100644 --- a/guix/transformations.scm +++ b/guix/transformations.scm @@ -38,6 +38,7 @@ #:use-module (srfi srfi-9) #:use-module (srfi srfi-11) #:use-module (srfi srfi-26) + #:use-module (srfi srfi-34) #:use-module (srfi srfi-37) #:use-module (ice-9 match) #:export (options->transformation @@ -169,7 +170,9 @@ package it refers to could not be found." (lambda (old) (proc old new))))) (x - (leave (G_ "invalid replacement specification: ~s~%") spec)))) + (raise (formatted-message + (G_ "invalid replacement specification: ~s") + spec))))) specs)) (define (transform-package-inputs replacement-specs) @@ -216,8 +219,9 @@ the source of PACKAGE is not fetched from a Git repository." ((git-checkout? source) (git-checkout-url source)) (else - (leave (G_ "the source of ~a is not a Git reference~%") - (package-full-name package)))))) + (raise + (formatted-message (G_ "the source of ~a is not a Git reference") + (package-full-name package))))))) (define (evaluate-git-replacement-specs specs proc) "Parse SPECS, a list of strings like \"guile=stable-2.2\", and return a list @@ -234,7 +238,9 @@ syntax, or if a package it refers to could not be found." (cons spec replace)) (_ - (leave (G_ "invalid replacement specification: ~s~%") spec)))) + (raise + (formatted-message (G_ "invalid replacement specification: ~s") + spec))))) specs)) (define (transform-package-source-branch replacement-specs) @@ -304,8 +310,10 @@ a checkout of the Git repository at the given URL." (source (git-checkout (url url) (recursive? #t))))))) (_ - (leave (G_ "~a: invalid Git URL replacement specification~%") - spec)))) + (raise + (formatted-message + (G_ "~a: invalid Git URL replacement specification") + spec))))) replacement-specs)) (define rewrite @@ -380,8 +388,10 @@ the equal sign." ((spec (= split-on-commas toolchain)) (cons spec (map specification->input toolchain))) (_ - (leave (G_ "~a: invalid toolchain replacement specification~%") - spec)))) + (raise + (formatted-message + (G_ "~a: invalid toolchain replacement specification") + spec))))) replacement-specs)) (lambda (obj)