RE: [PATCH] send-email: do not check for editor until needed
- From
Peter Kjellerstedt <peter.kjellerstedt@axis.com>
- Date
- Mar 23, 2010, 09:15 UTC
- Message-ID
- <A612847CFE53224C91B23E3A5B48BAC73EC0B01EE8@xmail3.se.axis.com>
- In-Reply-To
- <20100322232531.GA2168@progeny.tock>
Show 65 quoted lines
> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Jonathan Nieder
> Sent: den 23 mars 2010 00:26
> To: Uwe Kleine-König
> Cc: git@vger.kernel.org; Junio C Hamano; Marc Kleine-Budde; Jay Soffian
> Subject: [PATCH] send-email: do not check for editor until needed
>
> Since b4479f074 (add -i, send-email, svn, p4, etc: use "git var
> GIT_EDITOR", 2009-10-30), when called with TERM=dumb and without
> GIT_EDITOR set, git send-email has been failing whether it needs
> an editor or not:
>
> $ ssh localhost git send-email --to=me --suppress-cc=all HEAD^..HEAD
> fatal: Terminal is dumb, but EDITOR unset
> var GIT_EDITOR: command returned error: 128
>
> This breaks use of git send-email in existing hook scripts.
>
> So do not check for an editor unless it is needed.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Uwe Kleine-König wrote:
>
> > since b4479f074760a788dd4e353b8c86a7d735afc53e git send-email (and
> > others) use git var GIT_EDITOR. This is OK as such but it breaks the
> > post-receive hooks that I use on several repositories.
> [...]
> > IMHO git send-email should only call $(git var GIT_EDITOR) when it
> > actually needs it.
>
> Thanks for reporting. Does this patch work for you?
>
> Now if I try without a tty, I get a different error:
>
> $ ssh localhost cd $(pwd) '&&' \
> git send-email --to=me --suppress-cc=all HEAD^..HEAD
> Can't locate object method "IN" via package "FakeTerm" at
> /home/jrn/tmp-git/libexec/git-core/git-send-email line 645.
> /tmp/olTiwjzrjx/0001-Git-1.7.0.3.patch
>
> I assume I am not using it correctly, since the relevant code has
> been around for a while.
>
> git-send-email.perl | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index e05455f..9406cdd 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -162,9 +162,16 @@ my $compose_filename;
>
> # Handle interactive edition of files.
> my $multiedit;
> -my $editor = Git::command_oneline('var', 'GIT_EDITOR');
>
> sub do_edit {
> + my $editor;
> +
> + if ($#_ == 0) {
> + return;
> + }Shouldn't that be:
if ($#_ == -1) {
return;
}or more readable:
return if (!@_);
as I assume you are trying to protect do_edit() from being called without arguments?
Show 13 quoted lines
> + git_cmd_try {
> + $editor = Git::command_oneline('var', 'GIT_EDITOR');
> + } "no suitable text editor configured\n";
> if (defined($multiedit) && !$multiedit) {
> map {
> system('sh', '-c', $editor.' "$@"', $editor, $_);
> --
> 1.7.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html//Peter