{"thread":{"id":"11374","subject":"[PATCH] Make git send-email accept $EDITOR with arguments","startedAt":"2007-12-20T20:14:06Z","lastAt":"2007-12-22T09:13:26Z","messageCount":19,"participants":["Gustaf Hendeby","Luciano Rocha","Jeff King","Junio C Hamano","David Symonds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"63873","messageId":"1198181646-28632-1-git-send-email-hendeby@isy.liu.se","threadId":"11374","inReplyTo":null,"subject":"[PATCH] Make git send-email accept $EDITOR with arguments","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-20T20:14:06Z","receivedAt":"2007-12-20T20:14:06Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"Currently git send-email does not accept $EDITOR with arguments, eg,\nemacs -nw, when starting an editor to produce a cover letter.  This fixes\nthis in the simplest way possible, assume all spaces separates either\nthe command from the first argument, or two arguments.  This should\nwork in most cases, but will break with quoted strings embedded spaces.\nAn example of a problematic case is when there is a space in the path\nto the command.\n\nSigned-off-by:  Gustaf Hendeby <hendeby@isy.liu.se>\n---\n\nThis is related to the problems recently observed in the built in git\ncommit and git tag.  I guess the behavior of git send-email has been\nthe same from the start, but having it treat $EDITOR substantially\ndifferent from that of git commit and git tag seems like bug or at\nleast something that should be avoided.\n\nI'm not completely satisfied with the problem with embedded spaces,\nbut my Perl skills aren't good enough to do anything about it.  If\nanyone have any suggestions on how to do it, it would be greatly\nappreciated.  None-the-less, even with this shortcoming, I think this\nis a step in the right direction.\n\n git-send-email.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 248d035..47ae77c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -400,7 +400,7 @@ EOT\n \tclose(C);\n \n \tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\") || $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n-\tsystem($editor, $compose_filename);\n+\tsystem((split ' ', $editor), $compose_filename);\n \n \topen(C2,\">\",$compose_filename . \".final\")\n \t\tor die \"Failed to open $compose_filename.final : \" . $!;\n-- \n1.5.4.rc1.3.gc641f\n"},{"id":"63874","messageId":"20071220203211.GA12296@bit.office.eurotux.com","threadId":"11374","inReplyTo":"1198181646-28632-1-git-send-email-hendeby@isy.liu.se","subject":"Re: [PATCH] Make git send-email accept $EDITOR with arguments","fromName":"Luciano Rocha","fromEmail":"luciano@eurotux.com","sentAt":"2007-12-20T20:32:11Z","receivedAt":"2007-12-20T20:32:11Z","isPatch":true,"sender":{"key":"luciano@eurotux.com","avatar":null},"body":"On Thu, Dec 20, 2007 at 09:14:06PM +0100, Gustaf Hendeby wrote:\n<snip>\n> I'm not completely satisfied with the problem with embedded spaces,\n> but my Perl skills aren't good enough to do anything about it.  If\n> anyone have any suggestions on how to do it, it would be greatly\n> appreciated.  None-the-less, even with this shortcoming, I think this\n> is a step in the right direction.\n> \n>  git-send-email.perl |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 248d035..47ae77c 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -400,7 +400,7 @@ EOT\n>  \tclose(C);\n>  \n>  \tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\") || $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n> -\tsystem($editor, $compose_filename);\n> +\tsystem((split ' ', $editor), $compose_filename);\n\nThat should be enough. Use system(\"$editor $compose_filename\") to use\nperl's implicit split or, in case of meta-characters in the string,\nexternal sh -c.\n\nOr always use the shell:\n$shell = $ENV{SHELL} || \"/bin/sh\";\nsystem($shell, \"-c\", \"$editor $compose_filename\");\n\nBTW, maybe add a check for the return code?\nsystem(...) == 0 or die \"editor failed\\n\";\n\n-- \nLuciano Rocha <luciano@eurotux.com>\nEurotux Informática, S.A. <http://www.eurotux.com/>\n"},{"id":"63922","messageId":"1198237002-21470-1-git-send-email-hendeby@isy.liu.se","threadId":"11374","inReplyTo":"20071220203211.GA12296@bit.office.eurotux.com","subject":"[PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-21T11:36:42Z","receivedAt":"2007-12-21T11:36:42Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"Currently git send-email does not accept $EDITOR with arguments, eg,\nemacs -nw, when starting an editor to produce a cover letter.  This\nfix uses perl's implicit splitting to perform the task and that should\nhopefully cover most interesting cases.\n\nSigned-off-by:  Gustaf Hendeby <hendeby@isy.liu.se>\n---\n\nThanks to Luciano for the tip to use the internal splitting in perl,\nthat should be a better solution than to split on all spaces.  I don't\nthink it is necessary, though, to add an extra error message if the\nsystem call fails, system in it self already produces something that\nshould be clear enough.  If anyone got a strong oppinion for another\nerror message I'll fix that.\n\nJunio, even if this is technically not a bug fix, it would be nice to\nget this fix into the 1.5.4 so that the usage of $EDITOR becomes more\nconsistent throughout git.\n\n/Gustaf\n\n git-send-email.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 248d035..5764668 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -400,7 +400,7 @@ EOT\n \tclose(C);\n \n \tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\") || $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n-\tsystem($editor, $compose_filename);\n+\tsystem(\"$editor $compose_filename\");\n \n \topen(C2,\">\",$compose_filename . \".final\")\n \t\tor die \"Failed to open $compose_filename.final : \" . $!;\n-- \n1.5.4.rc1.4.gb8173-dirty\n"},{"id":"63925","messageId":"20071221133412.GA15198@sigill.intra.peff.net","threadId":"11374","inReplyTo":"1198237002-21470-1-git-send-email-hendeby@isy.liu.se","subject":"Re: [PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-21T13:34:12Z","receivedAt":"2007-12-21T13:34:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 21, 2007 at 12:36:42PM +0100, Gustaf Hendeby wrote:\n\n> -\tsystem($editor, $compose_filename);\n> +\tsystem(\"$editor $compose_filename\");\n\nIf you are going to do it that way, I suspect you want to quotemeta\n$compose_filename.\n\n-Peff\n"},{"id":"63927","messageId":"476BDA5F.4070306@isy.liu.se","threadId":"11374","inReplyTo":"20071221133412.GA15198@sigill.intra.peff.net","subject":"Re: [PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-21T15:23:11Z","receivedAt":"2007-12-21T15:23:11Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"On 12/21/2007 02:34 PM, Jeff King wrote:\n> On Fri, Dec 21, 2007 at 12:36:42PM +0100, Gustaf Hendeby wrote:\n> \n>> -\tsystem($editor, $compose_filename);\n>> +\tsystem(\"$editor $compose_filename\");\n> \n> If you are going to do it that way, I suspect you want to quotemeta\n> $compose_filename.\n> \n> -Peff\n\nGenerally that would be true, but is that really necessary when I know\n$compose_filename is defined as:\n\nmy $compose_filename = \".msg.$$\";\n\nOr, should I take it that you prefer the version using split?  I didn't\nreally feel good about the possibility of splitting paths with spaces\nthat came with that one though.\n\n/Gustaf\n"},{"id":"63934","messageId":"7vhcic9e17.fsf@gitster.siamese.dyndns.org","threadId":"11374","inReplyTo":"1198237002-21470-1-git-send-email-hendeby@isy.liu.se","subject":"Re: [PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-21T17:02:44Z","receivedAt":"2007-12-21T17:02:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gustaf Hendeby <hendeby@isy.liu.se> writes:\n\n> Junio, even if this is technically not a bug fix, it would be nice to\n> get this fix into the 1.5.4 so that the usage of $EDITOR becomes more\n> consistent throughout git.\n\nI can buy that, but at least a single line comment in front of that\nsystem() explaining why this is safe to do so would be beneficial.  I\nsuspect that somebody would propose moving $compse_filename inside\n$GIT_DIR, now people realized $compose_filename is currently \"./.msg.$$\",\nand $GIT_DIR could be anything.  Quotemeta would probably be better as the\ncode you are touching won't be affected by a future change to the value of\nthat constant defined far away in the source.\n"},{"id":"63945","messageId":"20071221192359.GA10660@coredump.intra.peff.net","threadId":"11374","inReplyTo":"476BDA5F.4070306@isy.liu.se","subject":"Re: [PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-21T19:23:59Z","receivedAt":"2007-12-21T19:23:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 21, 2007 at 04:23:11PM +0100, Gustaf Hendeby wrote:\n\n> > If you are going to do it that way, I suspect you want to quotemeta\n> > $compose_filename.\n> Generally that would be true, but is that really necessary when I know\n> $compose_filename is defined as:\n> \n> my $compose_filename = \".msg.$$\";\n\nI know; it is just easier to see that it is correct with the quotemeta\n(and correct in the face of somebody changing the message later).\n\n> Or, should I take it that you prefer the version using split?  I didn't\n> really feel good about the possibility of splitting paths with spaces\n> that came with that one though.\n\nI am fine with using the shell. Though keep in mind that the two\nsolutions will behave differently with\n\n  EDITOR='foo; bar'\n\nThat is, system(\"$editor $message\") will actually invoke the shell,\nwhereas system(split(/ /, $editor), $message) will _just_ split on\nwhitespace. We should do whatever is consistent with the rest of the git\ncommands (off the top of my head, I don't know).\n\n-Peff\n"},{"id":"63952","messageId":"476C2B2E.1080801@isy.liu.se","threadId":"11374","inReplyTo":"20071221192359.GA10660@coredump.intra.peff.net","subject":"Re: [PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-21T21:07:58Z","receivedAt":"2007-12-21T21:07:58Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"On 2007-12-21 20:23, Jeff King wrote:\n> On Fri, Dec 21, 2007 at 04:23:11PM +0100, Gustaf Hendeby wrote:\n> \n>>> If you are going to do it that way, I suspect you want to quotemeta\n>>> $compose_filename.\n>> Generally that would be true, but is that really necessary when I know\n>> $compose_filename is defined as:\n>>\n>> my $compose_filename = \".msg.$$\";\n> \n> I know; it is just easier to see that it is correct with the quotemeta\n> (and correct in the face of somebody changing the message later).\nPoint taken!\n\n> \n>> Or, should I take it that you prefer the version using split?  I didn't\n>> really feel good about the possibility of splitting paths with spaces\n>> that came with that one though.\n> \n> I am fine with using the shell. Though keep in mind that the two\n> solutions will behave differently with\n> \n>   EDITOR='foo; bar'\n> \n> That is, system(\"$editor $message\") will actually invoke the shell,\n> whereas system(split(/ /, $editor), $message) will _just_ split on\n> whitespace. We should do whatever is consistent with the rest of the git\n> commands (off the top of my head, I don't know).\n\nA quick look at the proposed solution to the similar problem with git\ncommit, using code now in git tag, it seems it uses a split like\nsolution, though taking \" and ' quoting into consideration.  On the top\nof my head I can't come up with any other commands using $EDITOR.  I'll\ntry to find some time the next couple of days to make a reasonable\nequivalent solution here.\n\nThanks,\n\tGustaf\n\n\nbegin:vcard\nfn:Gustaf Hendeby\nn:Hendeby;Gustaf\norg;quoted-printable:Link=C3=B6ping University;Department of Electrical Engineering\nadr;quoted-printable:;;;Link=C3=B6ping;;SE-581 83;Sweden\nemail;internet:hendeby@isy.liu.se\ntitle:PhD Student\ntel;work:+46 (0)13 282226\ntel;fax:+46 (0)13 282622\nx-mozilla-html:FALSE\nurl:http://www.control.isy.liu.se/~hendeby\nversion:2.1\nend:vcard\n\n"},{"id":"63953","messageId":"7vd4sz4uii.fsf@gitster.siamese.dyndns.org","threadId":"11374","inReplyTo":"476C2B2E.1080801@isy.liu.se","subject":"Re: [PATCH v2] Make git send-email accept $EDITOR with arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-21T21:17:57Z","receivedAt":"2007-12-21T21:17:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gustaf Hendeby <hendeby@isy.liu.se> writes:\n\n> On 2007-12-21 20:23, Jeff King wrote:\n> \n>> I am fine with using the shell. Though keep in mind that the two\n>> solutions will behave differently with\n>> \n>>   EDITOR='foo; bar'\n>> \n>> That is, system(\"$editor $message\") will actually invoke the shell,\n>> whereas system(split(/ /, $editor), $message) will _just_ split on\n>> whitespace. We should do whatever is consistent with the rest of the git\n>> commands (off the top of my head, I don't know).\n\nPersonally, I think EDITOR='foo; bar' is a user shooting his\nfoot off which we do not care about.\n\n> A quick look at the proposed solution to the similar problem with git\n> commit, using code now in git tag, it seems it uses a split like\n> solution, though taking \" and ' quoting into consideration.\n\nI think the easiest to read and compatible way is:\n\n\tsystem('sh', '-c', '$0 $@', $editor, $filename)\n"},{"id":"63972","messageId":"1198284052-20590-1-git-send-email-hendeby@isy.liu.se","threadId":"11374","inReplyTo":"7vd4sz4uii.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v3] Make git send-email accept $EDITOR with arguments","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-22T00:40:52Z","receivedAt":"2007-12-22T00:40:52Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"Currently git send-email does not accept $EDITOR with arguments, eg,\nemacs -nw, when starting an editor to produce a cover letter.  This\npatch changes this by letting the shell handle the option parsing.\n\nSigned-off-by:  Gustaf Hendeby <hendeby@isy.liu.se>\n---\n\nThis is based on Junio's suggestion on most readable and compatible\nsolution.  I'm not sure if it is identical to the C solution for git\ntag, but it seems to be a reasonable solution.\n\n git-send-email.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 248d035..e47994a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -400,7 +400,7 @@ EOT\n \tclose(C);\n \n \tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\") || $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n-\tsystem($editor, $compose_filename);\n+\tsystem('sh', '-c', '$0 $@', $editor, $compose_filename);\n \n \topen(C2,\">\",$compose_filename . \".final\")\n \t\tor die \"Failed to open $compose_filename.final : \" . $!;\n-- \n1.5.4.rc1.16.gc817f\n"},{"id":"63973","messageId":"1198284202-20666-1-git-send-email-hendeby@isy.liu.se","threadId":"11374","inReplyTo":"7vhcic9e17.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-22T00:43:22Z","receivedAt":"2007-12-22T00:43:22Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"git send-email with the --compose option now puts its temporary file\nin $GIT_DIR instead of the current working directory.  The file should\nbe removed on exit, still it is nice not to mess around in the working\ndirectory if not necessary.\n\nSigned-off-by: Gustaf Hendeby <hendeby@isy.liu.se>\n---\n git-send-email.perl |    8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex e47994a..c4eae9d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -150,9 +150,6 @@ my $auth;\n sub unique_email_list(@);\n sub cleanup_compose_files();\n \n-# Constants (essentially)\n-my $compose_filename = \".msg.$$\";\n-\n # Variables we fill in automatically, or via prompting:\n my (@to,@cc,@initial_cc,@bcclist,@xh,\n \t$initial_reply_to,$initial_subject,@files,$author,$sender,$compose,$time);\n@@ -170,6 +167,11 @@ if ($@) {\n \t$term = new FakeTerm \"$@: going non-interactive\";\n }\n \n+# Constants (essentially)\n+my $compose_filename = $repo->command(qw/rev-parse --git-dir/);\n+chomp($compose_filename);\n+$compose_filename =  $compose_filename . \"/.msg.$$\";\n+\n # Behavior modification variables\n my ($quiet, $dry_run) = (0, 0);\n \n-- \n1.5.4.rc1.16.gc817f\n"},{"id":"63975","messageId":"7vr6hf35fh.fsf@gitster.siamese.dyndns.org","threadId":"11374","inReplyTo":"1198284052-20590-1-git-send-email-hendeby@isy.liu.se","subject":"Re: [PATCH v3] Make git send-email accept $EDITOR with arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T01:05:06Z","receivedAt":"2007-12-22T01:05:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gustaf Hendeby <hendeby@isy.liu.se> writes:\n\n> Currently git send-email does not accept $EDITOR with arguments, eg,\n> emacs -nw, when starting an editor to produce a cover letter.  This\n> patch changes this by letting the shell handle the option parsing.\n>\n> Signed-off-by:  Gustaf Hendeby <hendeby@isy.liu.se>\n> ---\n>\n> This is based on Junio's suggestion on most readable and compatible\n> solution.  I'm not sure if it is identical to the C solution for git\n> tag, but it seems to be a reasonable solution.\n>\n>  git-send-email.perl |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 248d035..e47994a 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -400,7 +400,7 @@ EOT\n>  \tclose(C);\n>  \n>  \tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\") || $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n> -\tsystem($editor, $compose_filename);\n> +\tsystem('sh', '-c', '$0 $@', $editor, $compose_filename);\n>  \n>  \topen(C2,\">\",$compose_filename . \".final\")\n>  \t\tor die \"Failed to open $compose_filename.final : \" . $!;\n> -- \n> 1.5.4.rc1.16.gc817f\n\nThanks.  Has this been tested?  IOW, did you compose this\nmessage with this patch?\n"},{"id":"63976","messageId":"7vmys3358v.fsf@gitster.siamese.dyndns.org","threadId":"11374","inReplyTo":"1198284202-20666-1-git-send-email-hendeby@isy.liu.se","subject":"Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T01:09:04Z","receivedAt":"2007-12-22T01:09:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Don't you have $repo (an instance of Git) at that point?  You\nshould be able to ask repo_path() about it, shouldn't you?\n"},{"id":"63977","messageId":"ee77f5c20712211718g230802b6jb70e5db1f6a43973@mail.gmail.com","threadId":"11374","inReplyTo":"7vmys3358v.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2007-12-22T01:18:08Z","receivedAt":"2007-12-22T01:18:08Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Don't you have $repo (an instance of Git) at that point?  You\n> should be able to ask repo_path() about it, shouldn't you?\n\nIsn't git-send-email still useful outside a Git repo?\n\n\nDave.\n"},{"id":"63981","messageId":"7vhcib2phi.fsf@gitster.siamese.dyndns.org","threadId":"11374","inReplyTo":"ee77f5c20712211718g230802b6jb70e5db1f6a43973@mail.gmail.com","subject":"Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T06:49:29Z","receivedAt":"2007-12-22T06:49:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"David Symonds\" <dsymonds@gmail.com> writes:\n\n> On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Don't you have $repo (an instance of Git) at that point?  You\n>> should be able to ask repo_path() about it, shouldn't you?\n>\n> Isn't git-send-email still useful outside a Git repo?\n\nThen why does it run \"rev-parse --git-dir\"?\n"},{"id":"63982","messageId":"ee77f5c20712212304s598d344dg41d03f58084d794e@mail.gmail.com","threadId":"11374","inReplyTo":"7vhcib2phi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2007-12-22T07:04:03Z","receivedAt":"2007-12-22T07:04:03Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"David Symonds\" <dsymonds@gmail.com> writes:\n>\n> > On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> >> Don't you have $repo (an instance of Git) at that point?  You\n> >> should be able to ask repo_path() about it, shouldn't you?\n> >\n> > Isn't git-send-email still useful outside a Git repo?\n>\n> Then why does it run \"rev-parse --git-dir\"?\n\nI'm suggesting that it should still function just fine without being\ninside a repo, so it should adequately handle \"rev-parse --git-dir\"\nreturning 128.\n\n\nDave.\n"},{"id":"63987","messageId":"476CCF36.3010109@isy.liu.se","threadId":"11374","inReplyTo":"7vr6hf35fh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] Make git send-email accept $EDITOR with arguments","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-22T08:47:50Z","receivedAt":"2007-12-22T08:47:50Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"Junio C Hamano wrote:\n> Gustaf Hendeby <hendeby@isy.liu.se> writes:\n> \n>> Currently git send-email does not accept $EDITOR with arguments, eg,\n>> emacs -nw, when starting an editor to produce a cover letter.  This\n>> patch changes this by letting the shell handle the option parsing.\n>>\n>> Signed-off-by:  Gustaf Hendeby <hendeby@isy.liu.se>\n>> ---\n>>\n>> This is based on Junio's suggestion on most readable and compatible\n>> solution.  I'm not sure if it is identical to the C solution for git\n>> tag, but it seems to be a reasonable solution.\n>>\n>>  git-send-email.perl |    2 +-\n>>  1 files changed, 1 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 248d035..e47994a 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -400,7 +400,7 @@ EOT\n>>  \tclose(C);\n>>  \n>>  \tmy $editor = $ENV{GIT_EDITOR} || $repo->config(\"core.editor\") || $ENV{VISUAL} || $ENV{EDITOR} || \"vi\";\n>> -\tsystem($editor, $compose_filename);\n>> +\tsystem('sh', '-c', '$0 $@', $editor, $compose_filename);\n>>  \n>>  \topen(C2,\">\",$compose_filename . \".final\")\n>>  \t\tor die \"Failed to open $compose_filename.final : \" . $!;\n>> -- \n>> 1.5.4.rc1.16.gc817f\n> \n> Thanks.  Has this been tested?  IOW, did you compose this\n> message with this patch?\n\nYes, I sent out the patch with git send-email, with git including the \npatch itself and EDITOR=emacsclient -a emacs.  I have also played around \nwith it a bit trying to make sure it is valid, but no more rigorous \ntesting that that.  I'm not really sure how I would do that.\n\nThanks for all comments from everyone, I have learned a lot!\n\n/Gustaf\n"},{"id":"63988","messageId":"7vd4szyuva.fsf@gitster.siamese.dyndns.org","threadId":"11374","inReplyTo":"ee77f5c20712212304s598d344dg41d03f58084d794e@mail.gmail.com","subject":"Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T08:52:09Z","receivedAt":"2007-12-22T08:52:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"David Symonds\" <dsymonds@gmail.com> writes:\n\n> On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> \"David Symonds\" <dsymonds@gmail.com> writes:\n>>\n>> > On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> >> Don't you have $repo (an instance of Git) at that point?  You\n>> >> should be able to ask repo_path() about it, shouldn't you?\n>> >\n>> > Isn't git-send-email still useful outside a Git repo?\n>>\n>> Then why does it run \"rev-parse --git-dir\"?\n>\n> I'm suggesting that it should still function just fine without being\n> inside a repo, so it should adequately handle \"rev-parse --git-dir\"\n> returning 128.\n\nAh, true.  Then the current behaviour to use the $(pwd) for\ntemporary file area would be Ok for now.\n"},{"id":"63989","messageId":"476CD536.1060502@isy.liu.se","threadId":"11374","inReplyTo":"7vd4szyuva.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR","fromName":"Gustaf Hendeby","fromEmail":"hendeby@isy.liu.se","sentAt":"2007-12-22T09:13:26Z","receivedAt":"2007-12-22T09:13:26Z","isPatch":true,"sender":{"key":"hendeby@isy.liu.se","avatar":"https://avatars.githubusercontent.com/u/730316?v=4"},"body":"Junio C Hamano wrote:\n> \"David Symonds\" <dsymonds@gmail.com> writes:\n> \n>> On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> \"David Symonds\" <dsymonds@gmail.com> writes:\n>>>\n>>>> On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>>>> Don't you have $repo (an instance of Git) at that point?  You\n>>>>> should be able to ask repo_path() about it, shouldn't you?\n>>>> Isn't git-send-email still useful outside a Git repo?\n>>> Then why does it run \"rev-parse --git-dir\"?\n>> I'm suggesting that it should still function just fine without being\n>> inside a repo, so it should adequately handle \"rev-parse --git-dir\"\n>> returning 128.\n> \n> Ah, true.  Then the current behaviour to use the $(pwd) for\n> temporary file area would be Ok for now.\n\nOk, just drop the patch I don't feel strongly about it.\n\nHowever, the code today demands that git send-email is run from within a \ngit repository - it seems that $repo = Git->repository() assumes that. \nI'd suggest changing this behavior so that git send-email becomes \nrunnable from outside a git repository.  Unfortunately, I'm starting to \nget in above my head here, for one I really don't know the helper \nfunctions in Git.pm.  Is there any good place to read up on what is in \nGit.pm, except for the code itself?\n\n/Gustaf\n"}]}