[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#42146] [PATCH core-updates 1/?] build: substitute: Don't fail silen
From: |
Ludovic Courtès |
Subject: |
[bug#42146] [PATCH core-updates 1/?] build: substitute: Don't fail silently. |
Date: |
Mon, 23 Oct 2023 10:42:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi Maxim,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> Ludovic Courtès <ludo@gnu.org> writes:
[...]
>>> +;;; Extend regexp objects with a pattern field.
>>> +;;;
>>> +(define-record-type <regexp*>
>>> + (%make-regexp* pat flag rx)
>>> + regexp*?
>>> + (pat regexp*-pattern) ;the regexp pattern, a string
>>> + (flag regexp*-flag) ;regexp flags
>>> + (rx regexp*-rx)) ;the compiled regexp object
>>> +
>>> +;;; Work around regexp implementation.
>>> +;;; This record allows to track the regexp pattern and then display it.
>>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>>
>> I’m skeptical about the concrete benefits. I would not include it in
>> (guix build utils), or at least not in this patch series.
[...]
> The benefit is concrete: it makes it possible to show which regexp
> pattern failed to match (its textual representation), instead of
> something much less useful such as a generic placeholder such as
> "<unknown> (regexp)".
Yes, okay.
Can we keep the <regexp*> interface private, then? To me it’s an
implementation detail and perhaps a bit of a hack that I’d rather not
commit to maintaining.
The cost might be to duplicate it in teams.scm, but that’s an acceptable
cost IMO.
> It's in actual use if you look at the definition of substitute:
>
>
> (let ((rx+proc (map (match-lambda
> (((or (? regexp? pattern) (? regexp*? pattern)) .
> proc)
> (cons pattern proc))
> ((pattern . proc)
> (cons (make-regexp* pattern regexp/extended) proc)))
> pattern+procs)))
>
> The previous version followed a different approach, annotating the
> rx+proc list with the raw pattern; I think the approach here is a bit
> cleaner, and it should also enable users to pass pre-computed regexp*
> objects to substitute* and have useful error messages produced.
Note that ‘substitute*’ only takes string literals as patterns, not
regexp objects. The pattern part of clauses has sometimes been abused
to pass code, typically ‘string-append’ calls, but that’s definitely not
the spirit.
Another approach would have been, in ‘substitute*’, to capture the
regexp-as-string as well as other useful debugging info, such as its
source location.
Thanks,
Ludo’.