[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
[bug#43159] [PATCH 0/2] Make 'guix help' helpful, Efraim Flashner, 2020/09/02
[bug#43159] [PATCH 0/2] Make 'guix help' helpful, zimoun, 2020/09/03