Re: [PATCH] send-email: move process_address_list earlier to avoid, uninitialized address error
On 10/11/23 18:47, Jeff King wrote:
Show 28 quoted lines
> On Wed, Oct 11, 2023 at 03:37:39PM -0700, Junio C Hamano wrote:
>
>> On the other hand, I am not sure what is wrong with "after the user
>> typed", actually. As you said, anybody sane would be using --to (or
>> an equivalent configuration variable in the repository) to send
>> their patches to the project address instead of typing, and to them
>> it is not a problem. After getting the recipient address from the
>> end user, the validation may fail due to a wrong address, in which
>> case it is a good thing. If the validation failed due to wrong
>> contents of the patch (perhaps it included a change to the file with
>> trade secret that appeared in the context lines), as long as the
>> reason why the validation hook rejected the patches is clear enough
>> (e.g., "it's the patches, not the recipients"), such "a rejection
>> after typing" would be only once per a patch series, so it does not
>> sound too bad, either.
>>
>> But perhaps I am not seeing the reason why "fail after the user typed"
>> is so disliked and being unnecessarily unsympathetic. I dunno.
> I did not look carefully at the flow of send-email, so this may or may
> not be an issue. But what I think would be _really_ annoying is if you
> asked to write a cover letter, went through the trouble of writing it,
> and then send-email bailed due to some validation failure that could
> have been checked earlier.
>
> There is probably a way to recover your work (presumably we leave it in
> a temporary file somewhere), but it may not be entirely trivial,
> especially for users who are not comfortable with advanced usage of
> their editor. ;)
As I was looking at covering the case of interactive input (--compose) to the fix I noticed that this seems to be at least partly handled by the $compose_filename code. There is a nice output message telling you exactly where the intermediate version of the email you are composing is located if there are errors. I took a quick look inside and can verify that any lost work should be minimal as long as someone knows how to edit files with their editor of choice.
Show 14 quoted lines
>
> I seem to remember we had one or two such problems in the early days
> with "git commit", where you would go to the trouble to type a commit
> message only to bail on some condition which _could_ have been checked
> earlier. You can recover the message from .git/COMMIT_EDITMSG, but you
> need to remember to do so before re-invoking "git commit", otherwise it
> gets obliterated.
>
> Now for send-email, if your flow is to generate the patches with
> "format-patch", then edit the cover letter separately, and then finally
> ship it all out with "send-email", that might not be an issue. But some
> workflows use the --compose option instead.
>
> -Peff
I have been looking into handling the interactive input cases while solving this issue, but have yet to make a breakthrough. Simply moving the validation code below the original process_address_list code results in a a scenario where I get the email address being seen as something like "ARRAY (0x55ddb951d768)" rather than the email address I wrote in the compose buffer.