[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