{"thread":{"id":"21696","subject":"Re: [PATCH] send-email: new 'add-envelope' option","startedAt":"2009-11-21T17:43:30Z","lastAt":"2009-11-24T17:28:09Z","messageCount":11,"participants":["Felipe Contreras","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"128077","messageId":"1258825410-28592-1-git-send-email-felipe.contreras@gmail.com","threadId":"21696","inReplyTo":null,"subject":"[PATCH] send-email: new 'add-envelope' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-11-21T17:43:30Z","receivedAt":"2009-11-21T17:43:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Some MTAs make smart decisions based on the 'from' envelope (i.e. msmtp)\n\nSigned-off-by: Felipe Contreras \n---\n git-send-email.perl | 10 ++++++++--\n 1 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex a0279de..92bf491 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -140,6 +140,7 @@ my (@to,@cc,@initial_cc,@bcclist,@xh,\n \t$author,$sender,$smtp_authpass,$annotate,$compose,$time);\n \n my $envelope_sender;\n+my $envelope_from;\n \n # Example reply to:\n #$initial_reply_to = ''; #<20050203173208.GA23964@foobar.com>';\n@@ -208,6 +209,7 @@ my %config_settings = (\n \"aliasesfile\" => \\@alias_files,\n \"suppresscc\" => \\@suppress_cc,\n \"envelopesender\" => \\$envelope_sender,\n+ \"envelopefrom\" => \\$envelope_from,\n \"multiedit\" => \\$multiedit,\n \"confirm\" => \\$confirm,\n \"from\" => \\$sender,\n@@ -265,6 +267,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t \"confirm=s\" => \\$confirm,\n \t\t \"dry-run\" => \\$dry_run,\n \t\t \"envelope-sender=s\" => \\$envelope_sender,\n+\t\t \"envelope-from\" => \\$envelope_from,\n \t\t \"thread!\" => \\$thread,\n \t\t \"validate!\" => \\$validate,\n \t\t \"format-patch!\" => \\$format_patch,\n@@ -861,10 +864,13 @@ X-Mailer: git-send-email $gitversion\n \n \tmy @sendmail_parameters = ('-i', @recipients);\n \tmy $raw_from = $sanitized_sender;\n-\t$raw_from = $envelope_sender if (defined $envelope_sender);\n+\tif (defined $envelope_sender) {\n+\t\t$raw_from = $envelope_sender;\n+\t\t$envelope_from = 1;\n+\t}\n \t$raw_from = extract_valid_address($raw_from);\n \tunshift (@sendmail_parameters,\n-\t\t\t'-f', $raw_from) if(defined $envelope_sender);\n+\t\t\t'-f', $raw_from) if(defined $envelope_from);\n \n \tif ($needs_confirm && !$dry_run) {\n \t\tprint \"\\n$header\\n\";\n-- \n1.6.5.3.1.ga9388c\n"},{"id":"128073","messageId":"20091121193600.GA3296@coredump.intra.peff.net","threadId":"21696","inReplyTo":"1258825410-28592-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-21T19:36:00Z","receivedAt":"2009-11-21T19:36:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 21, 2009 at 07:43:30PM +0200, Felipe Contreras wrote:\n\n> Some MTAs make smart decisions based on the 'from' envelope (i.e. msmtp)\n\nSo my first thought was \"how in the world is this different from setting\nthe envelope sender?\"\n\nReading the code, it seems:\n\n> -\t$raw_from = $envelope_sender if (defined $envelope_sender);\n> +\tif (defined $envelope_sender) {\n> +\t\t$raw_from = $envelope_sender;\n> +\t\t$envelope_from = 1;\n> +\t}\n> \t$raw_from = extract_valid_address($raw_from);\n> \tunshift (@sendmail_parameters,\n> -\t\t\t'-f', $raw_from) if(defined $envelope_sender);\n> +\t\t\t'-f', $raw_from) if(defined $envelope_from);\n\nthat this is a boolean to mean \"use the from address as the envelope\nsender\".\n\nIt was of course all the more confusing for not being documented at all,\nbut even if documented, --envelope-from is IMHO confusingly similar to\n--envelope-sender. Maybe --use-from-in-envelope would be a better name?\n\nAnd of course, your patch is missing docs and tests.\n\n-Peff\n"},{"id":"128069","messageId":"94a0d4530911211159l1fadad0ldb0d760439ceb57@mail.gmail.com","threadId":"21696","inReplyTo":"20091121193600.GA3296@coredump.intra.peff.net","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-11-21T19:59:33Z","receivedAt":"2009-11-21T19:59:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 21, 2009 at 9:36 PM, Jeff King wrote:\n> So my first thought was \"how in the world is this different from setting\n> the envelope sender?\"\n>\n> Reading the code, it seems:\n\n[...]\n\n> that this is a boolean to mean \"use the from address as the envelope\n> sender\".\n>\n> It was of course all the more confusing for not being documented at all,\n> but even if documented,\n\nRight, I forgot the one-liner for the help.\n\n> --envelope-from is IMHO confusingly similar to\n> --envelope-sender. Maybe --use-from-in-envelope would be a better name?\n\nOk. I don't have any opinion on the name.\n\n> And of course, your patch is missing docs and tests.\n\nThere are no tests for 'envelope-sender', so I don't think it should\nbe a requirement for this patch to do so. I'll add the documentation\nthough.\n\nWill resend v2.\n\n-- \nFelipe Contreras\n"},{"id":"128090","messageId":"7v1vjrmfjw.fsf@alter.siamese.dyndns.org","threadId":"21696","inReplyTo":"94a0d4530911211159l1fadad0ldb0d760439ceb57@mail.gmail.com","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T02:58:11Z","receivedAt":"2009-11-22T02:58:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> There are no tests for 'envelope-sender', so I don't think it should\n> be a requirement for this patch to do so....\n\nThe fact that the lack of test was pointed out as a problem makes it a\nrequirement. Others' earlier mistakes are not an excuse for you to do a\npoor job.\n\nI do use --envelope-sender when sending patches out via msmtp. What\nwonderful things this patch adds am I missing in my current setup?\n"},{"id":"128100","messageId":"94a0d4530911220403w7238dab7saf18830a89f2add5@mail.gmail.com","threadId":"21696","inReplyTo":"7v1vjrmfjw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-11-22T12:03:01Z","receivedAt":"2009-11-22T12:03:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 22, 2009 at 4:58 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> There are no tests for 'envelope-sender', so I don't think it should\n>> be a requirement for this patch to do so....\n>\n> The fact that the lack of test was pointed out as a problem makes it a\n> requirement.  Others' earlier mistakes are not an excuse for you to do a\n> poor job.\n\nMaybe I should wait until somebody adds the tests to --envelope-sender\nbefore pushing this patch.\n\n> I do use --envelope-sender when sending patches out via msmtp.  What\n> wonderful things this patch adds am I missing in my current setup?\n\nYou need to specify your mail address when setting the variable,\nright? I use multiple email addresses, so that when I change it\n($EMAIL or user.email) the right one will be used in the envelope. If\nI use 'sendemail.envelopesender' I would\n need to change it as well.\n\nAnother option is to do something like 'sendemail.envelopesender=auto'.\n\n-- \nFelipe Contreras\n"},{"id":"128101","messageId":"94a0d4530911220437l5386b36ds4ef06dc66cc11bcc@mail.gmail.com","threadId":"21696","inReplyTo":"94a0d4530911211159l1fadad0ldb0d760439ceb57@mail.gmail.com","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-11-22T12:37:04Z","receivedAt":"2009-11-22T12:37:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 21, 2009 at 9:59 PM, Felipe Contreras\n wrote:\n> On Sat, Nov 21, 2009 at 9:36 PM, Jeff King wrote:\n>> --envelope-from is IMHO confusingly similar to\n>> --envelope-sender. Maybe --use-from-in-envelope would be a better name?\n>\n> Ok. I don't have any opinion on the name.\n\nI thought a bit more about this, and in the end what we really want is\nto add the sender envelope. The most typical case would be to use the\n'from' address, but we should be able to override it (we do both by\nusing --envelope-sender).\n\nSo here are other options.\n\na) --add-envelope: add the sender envelope, by default it would be the\n'from' address, but could be overridden by --envelope-sender.\n\nb) --envelope-sender=\"\" or \"auto\": this would require minimal changes\nbut looks a bit strange.\n\nAny thoughts?\n\n-- \nFelipe Contreras\n"},{"id":"128113","messageId":"7v4oomjytq.fsf@alter.siamese.dyndns.org","threadId":"21696","inReplyTo":"94a0d4530911220403w7238dab7saf18830a89f2add5@mail.gmail.com","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T16:42:25Z","receivedAt":"2009-11-22T16:42:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> I do use --envelope-sender when sending patches out via msmtp.  What\n>> wonderful things this patch adds am I missing in my current setup?\n>\n> You need to specify your mail address when setting the variable, right?\n> I use multiple email addresses, so that when I change it ($EMAIL or\n> user.email) the right one will be used in the envelope. If I use\n> 'sendemail.envelopesender' I would need to change it as well.\n\nAhh, I see. The above would be a good addition to the documentation part\nof the patch.\n"},{"id":"128118","messageId":"7v6392h2d9.fsf@alter.siamese.dyndns.org","threadId":"21696","inReplyTo":"94a0d4530911220403w7238dab7saf18830a89f2add5@mail.gmail.com","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T17:54:10Z","receivedAt":"2009-11-22T17:54:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Maybe I should wait until somebody adds the tests to --envelope-sender\n> before pushing this patch.\n\nYou can say that if you want to be difficult to work with, or you can be\nthat somebody yourself and make a difference.\n\nLet me show you that we can be constructive for a change ;-)\n\nHow about something trivial like this?\n\n-- >8 --\nSubject: [PATCH] t9001: test --envelope-sender option of send-email\n\nSigned-off-by: Junio C Hamano \n---\n t/t9001-send-email.sh | 17 +++++++++++++++++\n 1 files changed, 17 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 84a7f03..004e81c 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -95,6 +95,23 @@ test_expect_success \\\n 'Verify commandline' \\\n 'test_cmp expected commandline1'\n \n+test_expect_success 'Send patches with --envelope-sender' '\n+ clean_fake_sendmail &&\n+ git send-email --envelope-sender=\"Patch Contributer \" --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n+'\n+\n+cat >expected <<\\EOF\n+!patch@example.com!\n+!-i!\n+!nobody@example.com!\n+!author@example.com!\n+!one@example.com!\n+!two@example.com!\n+EOF\n+test_expect_success \\\n+ 'Verify commandline' \\\n+ 'test_cmp expected commandline1'\n+\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n (mbox) Adding cc: A from line 'From: A '\n"},{"id":"128136","messageId":"7v7htice04.fsf@alter.siamese.dyndns.org","threadId":"21696","inReplyTo":"94a0d4530911220437l5386b36ds4ef06dc66cc11bcc@mail.gmail.com","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T23:54:03Z","receivedAt":"2009-11-22T23:54:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> I thought a bit more about this, and in the end what we really want is\n> to add the sender envelope. The most typical case would be to use the\n> 'from' address, but we should be able to override it (we do both by\n> using --envelope-sender).\n>\n> So here are other options.\n>\n> a) --add-envelope: add the sender envelope, by default it would be the\n> 'from' address, but could be overridden by --envelope-sender.\n\nI do not think the latter half of this description makes much sense, as\nthe existing --envelope-sender= alone already says \"add this\nenvelope sender\".\n\nIt is unfortunate that we cannot sanely have an option that takes an\noptional string argument from the command line. Ideally, if we can\nspecify --envelope-sender without any argument, we could make it to mean\n\"pretend as if the 'from' address is given to this option\", but that would\nmake the command line pasing ambiguous, so we would need an extra option\nlike this one. \"--envelope-sender=from\" might be a more intuitive way to\nsay this, though.\n\n> b) --envelope-sender=\"\" or \"auto\": this would require minimal changes\n> but looks a bita strange.\n\nAn explicit empty string does look very strange, but I do not think a\nmagic word like \"auto\" (or \"from\") that cannot sanely be your envelope\nsender address is a bad idea.\n\n> Any thoughts?\n\nIt is much easier to work on the configuration front, by the way, and I\nexpect people who regularly interact with multiple projects to appreciate\nthis feature would configure their send-email once and forget about it, so\nthe command line clunkiness might not be such a big issue.\n\nA user who works with more than one projects with different identity known\nto each project would use $HOME/.gitconfig and send-email configuration\nidentity feature to set \"sendemail..from\" and friends in there,\nwhile setting sendemail.identity configuration in .git/config for each\nproject, so being able to say \"use whatever 'from' as the envelope sender\"\nonce in $HOME/.gitconfig would be convenient.\n\nSo I could have in $HOME/.gitconfig:\n\n [sendemail]\n\t\t; used as a boolean to say \"use from\"\n envelopesender\n [sendemail.git]\n from = Junio C Hamano \n to = git mailing list \n [sendemail.frotz]\n from = Junio C Hamano \n to = frotz mailing list \n\nand then in my {git,frotz}.git/.git/config would have\n\n [sendemail] [sendemail]\n identity = git identity = frotz\n\nrespectively. Without your patch, in $HOME/.gitconfig, I wouldn't have\nthe global sendemail.envelopesender but have separate individual\nconfiguration variables sendemail.{git,frotz}.envelopesender defined.\n"},{"id":"128200","messageId":"94a0d4530911231213k6ec3d4f4hb7bc293b8ddf92e8@mail.gmail.com","threadId":"21696","inReplyTo":"7v6392h2d9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-11-23T20:13:10Z","receivedAt":"2009-11-23T20:13:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 22, 2009 at 7:54 PM, Junio C Hamano wrote:\n> You can say that if you want to be difficult to work with, or you can be\n> that somebody yourself and make a difference.\n\nBeing difficult to work with goes both ways. You as a maintainer are\nentitled to say \"I won't accept this without test cases\", and I as a\ncontributor am entitled to say \"I'm not going to do that\". Since I'm\ndoing this on my free time I'd rather work on the things I like, which\ndoesn't include writing test cases from scratch, primarily because I'm\nnot familiar with them.\n\n> Let me show you that we can be constructive for a change ;-)\n>\n> How about something trivial like this?\n\nLooks good to me, but again, that doesn't say much.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"128253","messageId":"94a0d4530911240928w4f97cfa7of86b0d8676eb59db@mail.gmail.com","threadId":"21696","inReplyTo":"7v7htice04.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: new 'add-envelope' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-11-24T17:28:09Z","receivedAt":"2009-11-24T17:28:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 23, 2009 at 1:54 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>> a) --add-envelope: add the sender envelope, by default it would be the\n>> 'from' address, but could be overridden by --envelope-sender.\n>\n> I do not think the latter half of this description makes much sense, as\n> the existing --envelope-sender= alone already says \"add this\n> envelope sender\".\n\n--envelope-sender=foo@bar.com would imply --add-envelope.\n\n> It is unfortunate that we cannot sanely have an option that takes an\n> optional string argument from the command line.  Ideally, if we can\n> specify --envelope-sender without any argument, we could make it to mean\n> \"pretend as if the 'from' address is given to this option\", but that would\n> make the command line pasing ambiguous, so we would need an extra option\n> like this one.  \"--envelope-sender=from\" might be a more intuitive way to\n> say this, though.\n\nI agree, but --envelope-sender=from looks a bit strange to me.\n\n>> b) --envelope-sender=\"\" or \"auto\": this would require minimal changes\n>> but looks a bita strange.\n>\n> An explicit empty string does look very strange, but I do not think a\n> magic word like \"auto\" (or \"from\") that cannot sanely be your envelope\n> sender address is a bad idea.\n\nI think \"auto\" is the least worst option right now.\n\n>> Any thoughts?\n>\n> It is much easier to work on the configuration front, by the way, and I\n> expect people who regularly interact with multiple projects to appreciate\n> this feature would configure their send-email once and forget about it, so\n> the command line clunkiness might not be such a big issue.\n>\n> A user who works with more than one projects with different identity known\n> to each project would use $HOME/.gitconfig and send-email configuration\n> identity feature to set \"sendemail..from\" and friends in there,\n> while setting sendemail.identity configuration in .git/config for each\n> project, so being able to say \"use whatever 'from' as the envelope sender\"\n> once in $HOME/.gitconfig would be convenient.\n>\n> So I could have in $HOME/.gitconfig:\n>\n>        [sendemail]\n>                ; used as a boolean to say \"use from\"\n>                envelopesender\n>        [sendemail.git]\n>                from = Junio C Hamano \n>                to = git mailing list \n>        [sendemail.frotz]\n>                from = Junio C Hamano \n>                to = frotz mailing list \n>\n> and then in my {git,frotz}.git/.git/config would have\n>\n>        [sendemail]                     [sendemail]\n>                identity = git                  identity = frotz\n>\n> respectively.  Without your patch, in $HOME/.gitconfig, I wouldn't have\n> the global sendemail.envelopesender but have separate individual\n> configuration variables sendemail.{git,frotz}.envelopesender defined.\n\nI also thought about having an identity (profile) for sendmail; it\nwould be very useful to specify the smtp server, aliases file, and\nfrom address that multiple repositories can share and can be easily\nchanged (working for my company vs working for myself).\n\nHowever, even in this case I would like to have a global option to add\nthe sender automatically from the 'from' address.\n\n-- \nFelipe Contreras\n"}]}