[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 4/4] load: Display modules depth in output when using %loa
From: |
Maxime Devos |
Subject: |
Re: [PATCH v4 4/4] load: Display modules depth in output when using %load-verbosely. |
Date: |
Tue, 3 Oct 2023 21:03:21 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 |
Op 02-10-2023 om 18:13 schreef Maxim Cournoyer:
Something I didn't notice previously:
Op 25-09-2023 om 16:29 schreef Maxim Cournoyer:
+ if (scm_is_string (args)) {
+ /* C code written for 3.9 and earlier expects this function to
+ take a single argument (the file name). */
+ filename = args;
+ depth = scm_from_int(0);
+ }
IIUC, args is always a list and never a string. However, it might be
a list of one element (being a string) or two elements.
Surprisingly perhaps, this works, I guess because a string is an array.
See the 2009 commit 31ab99de563027fe2bceb60bbd712407fcaf868e which
exploits the same feature.
While apparently it works, that does not look like a feature to me but
rather an accident, though I don't know what hypothetical feature you
are referring to. Also, sure strings are arrays, but you don't seem to
be using that anywhere, so I'm not following.
%load-hook is carefully crafted as to accept both one or two arguments
(for backward compatibility). I didn't pay attention to %load-announce
and I'm surprised it's a public API, as its sole function seems to be
the default %load-hook value.
The default value might be, but it's supposed to be overwritable, and
according to the old API it's a one-argument procedure, so if there is
an old application/library setting it to something accepting only a
single argument, there is your API break.
To prevent future API breaks, I propose documenting turning %load-hook
into a keyword argument procedure with #:allow-other-keys, as something
like:
(lambda* (file-name #:key (depth the-default) #:allow-other-keys)
...)
and in the documentation mention that more keywords may be added in the
future (and hence #:allow-other-keys).
I think it's quite plausible that there will be more such arguments in
the future!
Best regards,
Maxime Devos.
OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature