Op 19-06-2023 om 01:37 schreef Daniel Dinnyes:
> I have to apologise for the massive hatched-job I did with that "rebase"! :P
> Some references to my own modules were left in, other variables mixed up,
> etc.
> 
> I think it's in better shape now. Checked that tests run at least!
> It's the same feature branch URL 
> <https://github.com/dadinn/guile/tree/wip-hygienic-expect>... but I've 
> force pushed it! ;)


Some remarks:

1. The first commit 'added expect module' should instead be named
'Add incomplete hygienic expect module', because:

1(a) in Guile, the convention is to use the active voice in commit messages
1(b) There is already an expect module; the hygiene bit is the difference.
1(c) There are some missing bits, e.g. ‘use defaults for undefined 
parameter objects’ in (guile)Expect.

Likewise, the other commits need some changes in commit message.

1. The first commit and ‘Added original attribution comments’
could be squashed together -- I don't see a reason to have temporarily 
missing attribution information.

2. Likewise, the first commit can be squashed with parts of ‘Added 
original comments’.

3. For ‘use defaults for undefined parameter bindings': Guile has a 
thing named 'parameters' (search for make-parameter etc.), which isn't 
what is used here.  I recommend searching for other terminology.

Also, for clarity, I would add to the commit message something like: ‘This
‘Nowadays something like this would be implemented with parameter 
objects or syntax-parameterize, but that would be a backwards 
incompatible change.’.

4. In ‘Added interact macro implementation’, there is:

  > +(define interact-expect-port-error
  > +)

    that should be on a single line instead IMO.

5. I haven't directly compared the (ice-9 expect) with the (ice-9 
expect) yet, but it should now be simple to verify (and anyway, there 
are tests).

Best regards,
Maxime Devos.