schemedoc / srfi-metadata

Import SRFI metadata into the Scheme API
https://docs.scheme.org/srfi/support/
MIT License
10 stars 2 forks source link

Add TR7 implementation #55

Closed a2379 closed 2 months ago

a2379 commented 3 months ago

Also small fix to let scripts run on Guix.

a2379 commented 2 months ago

Added a fix that catches an edge case where a number in a directory name would break the SRFI filename parsing regex.

Should be good to review now.

lassik commented 2 months ago

Thank you for the hard work. Sorry you had to deal with that awful shell-and-scheme contraption. It's my fault.

The shell scripts use bash instead of sh because set -o pipefail is an unfortunate omission from POSIX. The right thing is either to keep bash or to drop pipefail.

a2379 commented 2 months ago

Thank you for the hard work. Sorry you had to deal with that awful shell-and-scheme contraption. It's my fault.

I don't think there's a better (portable) way to do it, unless a SRFI for spawning subprocesses gets approved. But it would probably be impossible to get everyone to agree on such a SRFI.

The shell scripts use bash instead of sh because set -o pipefail is an unfortunate omission from POSIX. The right thing is either to keep bash or to drop pipefail.

What I wanted to do was account for distros where bash does not exist under /bin, like in Guix. Using /usr/bin/env to locate bash should work for all distros, I think. Updated PR with that change.

lassik commented 2 months ago

[...] unless a SRFI for spawning subprocesses gets approved. But it would probably be impossible to get everyone to agree on such a SRFI.

Hard to tell. The problem can be attacked at multiple levels of abstraction, and multiple levels of flexibility.

(srfi-tools private sysdep) is an example of a useful minimal interface.

What I wanted to do was account for distros where bash does not exist under /bin, like in Guix. Using /usr/bin/env to locate bash should work for all distros, I think. Updated PR with that change.

Right. Good call.

The PR looks good to me. Ready to merge?

a2379 commented 2 months ago

The PR looks good to me. Ready to merge?

Yes, it's good to go.