guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix hel


From: Maxim Cournoyer
Subject: [bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix help' use that.
Date: Sun, 13 Sep 2020 19:33:24 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Hi Ludovic!

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Sorry I couldn't reply faster.
>
> No problem.  I went ahead with this patch series, but nothing’s set in
> stone and I’m open to further changes.

No problem!  I'm glad this nice UI improvement has already landed.

>> Thanks for the detailed measurements!  It does indeed seem your approach
>> is better, especially considering memory usage.  Perhaps the commands
>> could have been moved to dedicated modules not using much dependency at
>> all so that their closure would have been small hence fast to load, but
>> keeping the commands definitions local to where they are useful is
>> definitely a nice property.
>
> We can’t really reduce the closure of commands.  In particular, (guix
> scripts system) has to load pretty much “everything”.
>
> What we could do is use #:autoload aggressively in the (guix scripts …)
> modules, such that startup time would be as small as possible—e.g.,
> ‘--help’ would not trigger loading of a zillion modules.
>
> It’s a bit tedious though and not necessarily helpful in the general
> case where one is doing something non-trivial with the command.  Well
> dunno, we could try!

Seems like too much micro-management.  This further solidifies your
design choice as the right one :-).

>>> In summary, while this approach undoubtedly looks awkward to any Lisper,
>>> I think it’s a good way to not contribute to the general impression of
>>> sluggishness and resource-hungriness of ‘guix’ commands.  :-)
>>>
>>>>> +  (define (display-commands commands)
>>>>> +    (let* ((names     (map (lambda (command)
>>>>> +                             (string-join (command-name command)))
>>>>> +                           commands))
>>>>> +           (max-width (reduce max 0 (map string-length names))))
>>>>
>>>> You can drop reduce and use (max (map string-length names)) instead.
>>>
>>> I could do (apply max (map …)) but I don’t like the idea of abusing
>>> variadic argument lists in that way—I know, it’s very subjective.  ;-)
>>
>> Eh, I wonder why?  I may be missing something, but if max allows it,
>> doesn't it mean it's a valid use?  Anyway, just curious to know what are
>> the grounds for this personal preference :-).
>
> It’s mostly aesthetic, but it comes from the idea that there could be
> limitations on the maximum number of arguments a procedure can take, or
> inefficiencies with dealing with many arguments.  Now, in today’s Guile,
> there are no such issues…  (And now I look really silly!)  :-)

Ha!  Thanks for explaining.  Now I know I can shamelessly continue using
apply on procedures accepting N arguments :-).

Maxim





reply via email to

[Prev in Thread] Current Thread [Next in Thread]