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

Previous Next

Package: mumi;

Reported by: Arun Isaac <arunisaac <at> systemreboot.net>

Date: Tue, 30 May 2023 12:13:01 UTC

Severity: normal

Tags: patch

Done: Arun Isaac <arunisaac <at> systemreboot.net>

Bug is archived. No further changes may be made.

Full log


Message #65 received at 63802 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arun Isaac <arunisaac <at> systemreboot.net>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 63802 <at> debbugs.gnu.org
Subject: Re: bug#63802: [mumi PATCH 0/3] Use consolidated X-Debbugs-Cc header
Date: Tue, 18 Jul 2023 11:32:38 -0400
Hello,

Arun Isaac <arunisaac <at> 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




This bug report was last modified 1 year and 116 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.