On 10/11/23 18:47, Jeff King wrote: > 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. > > 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.
