guix-patches
[Top][All Lists]
Advanced

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

[bug#63802] [mumi PATCH 0/3] Use consolidated X-Debbugs-Cc header


From: Maxim Cournoyer
Subject: [bug#63802] [mumi PATCH 0/3] Use consolidated X-Debbugs-Cc header
Date: Tue, 18 Jul 2023 11:32:38 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)

Hello,

Arun Isaac <arunisaac@systemreboot.net> writes:


[...]

>>> +            git-send-email-headers
>>> +            compose))
>>
>> I think you've exported 'compose' erroneously here.
>
> Good catch! compose is part of a new "mumi compose" feature I am working
> on. I had accidentally committed it. I have removed it from this commit.
>
> Now that you mention it, maybe I should call it compose-email so as to
> not conflict with compose from guile core.

Good idea!  Shadowing builtins should be avoided; the warnings are
annoying and require the use of #:hide on imports (and the code more
confusing to read).

[...]

>> but: does call-with-input-pipe* raise an exception when git is available
>> but 'sendemail.headerCmd' not set, thus exiting with status 1?  I wasn't
>> able to find its documentation in the Guile Reference manual.
>
> call-with-input-pipe* and call-with-input-pipe are both defined in
> mumi/client.scm. They are not part of guile. The only difference between
> them is whether they accept the command as a string or as a list of
> arguments---thus, they parallel open-pipe and open-pipe*.
>
>> Otherwise you'd get header-command set to the empty string, which
>> seems like it'd be a problem...
>
> call-with-input-pipe* does raise an exception when git is available but
> sendemail.headerCmd is not set. I checked. So, this is not a problem.

Good, thanks for checking.

>>> +         (headers
>>> +          (if header-command
>>> +              (call-with-input-pipe (string-append header-command " " 
>>> patch)
>>
>>                   ^ ... here.  Also, why the mixed use of
>>                   'call-with-input-pipe*' and 'call-with-input-pipe'?  I'd
>>                   stick with the former.
>
> sendemail.headerCmd is only available to us as a string, and not as a
> list of arguments. It is quite non-trivial to correctly split the string
> back into a list of arguments. That would require correct handling of
> quotes like the shell does. So, we use call-with-input-pipe to handle
> this case.

Ah, I see.  It's reasonable then to use it as is.

> But everywhere else (such as when invoking "git config
> sendemail.headerCmd"), we prefer to pass commands as a list of
> arguments. So, we need call-with-input-pipe*.
>
> I understand it's a bit confusing to have two very similar
> functions. But, the only possible compromise is to use
> call-with-input-pipe everywhere. Should I make that compromise? WDYT?

No, just the explanation here (and a possible comment in the source
mirroring it) is enough!

LGTM.

-- 
Thanks,
Maxim





reply via email to

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