Re: [PATCH v4 4/4] load: Display modules depth in output when using %load-verbosely.

[Maxime Devos <maximedevos@telenet.be>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1970 bytes --]



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.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]