Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't fail silently.

From: Maxim Cournoyer <[email-masked]>
Date: Mon, 23 Oct 2023 12:25:27 -0400

Hi Ludo,

Ludovic Courtès <[email-masked]> writes:

[...]

>> @@ -1048,9 +1086,19 @@ (define-syntax substitute*
>>  Alternatively, FILE may be a list of file names, in which case they are
>>  all subject to the substitutions.
>>  
>> +By default, SUBSTITUTE* will raise a &message condition if one of the patterns
>
> I think it’s ‘&substitute-error’ rather than ‘&message’.

Good catch.  Fixed.

>> +(define-substitute*-test test-error "substitute*, partial no match error"
>> +  #t                                  ;expected
>> +  "a\0b"                              ;content
>> +  (("a") "c"
>> +   ("Oops!") "c"))
>
> Maybe it’s not all that important here, but I think ‘test-error’ tests
> for any exception, whereas we’re looking specifically for a
> ‘&substitute-error’ condition filled with the right values.

I thought test-error was limited in srfi-64 that made it not work with
condition types, but it seems that's not the case and it actually does
work:

--8<---------------cut here---------------start------------->8---
modified   tests/build-utils.scm
@@ -320,8 +320,8 @@ (define-substitute*-test test-equal
   (("b") "d"))
 
 (define-substitute*-test test-error "substitute*, no match error"
-  #t                                  ;expected
-  "a\0b"                              ;content
+  &substitute-error                     ;expected
+  "a\0b"                                ;content
   (("Oops!") "c"))
 
 (define-substitute*-test test-equal "substitute*, no match, ignored"
@@ -331,8 +331,8 @@ (define-substitute*-test test-equal "substitute*, no match, ignored"
   #:require-matches? #f)
 
 (define-substitute*-test test-error "substitute*, partial no match error"
-  #t                                  ;expected
-  "a\0b"                              ;content
+  &substitute-error                     ;expected
+  "a\0b"                                ;content
   (("a") "c"
    ("Oops!") "c"))
--8<---------------cut here---------------end--------------->8---

and all tests still pass :-).  I'll send a v4.

-- 
Thanks,
Maxim