* [PATCH] Make git send-email accept $EDITOR with arguments
@ 2007-12-20 20:14 Gustaf Hendeby
2007-12-20 20:32 ` Luciano Rocha
0 siblings, 1 reply; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-20 20:14 UTC (permalink / raw
To: git; +Cc: gitster, Gustaf Hendeby
Currently git send-email does not accept $EDITOR with arguments, eg,
emacs -nw, when starting an editor to produce a cover letter. This fixes
this in the simplest way possible, assume all spaces separates either
the command from the first argument, or two arguments. This should
work in most cases, but will break with quoted strings embedded spaces.
An example of a problematic case is when there is a space in the path
to the command.
Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---
This is related to the problems recently observed in the built in git
commit and git tag. I guess the behavior of git send-email has been
the same from the start, but having it treat $EDITOR substantially
different from that of git commit and git tag seems like bug or at
least something that should be avoided.
I'm not completely satisfied with the problem with embedded spaces,
but my Perl skills aren't good enough to do anything about it. If
anyone have any suggestions on how to do it, it would be greatly
appreciated. None-the-less, even with this shortcoming, I think this
is a step in the right direction.
git-send-email.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 248d035..47ae77c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -400,7 +400,7 @@ EOT
close(C);
my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
- system($editor, $compose_filename);
+ system((split ' ', $editor), $compose_filename);
open(C2,">",$compose_filename . ".final")
or die "Failed to open $compose_filename.final : " . $!;
--
1.5.4.rc1.3.gc641f
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] Make git send-email accept $EDITOR with arguments
2007-12-20 20:14 [PATCH] Make git send-email accept $EDITOR with arguments Gustaf Hendeby
@ 2007-12-20 20:32 ` Luciano Rocha
2007-12-21 11:36 ` [PATCH v2] " Gustaf Hendeby
0 siblings, 1 reply; 19+ messages in thread
From: Luciano Rocha @ 2007-12-20 20:32 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: git, gitster
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
On Thu, Dec 20, 2007 at 09:14:06PM +0100, Gustaf Hendeby wrote:
<snip>
> I'm not completely satisfied with the problem with embedded spaces,
> but my Perl skills aren't good enough to do anything about it. If
> anyone have any suggestions on how to do it, it would be greatly
> appreciated. None-the-less, even with this shortcoming, I think this
> is a step in the right direction.
>
> git-send-email.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 248d035..47ae77c 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -400,7 +400,7 @@ EOT
> close(C);
>
> my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
> - system($editor, $compose_filename);
> + system((split ' ', $editor), $compose_filename);
That should be enough. Use system("$editor $compose_filename") to use
perl's implicit split or, in case of meta-characters in the string,
external sh -c.
Or always use the shell:
$shell = $ENV{SHELL} || "/bin/sh";
system($shell, "-c", "$editor $compose_filename");
BTW, maybe add a check for the return code?
system(...) == 0 or die "editor failed\n";
--
Luciano Rocha <luciano@eurotux.com>
Eurotux Informática, S.A. <http://www.eurotux.com/>
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-20 20:32 ` Luciano Rocha
@ 2007-12-21 11:36 ` Gustaf Hendeby
2007-12-21 13:34 ` Jeff King
2007-12-21 17:02 ` [PATCH v2] " Junio C Hamano
0 siblings, 2 replies; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-21 11:36 UTC (permalink / raw
To: luciano; +Cc: git, gitster, Gustaf Hendeby
Currently git send-email does not accept $EDITOR with arguments, eg,
emacs -nw, when starting an editor to produce a cover letter. This
fix uses perl's implicit splitting to perform the task and that should
hopefully cover most interesting cases.
Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---
Thanks to Luciano for the tip to use the internal splitting in perl,
that should be a better solution than to split on all spaces. I don't
think it is necessary, though, to add an extra error message if the
system call fails, system in it self already produces something that
should be clear enough. If anyone got a strong oppinion for another
error message I'll fix that.
Junio, even if this is technically not a bug fix, it would be nice to
get this fix into the 1.5.4 so that the usage of $EDITOR becomes more
consistent throughout git.
/Gustaf
git-send-email.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 248d035..5764668 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -400,7 +400,7 @@ EOT
close(C);
my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
- system($editor, $compose_filename);
+ system("$editor $compose_filename");
open(C2,">",$compose_filename . ".final")
or die "Failed to open $compose_filename.final : " . $!;
--
1.5.4.rc1.4.gb8173-dirty
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-21 11:36 ` [PATCH v2] " Gustaf Hendeby
@ 2007-12-21 13:34 ` Jeff King
2007-12-21 15:23 ` Gustaf Hendeby
2007-12-21 17:02 ` [PATCH v2] " Junio C Hamano
1 sibling, 1 reply; 19+ messages in thread
From: Jeff King @ 2007-12-21 13:34 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: luciano, git, gitster
On Fri, Dec 21, 2007 at 12:36:42PM +0100, Gustaf Hendeby wrote:
> - system($editor, $compose_filename);
> + system("$editor $compose_filename");
If you are going to do it that way, I suspect you want to quotemeta
$compose_filename.
-Peff
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-21 13:34 ` Jeff King
@ 2007-12-21 15:23 ` Gustaf Hendeby
2007-12-21 19:23 ` Jeff King
0 siblings, 1 reply; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-21 15:23 UTC (permalink / raw
To: Jeff King; +Cc: luciano, git, gitster
On 12/21/2007 02:34 PM, Jeff King wrote:
> On Fri, Dec 21, 2007 at 12:36:42PM +0100, Gustaf Hendeby wrote:
>
>> - system($editor, $compose_filename);
>> + system("$editor $compose_filename");
>
> If you are going to do it that way, I suspect you want to quotemeta
> $compose_filename.
>
> -Peff
Generally that would be true, but is that really necessary when I know
$compose_filename is defined as:
my $compose_filename = ".msg.$$";
Or, should I take it that you prefer the version using split? I didn't
really feel good about the possibility of splitting paths with spaces
that came with that one though.
/Gustaf
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-21 11:36 ` [PATCH v2] " Gustaf Hendeby
2007-12-21 13:34 ` Jeff King
@ 2007-12-21 17:02 ` Junio C Hamano
2007-12-22 0:43 ` [PATCH] Move git send-email cover letter temporary file to $GIT_DIR Gustaf Hendeby
1 sibling, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2007-12-21 17:02 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: luciano, git, gitster
Gustaf Hendeby <hendeby@isy.liu.se> writes:
> Junio, even if this is technically not a bug fix, it would be nice to
> get this fix into the 1.5.4 so that the usage of $EDITOR becomes more
> consistent throughout git.
I can buy that, but at least a single line comment in front of that
system() explaining why this is safe to do so would be beneficial. I
suspect that somebody would propose moving $compse_filename inside
$GIT_DIR, now people realized $compose_filename is currently "./.msg.$$",
and $GIT_DIR could be anything. Quotemeta would probably be better as the
code you are touching won't be affected by a future change to the value of
that constant defined far away in the source.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-21 15:23 ` Gustaf Hendeby
@ 2007-12-21 19:23 ` Jeff King
2007-12-21 21:07 ` Gustaf Hendeby
0 siblings, 1 reply; 19+ messages in thread
From: Jeff King @ 2007-12-21 19:23 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: luciano, git, gitster
On Fri, Dec 21, 2007 at 04:23:11PM +0100, Gustaf Hendeby wrote:
> > If you are going to do it that way, I suspect you want to quotemeta
> > $compose_filename.
> Generally that would be true, but is that really necessary when I know
> $compose_filename is defined as:
>
> my $compose_filename = ".msg.$$";
I know; it is just easier to see that it is correct with the quotemeta
(and correct in the face of somebody changing the message later).
> Or, should I take it that you prefer the version using split? I didn't
> really feel good about the possibility of splitting paths with spaces
> that came with that one though.
I am fine with using the shell. Though keep in mind that the two
solutions will behave differently with
EDITOR='foo; bar'
That is, system("$editor $message") will actually invoke the shell,
whereas system(split(/ /, $editor), $message) will _just_ split on
whitespace. We should do whatever is consistent with the rest of the git
commands (off the top of my head, I don't know).
-Peff
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-21 19:23 ` Jeff King
@ 2007-12-21 21:07 ` Gustaf Hendeby
2007-12-21 21:17 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-21 21:07 UTC (permalink / raw
To: Jeff King; +Cc: luciano, git, gitster
[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]
On 2007-12-21 20:23, Jeff King wrote:
> On Fri, Dec 21, 2007 at 04:23:11PM +0100, Gustaf Hendeby wrote:
>
>>> If you are going to do it that way, I suspect you want to quotemeta
>>> $compose_filename.
>> Generally that would be true, but is that really necessary when I know
>> $compose_filename is defined as:
>>
>> my $compose_filename = ".msg.$$";
>
> I know; it is just easier to see that it is correct with the quotemeta
> (and correct in the face of somebody changing the message later).
Point taken!
>
>> Or, should I take it that you prefer the version using split? I didn't
>> really feel good about the possibility of splitting paths with spaces
>> that came with that one though.
>
> I am fine with using the shell. Though keep in mind that the two
> solutions will behave differently with
>
> EDITOR='foo; bar'
>
> That is, system("$editor $message") will actually invoke the shell,
> whereas system(split(/ /, $editor), $message) will _just_ split on
> whitespace. We should do whatever is consistent with the rest of the git
> commands (off the top of my head, I don't know).
A quick look at the proposed solution to the similar problem with git
commit, using code now in git tag, it seems it uses a split like
solution, though taking " and ' quoting into consideration. On the top
of my head I can't come up with any other commands using $EDITOR. I'll
try to find some time the next couple of days to make a reasonable
equivalent solution here.
Thanks,
Gustaf
[-- Attachment #2: hendeby.vcf --]
[-- Type: text/x-vcard, Size: 378 bytes --]
begin:vcard
fn:Gustaf Hendeby
n:Hendeby;Gustaf
org;quoted-printable:Link=C3=B6ping University;Department of Electrical Engineering
adr;quoted-printable:;;;Link=C3=B6ping;;SE-581 83;Sweden
email;internet:hendeby@isy.liu.se
title:PhD Student
tel;work:+46 (0)13 282226
tel;fax:+46 (0)13 282622
x-mozilla-html:FALSE
url:http://www.control.isy.liu.se/~hendeby
version:2.1
end:vcard
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] Make git send-email accept $EDITOR with arguments
2007-12-21 21:07 ` Gustaf Hendeby
@ 2007-12-21 21:17 ` Junio C Hamano
2007-12-22 0:40 ` [PATCH v3] " Gustaf Hendeby
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2007-12-21 21:17 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: Jeff King, luciano, git, gitster
Gustaf Hendeby <hendeby@isy.liu.se> writes:
> On 2007-12-21 20:23, Jeff King wrote:
>
>> I am fine with using the shell. Though keep in mind that the two
>> solutions will behave differently with
>>
>> EDITOR='foo; bar'
>>
>> That is, system("$editor $message") will actually invoke the shell,
>> whereas system(split(/ /, $editor), $message) will _just_ split on
>> whitespace. We should do whatever is consistent with the rest of the git
>> commands (off the top of my head, I don't know).
Personally, I think EDITOR='foo; bar' is a user shooting his
foot off which we do not care about.
> A quick look at the proposed solution to the similar problem with git
> commit, using code now in git tag, it seems it uses a split like
> solution, though taking " and ' quoting into consideration.
I think the easiest to read and compatible way is:
system('sh', '-c', '$0 $@', $editor, $filename)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3] Make git send-email accept $EDITOR with arguments
2007-12-21 21:17 ` Junio C Hamano
@ 2007-12-22 0:40 ` Gustaf Hendeby
2007-12-22 1:05 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-22 0:40 UTC (permalink / raw
To: gitster; +Cc: peff, luciano, git, Gustaf Hendeby
Currently git send-email does not accept $EDITOR with arguments, eg,
emacs -nw, when starting an editor to produce a cover letter. This
patch changes this by letting the shell handle the option parsing.
Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---
This is based on Junio's suggestion on most readable and compatible
solution. I'm not sure if it is identical to the C solution for git
tag, but it seems to be a reasonable solution.
git-send-email.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 248d035..e47994a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -400,7 +400,7 @@ EOT
close(C);
my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
- system($editor, $compose_filename);
+ system('sh', '-c', '$0 $@', $editor, $compose_filename);
open(C2,">",$compose_filename . ".final")
or die "Failed to open $compose_filename.final : " . $!;
--
1.5.4.rc1.16.gc817f
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-21 17:02 ` [PATCH v2] " Junio C Hamano
@ 2007-12-22 0:43 ` Gustaf Hendeby
2007-12-22 1:09 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-22 0:43 UTC (permalink / raw
To: gitster; +Cc: luciano, git, Gustaf Hendeby
git send-email with the --compose option now puts its temporary file
in $GIT_DIR instead of the current working directory. The file should
be removed on exit, still it is nice not to mess around in the working
directory if not necessary.
Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---
git-send-email.perl | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index e47994a..c4eae9d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -150,9 +150,6 @@ my $auth;
sub unique_email_list(@);
sub cleanup_compose_files();
-# Constants (essentially)
-my $compose_filename = ".msg.$$";
-
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@initial_cc,@bcclist,@xh,
$initial_reply_to,$initial_subject,@files,$author,$sender,$compose,$time);
@@ -170,6 +167,11 @@ if ($@) {
$term = new FakeTerm "$@: going non-interactive";
}
+# Constants (essentially)
+my $compose_filename = $repo->command(qw/rev-parse --git-dir/);
+chomp($compose_filename);
+$compose_filename = $compose_filename . "/.msg.$$";
+
# Behavior modification variables
my ($quiet, $dry_run) = (0, 0);
--
1.5.4.rc1.16.gc817f
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Make git send-email accept $EDITOR with arguments
2007-12-22 0:40 ` [PATCH v3] " Gustaf Hendeby
@ 2007-12-22 1:05 ` Junio C Hamano
2007-12-22 8:47 ` Gustaf Hendeby
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2007-12-22 1:05 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: peff, luciano, git
Gustaf Hendeby <hendeby@isy.liu.se> writes:
> Currently git send-email does not accept $EDITOR with arguments, eg,
> emacs -nw, when starting an editor to produce a cover letter. This
> patch changes this by letting the shell handle the option parsing.
>
> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
> ---
>
> This is based on Junio's suggestion on most readable and compatible
> solution. I'm not sure if it is identical to the C solution for git
> tag, but it seems to be a reasonable solution.
>
> git-send-email.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 248d035..e47994a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -400,7 +400,7 @@ EOT
> close(C);
>
> my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
> - system($editor, $compose_filename);
> + system('sh', '-c', '$0 $@', $editor, $compose_filename);
>
> open(C2,">",$compose_filename . ".final")
> or die "Failed to open $compose_filename.final : " . $!;
> --
> 1.5.4.rc1.16.gc817f
Thanks. Has this been tested? IOW, did you compose this
message with this patch?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-22 0:43 ` [PATCH] Move git send-email cover letter temporary file to $GIT_DIR Gustaf Hendeby
@ 2007-12-22 1:09 ` Junio C Hamano
2007-12-22 1:18 ` David Symonds
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2007-12-22 1:09 UTC (permalink / raw
To: Gustaf Hendeby; +Cc: luciano, git
Don't you have $repo (an instance of Git) at that point? You
should be able to ask repo_path() about it, shouldn't you?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-22 1:09 ` Junio C Hamano
@ 2007-12-22 1:18 ` David Symonds
2007-12-22 6:49 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: David Symonds @ 2007-12-22 1:18 UTC (permalink / raw
To: Junio C Hamano; +Cc: Gustaf Hendeby, luciano, git
On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Don't you have $repo (an instance of Git) at that point? You
> should be able to ask repo_path() about it, shouldn't you?
Isn't git-send-email still useful outside a Git repo?
Dave.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-22 1:18 ` David Symonds
@ 2007-12-22 6:49 ` Junio C Hamano
2007-12-22 7:04 ` David Symonds
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2007-12-22 6:49 UTC (permalink / raw
To: David Symonds; +Cc: Junio C Hamano, Gustaf Hendeby, luciano, git
"David Symonds" <dsymonds@gmail.com> writes:
> On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Don't you have $repo (an instance of Git) at that point? You
>> should be able to ask repo_path() about it, shouldn't you?
>
> Isn't git-send-email still useful outside a Git repo?
Then why does it run "rev-parse --git-dir"?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-22 6:49 ` Junio C Hamano
@ 2007-12-22 7:04 ` David Symonds
2007-12-22 8:52 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: David Symonds @ 2007-12-22 7:04 UTC (permalink / raw
To: Junio C Hamano; +Cc: Gustaf Hendeby, luciano, git
On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> "David Symonds" <dsymonds@gmail.com> writes:
>
> > On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Don't you have $repo (an instance of Git) at that point? You
> >> should be able to ask repo_path() about it, shouldn't you?
> >
> > Isn't git-send-email still useful outside a Git repo?
>
> Then why does it run "rev-parse --git-dir"?
I'm suggesting that it should still function just fine without being
inside a repo, so it should adequately handle "rev-parse --git-dir"
returning 128.
Dave.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Make git send-email accept $EDITOR with arguments
2007-12-22 1:05 ` Junio C Hamano
@ 2007-12-22 8:47 ` Gustaf Hendeby
0 siblings, 0 replies; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-22 8:47 UTC (permalink / raw
To: Junio C Hamano; +Cc: peff, luciano, git
Junio C Hamano wrote:
> Gustaf Hendeby <hendeby@isy.liu.se> writes:
>
>> Currently git send-email does not accept $EDITOR with arguments, eg,
>> emacs -nw, when starting an editor to produce a cover letter. This
>> patch changes this by letting the shell handle the option parsing.
>>
>> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
>> ---
>>
>> This is based on Junio's suggestion on most readable and compatible
>> solution. I'm not sure if it is identical to the C solution for git
>> tag, but it seems to be a reasonable solution.
>>
>> git-send-email.perl | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 248d035..e47994a 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -400,7 +400,7 @@ EOT
>> close(C);
>>
>> my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
>> - system($editor, $compose_filename);
>> + system('sh', '-c', '$0 $@', $editor, $compose_filename);
>>
>> open(C2,">",$compose_filename . ".final")
>> or die "Failed to open $compose_filename.final : " . $!;
>> --
>> 1.5.4.rc1.16.gc817f
>
> Thanks. Has this been tested? IOW, did you compose this
> message with this patch?
Yes, I sent out the patch with git send-email, with git including the
patch itself and EDITOR=emacsclient -a emacs. I have also played around
with it a bit trying to make sure it is valid, but no more rigorous
testing that that. I'm not really sure how I would do that.
Thanks for all comments from everyone, I have learned a lot!
/Gustaf
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-22 7:04 ` David Symonds
@ 2007-12-22 8:52 ` Junio C Hamano
2007-12-22 9:13 ` Gustaf Hendeby
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2007-12-22 8:52 UTC (permalink / raw
To: David Symonds; +Cc: Junio C Hamano, Gustaf Hendeby, luciano, git
"David Symonds" <dsymonds@gmail.com> writes:
> On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "David Symonds" <dsymonds@gmail.com> writes:
>>
>> > On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> Don't you have $repo (an instance of Git) at that point? You
>> >> should be able to ask repo_path() about it, shouldn't you?
>> >
>> > Isn't git-send-email still useful outside a Git repo?
>>
>> Then why does it run "rev-parse --git-dir"?
>
> I'm suggesting that it should still function just fine without being
> inside a repo, so it should adequately handle "rev-parse --git-dir"
> returning 128.
Ah, true. Then the current behaviour to use the $(pwd) for
temporary file area would be Ok for now.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Move git send-email cover letter temporary file to $GIT_DIR
2007-12-22 8:52 ` Junio C Hamano
@ 2007-12-22 9:13 ` Gustaf Hendeby
0 siblings, 0 replies; 19+ messages in thread
From: Gustaf Hendeby @ 2007-12-22 9:13 UTC (permalink / raw
To: Junio C Hamano; +Cc: David Symonds, luciano, git
Junio C Hamano wrote:
> "David Symonds" <dsymonds@gmail.com> writes:
>
>> On Dec 22, 2007 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> "David Symonds" <dsymonds@gmail.com> writes:
>>>
>>>> On Dec 22, 2007 12:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Don't you have $repo (an instance of Git) at that point? You
>>>>> should be able to ask repo_path() about it, shouldn't you?
>>>> Isn't git-send-email still useful outside a Git repo?
>>> Then why does it run "rev-parse --git-dir"?
>> I'm suggesting that it should still function just fine without being
>> inside a repo, so it should adequately handle "rev-parse --git-dir"
>> returning 128.
>
> Ah, true. Then the current behaviour to use the $(pwd) for
> temporary file area would be Ok for now.
Ok, just drop the patch I don't feel strongly about it.
However, the code today demands that git send-email is run from within a
git repository - it seems that $repo = Git->repository() assumes that.
I'd suggest changing this behavior so that git send-email becomes
runnable from outside a git repository. Unfortunately, I'm starting to
get in above my head here, for one I really don't know the helper
functions in Git.pm. Is there any good place to read up on what is in
Git.pm, except for the code itself?
/Gustaf
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2007-12-22 9:14 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-20 20:14 [PATCH] Make git send-email accept $EDITOR with arguments Gustaf Hendeby
2007-12-20 20:32 ` Luciano Rocha
2007-12-21 11:36 ` [PATCH v2] " Gustaf Hendeby
2007-12-21 13:34 ` Jeff King
2007-12-21 15:23 ` Gustaf Hendeby
2007-12-21 19:23 ` Jeff King
2007-12-21 21:07 ` Gustaf Hendeby
2007-12-21 21:17 ` Junio C Hamano
2007-12-22 0:40 ` [PATCH v3] " Gustaf Hendeby
2007-12-22 1:05 ` Junio C Hamano
2007-12-22 8:47 ` Gustaf Hendeby
2007-12-21 17:02 ` [PATCH v2] " Junio C Hamano
2007-12-22 0:43 ` [PATCH] Move git send-email cover letter temporary file to $GIT_DIR Gustaf Hendeby
2007-12-22 1:09 ` Junio C Hamano
2007-12-22 1:18 ` David Symonds
2007-12-22 6:49 ` Junio C Hamano
2007-12-22 7:04 ` David Symonds
2007-12-22 8:52 ` Junio C Hamano
2007-12-22 9:13 ` Gustaf Hendeby
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).