Re: [PATCH] send-email: lazily assign editor variable

Michael J Gruber [off-list ref] writes:

b4479f0 (add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR",
2009-10-30) introduced the use of git var GIT_EDITOR which may lead to
problems when send-mail is used without a tty.

We would want to describe what kind of problems they are better than "may
lead to problems", though.  Something like this?

    b4479f0 (add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR",
    2009-10-30) introduced the use of "git var GIT_EDITOR" to obtain the
    preferred editor program, instead of reading environment variables
    themselves.

    However, "git var GIT_EDITOR" run without a tty (think "cron job") would
    give a fatal error "Terminal is dumb, but EDITOR unset".  This is not a
    problem for add-i, svn, p4 and callers of git_editor() defined in
    git-sh-setup, as all of these call it just before launching the editor.
    At that point, we know the caller wants to edit, and they cannot without a
    tty.

    But send-email ran this near the beginning of the program, even if it is
    not going to use any editor (e.g. run without --compose).  Fix this by
    calling the command only when we edit a file.

Junio C Hamano wrote:

We would want to describe what kind of problems they are better than "may
lead to problems", though.  Something like this?

    b4479f0 (add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR",
    2009-10-30) introduced the use of "git var GIT_EDITOR" to obtain the
    preferred editor program, instead of reading environment variables
    themselves.

    However, "git var GIT_EDITOR" run without a tty (think "cron job") would
    give a fatal error "Terminal is dumb, but EDITOR unset".  This is not a
    problem for add-i, svn, p4 and callers of git_editor() defined in
    git-sh-setup, as all of these call it just before launching the editor.
    At that point, we know the caller wants to edit, and they cannot without a
    tty.

    But send-email ran this near the beginning of the program, even if it is
    not going to use any editor (e.g. run without --compose).  Fix this by
    calling the command only when we edit a file.

Yes, please.  It would be more precise to

  s/without a tty/with TERM=dumb/

in the second paragraph but regardless this is a good description of the
problem.

Junio C Hamano venit, vidit, dixit 24.03.2010 18:52:

Michael J Gruber [off-list ref] writes:

quoted

b4479f0 (add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR",
2009-10-30) introduced the use of git var GIT_EDITOR which may lead to
problems when send-mail is used without a tty.

We would want to describe what kind of problems they are better than "may
lead to problems", though.  Something like this?

    b4479f0 (add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR",
    2009-10-30) introduced the use of "git var GIT_EDITOR" to obtain the
    preferred editor program, instead of reading environment variables
    themselves.

    However, "git var GIT_EDITOR" run without a tty (think "cron job") would

Also think "hook" ;)

    give a fatal error "Terminal is dumb, but EDITOR unset".  This is not a
    problem for add-i, svn, p4 and callers of git_editor() defined in
    git-sh-setup, as all of these call it just before launching the editor.
    At that point, we know the caller wants to edit, and they cannot without a
    tty.

    But send-email ran this near the beginning of the program, even if it is
    not going to use any editor (e.g. run without --compose).  Fix this by
    calling the command only when we edit a file.

Yep, I'm fine with this.

Cheers,
Michael

Michael J Gruber [off-list ref] writes:

quoted

    However, "git var GIT_EDITOR" run without a tty (think "cron job") would

Also think "hook" ;)

Yes, remote hooks invoked over ssh transport won't usually have a tty, but
the local hooks (e.g. post-commit) usually do, I think.

Yep, I'm fine with this.

Thanks.

Jonathan Nieder [off-list ref] writes:

Yes, please.  It would be more precise to

  s/without a tty/with TERM=dumb/

in the second paragraph but regardless this is a good description of the
problem.

Strictly speaking, with TERM=dumb the true "vi" fell back to the ex mode
and should have been usable.  A purist might say it is a bug that we fatal
out in this case (I know that it is deliberately done to help newbies from
common confusion by not running any editor with TERM=dumb, but to a purist
in me it feels somewhat wrong).

Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

Yes, please.  It would be more precise to

  s/without a tty/with TERM=dumb/

in the second paragraph but regardless this is a good description of the
problem.

Strictly speaking, with TERM=dumb the true "vi" fell back to the ex mode
and should have been usable.  A purist might say it is a bug that we fatal
out in this case (I know that it is deliberately done to help newbies from
common confusion by not running any editor with TERM=dumb, but to a purist
in me it feels somewhat wrong).

With DISPLAY set, running “TERM=dumb vi | cat” with an appropriate
symlink in $PATH:

vim
	Prints “Vim: Warning: Output is not to a terminal” to stderr,
	waits about a second, then
	runs without clearing the screen, using ANSI escapes to move around.

elvis 1.4
	Prints “This termcap entry lacks the :up=: capability” to stderr,
	fails with status 1.

elvis 2 (more precisely, the elvisnox script from Debian)
	Prints “termcap needs up” to stderr,
	fails with status 1.

nvi
	Prints “ex/vi: Vi's standard input and output must be a terminal” to stderr,
	fails with status 1.

With TERM=dumb but standard output going straight to the terminal, the
situation is not very different for vim and elvis, while nvi gets
utterly confused (it only writes output to the bottom line for some
reason).

So I think it still makes sense to work around this bug in the vi clones.

HTH,
Jonathan