From: Junio C Hamano <hidden> Date: 2016-06-15 22:47:37
Ben Walton [off-list ref] writes:
On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which forces
users to alter their environment (PATH, GIT_PAGER, LESS, etc) or have
a local or global gitconfig before paging works as expected.
Would it be completely out of line to provide a knob so that the
fallback $pager could be set to something more specific/appropriate
during the build?
I think that is a sensible thing to do. Something like this?
Makefile | 6 ++++++
pager.c | 6 +++++-
2 files changed, 11 insertions(+), 1 deletions(-)
@@ -200,6 +200,9 @@ all::# memory allocators with the nedmalloc allocator written by Niall Douglas.## Define NO_REGEX if you have no or inferior regex support in your C library.+#+# Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if+# you want to use something different.GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Junio C Hamano wrote:
Ben Walton [off-list ref] writes:
quoted
Would it be completely out of line to provide a knob so that the
fallback $pager could be set to something more specific/appropriate
during the build?
I think that is a sensible thing to do. Something like this?
Makefile | 6 ++++++
pager.c | 6 +++++-
2 files changed, 11 insertions(+), 1 deletions(-)
That looks good to me, but it’s missing a corresponding change to
git-svn. I think something like this could be useful for Debian to
avoid patching to comply with the distro policy of falling back to
generic "pager" and "editor" symlinks. How about the following two
patches?
Jonathan Nieder (1):
Provide a build time default-editor setting
Junio C Hamano (1):
Provide a build time default-pager setting
Makefile | 18 ++++++++++++++++++
editor.c | 2 +-
git-add--interactive.perl | 3 ++-
git-sh-setup.sh | 6 ++++--
git-svn.perl | 8 +++++---
pager.c | 2 +-
t/Makefile | 2 ++
t/t7005-editor.sh | 29 ++++++++++++++++++++++-------
8 files changed, 55 insertions(+), 15 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
From: Junio C Hamano <redacted>
On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which
forces users to alter their environment (PATH, GIT_PAGER, LESS,
etc) or have a local or global gitconfig before paging works as
expected.
On Debian systems, by policy packages must fall back to the
'pager' command, so that changing the target of the
/usr/bin/pager symlink changes the default pager for all packages
at once.
Provide a DEFAULT_PAGER knob so packagers can set the fallback
pager to something appropriate during the build.
This puts the "less" default in the Makefile instead of pager.c,
since it is needed for git-svn, too. This means that the
DEFAULT_PAGER preprocessor token _has_ to be defined on the
command line for git to build. I was worried about this for a
moment, but GIT_VERSION already works this way without trouble.
Probably the DEFAULT_PAGER setting should be added to something
like TRACK_CFLAGS as well. Actually, some other settings that
can change without forcing files to be rebuilt (e.g. SHELL_PATH),
too. This should be probably be addressed separately.
Reported-by: Ben Walton <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Makefile | 8 ++++++++
git-svn.perl | 5 +++--
pager.c | 2 +-
3 files changed, 12 insertions(+), 3 deletions(-)
@@ -200,6 +200,9 @@ all::# memory allocators with the nedmalloc allocator written by Niall Douglas.## Define NO_REGEX if you have no or inferior regex support in your C library.+#+# Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if+# you want to use something different.GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
@@ -3,11 +3,12 @@# License: GPL v2 or laterusewarnings;usestrict;-usevarsqw/$AUTHOR$VERSION+usevarsqw/$AUTHOR$VERSION$DEFAULT_PAGER$sha1$sha1_short$_revision$_repository$_q$_authors$_authors_prog%users/;$AUTHOR='Eric Wong <normalperson@yhbt.net>';$VERSION='@@GIT_VERSION@@';+$DEFAULT_PAGER='@@DEFAULT_PAGER@@';# From which subdir have we been invoked?my$cmd_dir_prefix=eval{
@@ -5174,7 +5175,7 @@ sub git_svn_log_cmd {subconfig_pager{$pager||=$ENV{GIT_PAGER}||$ENV{PAGER};if(!defined$pager){-$pager='less';+$pager=$DEFAULT_PAGER;}elsif(length$pager==0||$pagereq'cat'){$pager=undef;}
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Provide a DEFAULT_EDITOR knob to allow the fallback editor (to
use instead of vi if VISUAL, EDITOR, and GIT_EDITOR are unset) to
be set at build time according to a system’s policy. For
example, on Debian systems, the default editor should be the
'editor' command.
The contrib/fast-import/git-p4 script still uses vi, since it is
not modified by the Makefile currently, and making it require
build-time modification would create too much trouble for people
deploying that script.
This change makes t7005-editor into a mess. Any ideas for fixing
this?
Signed-off-by: Jonathan Nieder <redacted>
---
Makefile | 10 ++++++++++
editor.c | 2 +-
git-add--interactive.perl | 3 ++-
git-sh-setup.sh | 6 ++++--
git-svn.perl | 5 +++--
t/Makefile | 2 ++
t/t7005-editor.sh | 29 ++++++++++++++++++++++-------
7 files changed, 44 insertions(+), 13 deletions(-)
@@ -203,6 +203,9 @@ all::## Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if# you want to use something different.+#+# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you+# want to use something different.GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
@@ -19,7 +19,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *enreturnerror("Terminal is dumb but no VISUAL nor EDITOR defined.");if(!editor)-editor="vi";+editor=DEFAULT_EDITOR;if(strcmp(editor,":")){size_tlen=strlen(editor);
@@ -99,19 +99,21 @@ set_reflog_action() {} git_editor(){+:"${DEFAULT_EDITOR:=vi}":"${GIT_EDITOR:=$(gitconfigcore.editor)}":"${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"case"$GIT_EDITOR,$TERM"in,dumb)echo>&2"No editor specified in GIT_EDITOR, core.editor, VISUAL,"-echo>&2"or EDITOR. Tried to fall back to vi but terminal is dumb."+echo>&2"or EDITOR. Tried to fall back to $DEFAULT_EDITOR"\+"but terminal is dumb."echo>&2"Please set one of these variables to an appropriate"echo>&2"editor or run $0 with options that will not cause an"echo>&2"editor to be invoked (e.g., -m or -F for git-commit)."exit1;;esac-eval"${GIT_EDITOR:=vi}"'"$@"'+eval"${GIT_EDITOR:=$DEFAULT_EDITOR}"'"$@"'} is_bare_repository(){
@@ -3,12 +3,13 @@# License: GPL v2 or laterusewarnings;usestrict;-usevarsqw/$AUTHOR$VERSION$DEFAULT_PAGER+usevarsqw/$AUTHOR$VERSION$DEFAULT_PAGER$DEFAULT_EDITOR$sha1$sha1_short$_revision$_repository$_q$_authors$_authors_prog%users/;$AUTHOR='Eric Wong <normalperson@yhbt.net>';$VERSION='@@GIT_VERSION@@';$DEFAULT_PAGER='@@DEFAULT_PAGER@@';+$DEFAULT_EDITOR='@@DEFAULT_EDITOR@@';# From which subdir have we been invoked?my$cmd_dir_prefix=eval{
@@ -1322,7 +1323,7 @@ sub get_commit_entry {close$log_fhorcroak$!;if($_edit||($typeeq'tree')){-my$editor=$ENV{VISUAL}||$ENV{EDITOR}||'vi';+my$editor=$ENV{VISUAL}||$ENV{EDITOR}||$DEFAULT_EDITOR;# TODO: strip out spaces, comments, like git-commit.shsystem($editor,$commit_editmsg);}
@@ -44,7 +57,8 @@ test_expect_success 'dumb should error out when falling back on vi' 'TERM=vt100exportTERM-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"}\+EDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectunsetEDITORVISUALGIT_EDITOR
@@ -68,7 +82,8 @@ doneunsetEDITORVISUALGIT_EDITOR gitconfig--unset-allcore.editor-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"}\+EDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectcase"$i"in
From: David Roundy <hidden> Date: 2016-06-15 22:47:38
Any chance this will be exported as plumbing? I know it's pretty
high-level, but it'd be handy to have be able to write `git editor
$FILENAME` and just have it do the right thing. This would also mean
that the perl scripts below could be simplified.
Same goes for pager, of course...
David
On Thu, Oct 29, 2009 at 3:50 AM, Jonathan Nieder [off-list ref] wrote:
quoted hunk
Provide a DEFAULT_EDITOR knob to allow the fallback editor (to
use instead of vi if VISUAL, EDITOR, and GIT_EDITOR are unset) to
be set at build time according to a system’s policy. For
example, on Debian systems, the default editor should be the
'editor' command.
The contrib/fast-import/git-p4 script still uses vi, since it is
not modified by the Makefile currently, and making it require
build-time modification would create too much trouble for people
deploying that script.
This change makes t7005-editor into a mess. Any ideas for fixing
this?
Signed-off-by: Jonathan Nieder <redacted>
---
Makefile | 10 ++++++++++
editor.c | 2 +-
git-add--interactive.perl | 3 ++-
git-sh-setup.sh | 6 ++++--
git-svn.perl | 5 +++--
t/Makefile | 2 ++
t/t7005-editor.sh | 29 ++++++++++++++++++++++-------
7 files changed, 44 insertions(+), 13 deletions(-)
#
# Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if
# you want to use something different.
+#
+# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you
+# want to use something different.
GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
@$(SHELL_PATH) ./GIT-VERSION-GEN
return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
if (!editor)
- editor = "vi";
+ editor = DEFAULT_EDITOR;
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
}
git_editor() {
+ : "${DEFAULT_EDITOR:=vi}"
: "${GIT_EDITOR:=$(git config core.editor)}"
: "${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"
case "$GIT_EDITOR,$TERM" in
,dumb)
echo >&2 "No editor specified in GIT_EDITOR, core.editor, VISUAL,"
- echo >&2 "or EDITOR. Tried to fall back to vi but terminal is dumb."
+ echo >&2 "or EDITOR. Tried to fall back to $DEFAULT_EDITOR" \
+ "but terminal is dumb."
echo >&2 "Please set one of these variables to an appropriate"
echo >&2 "editor or run $0 with options that will not cause an"
echo >&2 "editor to be invoked (e.g., -m or -F for git-commit)."
exit 1
;;
esac
- eval "${GIT_EDITOR:=vi}" '"$@"'
+ eval "${GIT_EDITOR:=$DEFAULT_EDITOR}" '"$@"'
}
is_bare_repository () {
# License: GPL v2 or later
use warnings;
use strict;
-use vars qw/ $AUTHOR $VERSION $DEFAULT_PAGER
+use vars qw/ $AUTHOR $VERSION $DEFAULT_PAGER $DEFAULT_EDITOR
$sha1 $sha1_short $_revision $_repository
$_q $_authors $_authors_prog %users/;
$AUTHOR = 'Eric Wong [off-list ref]';
$VERSION = '@@GIT_VERSION@@';
$DEFAULT_PAGER = '@@DEFAULT_PAGER@@';
+$DEFAULT_EDITOR = '@@DEFAULT_EDITOR@@';
# From which subdir have we been invoked?
my $cmd_dir_prefix = eval {
@@ -1322,7 +1323,7 @@ sub get_commit_entry {
close $log_fh or croak $!;
if ($_edit || ($type eq 'tree')) {
- my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
+ my $editor = $ENV{VISUAL} || $ENV{EDITOR} || $DEFAULT_EDITOR;
# TODO: strip out spaces, comments, like git-commit.sh
system($editor, $commit_editmsg);
}
@@ -44,7 +57,8 @@ test_expect_success 'dumb should error out when falling back on vi' '
TERM=vt100
export TERM
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"} \
+ EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -68,7 +82,8 @@ done
unset EDITOR VISUAL GIT_EDITOR
git config --unset-all core.editor
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"} \
+ EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
--
1.6.5.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
From: Johannes Sixt <hidden> Date: 2016-06-15 22:47:38
David Roundy schrieb:
Any chance this will be exported as plumbing? I know it's pretty
high-level, but it'd be handy to have be able to write `git editor
$FILENAME` and just have it do the right thing. This would also mean
that the perl scripts below could be simplified.
Something like below? Possible usage in shell scripts:
editor=$(git var GIT_EDITOR)
"$editor" "$filename"
-- Hannes
PS: warning: linewrapped.
Subject: [PATCH] Teach git var about GIT_EDITOR
Signed-off-by: Johannes Sixt <redacted>
---
cache.h | 1 +
editor.c | 13 +++++++++++--
var.c | 6 ++++++
3 files changed, 18 insertions(+), 2 deletions(-)
@@ -16,11 +16,20 @@ int launch_editor(const char *path, struct strbuf terminal = getenv("TERM"); if (!editor && (!terminal || !strcmp(terminal, "dumb")))- return error("Terminal is dumb but no VISUAL nor EDITOR defined.");+ return "/dev/null"; if (!editor) editor = "vi";+ return editor;+}++int launch_editor(const char *path, struct strbuf *buffer, const char
*const *env)
+{
+ const char *editor = git_editor();
+
+ if (!strcmp(editor, "/dev/null"))
+ return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
From: Ben Walton <hidden> Date: 2016-06-15 22:47:38
From: Junio C Hamano <redacted>
On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which
forces users to alter their environment (PATH, GIT_PAGER, LESS,
etc) or have a local or global gitconfig before paging works as
expected.
On Debian systems, by policy packages must fall back to the
'pager' command, so that changing the target of the
/usr/bin/pager symlink changes the default pager for all packages
at once.
Provide a DEFAULT_PAGER knob so packagers can set the fallback
pager to something appropriate during the build.
This puts the "less" default in the Makefile instead of pager.c, since
it is needed for git-svn and git-am, too. This means that the
DEFAULT_PAGER preprocessor token _has_ to be defined on the command
line for git to build. I was worried about this for a moment, but
GIT_VERSION already works this way without trouble.
Probably the DEFAULT_PAGER setting should be added to something
like TRACK_CFLAGS as well. Actually, some other settings that
can change without forcing files to be rebuilt (e.g. SHELL_PATH),
too. This should be probably be addressed separately.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <redacted>
---
Makefile | 9 +++++++++
git-am.sh | 2 +-
git-svn.perl | 5 +++--
pager.c | 2 +-
4 files changed, 14 insertions(+), 4 deletions(-)
@@ -200,6 +200,9 @@ all::# memory allocators with the nedmalloc allocator written by Niall Douglas.## Define NO_REGEX if you have no or inferior regex support in your C library.+#+# Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if+# you want to use something different.GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
@@ -3,11 +3,12 @@# License: GPL v2 or laterusewarnings;usestrict;-usevarsqw/$AUTHOR$VERSION+usevarsqw/$AUTHOR$VERSION$DEFAULT_PAGER$sha1$sha1_short$_revision$_repository$_q$_authors$_authors_prog%users/;$AUTHOR='Eric Wong <normalperson@yhbt.net>';$VERSION='@@GIT_VERSION@@';+$DEFAULT_PAGER='@@DEFAULT_PAGER@@';# From which subdir have we been invoked?my$cmd_dir_prefix=eval{
@@ -5031,7 +5032,7 @@ sub git_svn_log_cmd {subconfig_pager{$pager||=$ENV{GIT_PAGER}||$ENV{PAGER};if(!defined$pager){-$pager='less';+$pager=$DEFAULT_PAGER;}elsif(length$pager==0||$pagereq'cat'){$pager=undef;}
From: Ben Walton <hidden> Date: 2016-06-15 22:47:38
The two patches look ok to me (not withstanding the comments already
made about the test and possible future changes required).
I did make a few small tweaks to use | instead of / as the sed
substitution separator (since we're presumably working with full
paths). The substitution was also extended into the .sh scripts
(git-am, explicitly).
Jonathan Nieder (1):
Provide a build time default-editor setting
Junio C Hamano (1):
Provide a build time default-pager setting
Makefile | 19 +++++++++++++++++++
editor.c | 2 +-
git-add--interactive.perl | 3 ++-
git-am.sh | 2 +-
git-sh-setup.sh | 6 ++++--
git-svn.perl | 8 +++++---
pager.c | 2 +-
t/Makefile | 2 ++
t/t7005-editor.sh | 29 ++++++++++++++++++++++-------
9 files changed, 57 insertions(+), 16 deletions(-)
From: Junio C Hamano <hidden> Date: 2016-06-15 22:47:38
Johannes Sixt [off-list ref] writes:
David Roundy schrieb:
quoted
Any chance this will be exported as plumbing? I know it's pretty
high-level, but it'd be handy to have be able to write `git editor
$FILENAME` and just have it do the right thing. This would also mean
that the perl scripts below could be simplified.
Something like below? Possible usage in shell scripts:
editor=$(git var GIT_EDITOR)
"$editor" "$filename"
I think we support GIT_EDITOR that is command path plus its initial
command line arguments, so you do not want dq around $editor, right?
From: Junio C Hamano <hidden> Date: 2016-06-15 22:47:38
Jonathan Nieder [off-list ref] writes:
Provide a DEFAULT_EDITOR knob to allow the fallback editor (to
use instead of vi if VISUAL, EDITOR, and GIT_EDITOR are unset) to
be set at build time according to a system’s policy. For
example, on Debian systems, the default editor should be the
'editor' command.
I think we allow things like
GIT_EDITOR='"/c/my program/vi" --i-like-color --config=$HOME/.myvicfg'
and the eval construct in git-sh-setup.sh is about supporting that kind of
insanity^Wflexibility.
My "how about" patch on DEFAULT_PAGER might be minimally safe with
make DEFAULT_PAGER="/c/my program/less"
but if you are going to do this for real, you would need to use proper
quoting in the Makefile (look for _SQ for hints).
Also I do not think it allows this at all:
make DEFAULT_PAGER='"/c/my program/less" --i-like-color'
It probably is Ok to force the "default" one to be just the path to the
command (i.e. not part of command line), but I thought this would be worth
pointing out.
This change makes t7005-editor into a mess. Any ideas for fixing
this?
I think the introduction of DEFAULT_EDITOR makes it unfixable; your
DEFAULT_EDITOR may be set to '/usr/bin/vi' not 'vi'.
Just detect DEFAULT_EDITOR being not the default 'vi' and abort/skip the
entire test, perhaps?
From: Johannes Sixt <hidden> Date: 2016-06-15 22:47:38
On Donnerstag, 29. Oktober 2009, Junio C Hamano wrote:
Johannes Sixt [off-list ref] writes:
quoted
Something like below? Possible usage in shell scripts:
editor=$(git var GIT_EDITOR)
"$editor" "$filename"
I think we support GIT_EDITOR that is command path plus its initial
command line arguments, so you do not want dq around $editor, right?
Yeah, whatever, I didn't take the time to think it through. But this may be an
opportunity to give some life back to the zombie that git-var currently is,
that is, to make it the plumbing that does value discovery for variables like
GIT_AUTHOR_INDENT, GIT_COMMITTER_IDENT, GIT_EDITOR, and perhaps also
GIT_PAGER.
-- Hannes
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Junio C Hamano wrote:
My "how about" patch on DEFAULT_PAGER might be minimally safe with
make DEFAULT_PAGER="/c/my program/less"
It isn’t, actually, since in pager.c the pager already gets run through
sh if it contains certain shell metacharacters.
but if you are going to do this for real, you would need to use proper
quoting in the Makefile (look for _SQ for hints).
Good catch --- thanks.
quoted
This change makes t7005-editor into a mess. Any ideas for fixing
this?
I think the introduction of DEFAULT_EDITOR makes it unfixable; your
DEFAULT_EDITOR may be set to '/usr/bin/vi' not 'vi'.
Just detect DEFAULT_EDITOR being not the default 'vi' and abort/skip the
entire test, perhaps?
Yep, unfortunately that looks like the best thing to do. I tried to
salvage some of the test for distros (like Debian) that might override
the default without using an absolute path.
Here’s an updated series. It doesn’t provide git var --run yet since
the Windows exit status magic means that would require either futzing
with the run_pager() implementation or reimplementing cat in var.c.
Thoughts?
Johannes Sixt (1):
Teach git var about GIT_EDITOR
Jonathan Nieder (6):
launch_editor: Longer error message when TERM=dumb
Handle more shell metacharacters in editor name
Teach git var about GIT_PAGER
add -i, send-email, svn, p4, etc: Use "git var GIT_EDITOR"
am -i, git-svn: use $(git var GIT_PAGER) instead of 'less'
Provide a build time default-editor setting
Junio C Hamano (1):
Provide a build time default-pager setting
Documentation/config.txt | 4 +---
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 ++--
Documentation/git-var.txt | 14 ++++++++++++++
Makefile | 28 ++++++++++++++++++++++++++++
cache.h | 2 ++
contrib/fast-import/git-p4 | 5 +----
editor.c | 33 ++++++++++++++++++++++++++++-----
git-add--interactive.perl | 3 +--
git-am.sh | 5 ++++-
git-send-email.perl | 3 ++-
git-sh-setup.sh | 19 ++++++-------------
git-svn.perl | 11 ++++-------
pager.c | 24 ++++++++++++++++++++----
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
var.c | 20 ++++++++++++++++++++
16 files changed, 158 insertions(+), 50 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Before falling back to vi, git checks if the terminal can support
such an editor by checking if $TERM is dumb. git-sh-setup and
editor.c have similar but distinct error messages for this case.
To avoid changes in behavior when switching from one
implementation to the other, standardize on one error message.
Signed-off-by: Jonathan Nieder <redacted>
---
Probably this check should apply to $VISUAL, too, but that is a
separate topic.
I am not sure which is the better error message. It looks like some
effort went into the longer message so I thought I should give it a
try, but I kind of prefer the shorter one.
editor.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
@@ -16,7 +16,13 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *enterminal=getenv("TERM");if(!editor&&(!terminal||!strcmp(terminal,"dumb")))-returnerror("Terminal is dumb but no VISUAL nor EDITOR defined.");+/* Terminal is dumb but no VISUAL nor EDITOR defined. */+returnerror(+"No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"+"or EDITOR. Tried to fall back to vi but terminal is dumb.\n"+"Please set one of these variables to an appropriate\n"+"editor or run again with options that will not cause an\n"+"editor to be invoked (e.g., -m or -F for git commit).");if(!editor)editor="vi";
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Pass the editor name to the shell if it contains any susv3 shell
special character (globs, redirections, variable substitutions,
escapes, etc). This way, the meaning of some characters will not
meaninglessly change when others are added, and git commands
implemented in C and in shell scripts will interpret editor names
in the same way.
This does not make the GIT_EDITOR setting any more expressive,
since one could always use single quotes to force the editor to
be passed to the shell.
Signed-off-by: Jonathan Nieder<redacted>
---
editor.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
From: Johannes Sixt <redacted>
Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.
Signed-off-by: Johannes Sixt <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 34 insertions(+), 3 deletions(-)
@@ -36,6 +36,14 @@ GIT_AUTHOR_IDENT:: GIT_COMMITTER_IDENT:: The person who put a piece of code into git.+GIT_EDITOR::+ Text editor for use by git commands. The value is meant to be+ interpreted by the shell when it is used. Examples: `~/bin/vi`,+ `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"+ --nofork`. The order of preference is the `$GIT_EDITOR`+ environment variable, then `core.editor` configuration, then+ `$VISUAL`, then `$EDITOR`, and then finally 'vi'.+ Diagnostics ----------- You don't exist. Go away!::
@@ -15,18 +15,30 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *eneditor=getenv("EDITOR");terminal=getenv("TERM");-if(!editor&&(!terminal||!strcmp(terminal,"dumb")))+if(!editor&&(!terminal||!strcmp(terminal,"dumb"))){/* Terminal is dumb but no VISUAL nor EDITOR defined. */-returnerror(+error("No editor specified in GIT_EDITOR, core.editor, VISUAL,\n""or EDITOR. Tried to fall back to vi but terminal is dumb.\n""Please set one of these variables to an appropriate\n""editor or run again with options that will not cause an\n""editor to be invoked (e.g., -m or -F for git commit).");+returnNULL;+}if(!editor)editor="vi";+returneditor;+}++intlaunch_editor(constchar*path,structstrbuf*buffer,constchar*const*env)+{+constchar*editor=git_editor();++if(!editor)+return-1;+if(strcmp(editor,":")){size_tlen=strlen(editor);inti=0;
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Expose the command found by setup_pager() for scripts to use.
Scripts can use this to avoid repeating the logic to look for a
proper pager in each command.
Signed-off-by: Jonathan Nieder <redacted>
---
Documentation/git-var.txt | 6 ++++++
cache.h | 1 +
pager.c | 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
@@ -44,6 +44,12 @@ GIT_EDITOR:: environment variable, then `core.editor` configuration, then `$VISUAL`, then `$EDITOR`, and then finally 'vi'.+GIT_PAGER::+ Text viewer for use by git commands (e.g., 'less'). The value+ is meant to be interpreted by the shell. The order of preference+ is the `$GIT_PAGER` environment variable, then `core.pager`+ configuration, then `$PAGER`, and then finally 'less'.+ Diagnostics ----------- You don't exist. Go away!::
@@ -60,6 +62,16 @@ void setup_pager(void)if(!pager)pager="less";elseif(!*pager||!strcmp(pager,"cat"))+pager=NULL;++returnpager;+}++voidsetup_pager(void)+{+constchar*pager=git_pager();++if(!pager)return;spawned_pager=1;/* means we are emitting to terminal */
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Use the new "git var GIT_EDITOR" feature to decide what editor to
use, instead of duplicating its logic elsewhere. This should make
the behavior of commands in edge cases (e.g., editor names with
spaces) a little more consistent.
Signed-off-by: Jonathan Nieder <redacted>
---
Documentation/config.txt | 4 +---
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 ++--
contrib/fast-import/git-p4 | 5 +----
git-add--interactive.perl | 3 +--
git-send-email.perl | 3 ++-
git-sh-setup.sh | 19 ++++++-------------
git-svn.perl | 5 ++---
8 files changed, 16 insertions(+), 29 deletions(-)
@@ -387,9 +387,7 @@ core.editor:: Commands such as `commit` and `tag` that lets you edit messages by launching an editor uses the value of this variable when it is set, and the environment variable- `GIT_EDITOR` is not set. The order of preference is- `GIT_EDITOR` environment, `core.editor`, `VISUAL` and- `EDITOR` environment variables and then finally `vi`.+ `GIT_EDITOR` is not set. See linkgit:git-var[1]. core.pager:: The command that git will use to paginate output. Can
@@ -323,7 +323,7 @@ ENVIRONMENT AND CONFIGURATION VARIABLES The editor used to edit the commit log message will be chosen from the GIT_EDITOR environment variable, the core.editor configuration variable, the VISUAL environment variable, or the EDITOR environment variable (in that-order).+order). See linkgit:git-var[1] for details. HOOKS -----
@@ -60,8 +60,8 @@ The --bcc option must be repeated for each user you want on the bcc list. The --cc option must be repeated for each user you want on the cc list. --compose::- Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an- introductory message for the patch series.+ Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])+ to edit an introductory message for the patch series. + When '--compose' is used, git send-email will use the From, Subject, and In-Reply-To headers specified in the message. If the body of the message
@@ -987,8 +987,7 @@ sub edit_hunk_manually {EOFclose$fh;-my$editor=$ENV{GIT_EDITOR}||$repo->config("core.editor")-||$ENV{VISUAL}||$ENV{EDITOR}||"vi";+chomp(my$editor=run_cmd_pipe(qw(git var GIT_EDITOR)));system('sh','-c',$editor.' "$@"',$editor,$hunkfile);if($?!=0){
@@ -99,19 +99,12 @@ set_reflog_action() {} git_editor(){-:"${GIT_EDITOR:=$(gitconfigcore.editor)}"-:"${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"-case"$GIT_EDITOR,$TERM"in-,dumb)-echo>&2"No editor specified in GIT_EDITOR, core.editor, VISUAL,"-echo>&2"or EDITOR. Tried to fall back to vi but terminal is dumb."-echo>&2"Please set one of these variables to an appropriate"-echo>&2"editor or run $0 with options that will not cause an"-echo>&2"editor to be invoked (e.g., -m or -F for git-commit)."-exit1-;;-esac-eval"${GIT_EDITOR:=vi}"'"$@"'+iftest-z"${GIT_EDITOR:+set}"+then+GIT_EDITOR="$(gitvarGIT_EDITOR)"||return$?+fi++eval"$GIT_EDITOR"'"$@"'} is_bare_repository(){
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Use the new "git var GIT_PAGER" command to ask what pager to use.
Without this change, the core.pager configuration is ignored by
these commands.
Signed-off-by: Jonathan Nieder <redacted>
---
git-am.sh | 5 ++++-
git-svn.perl | 6 ++----
2 files changed, 6 insertions(+), 5 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Provide a DEFAULT_EDITOR knob to allow setting the fallback
editor to use instead of vi (when VISUAL, EDITOR, and GIT_EDITOR
are unset). The value can be set at build time according to a
system’s policy. For example, on Debian systems, the default
editor should be the 'editor' command.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <bwalton@bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <redacted>
---
Makefile | 17 +++++++++++++++++
editor.c | 11 ++++++++---
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
3 files changed, 49 insertions(+), 10 deletions(-)
@@ -200,6 +200,14 @@ all::# memory allocators with the nedmalloc allocator written by Niall Douglas.## Define NO_REGEX if you have no or inferior regex support in your C library.+#+# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you+# want to use something different. The value will be interpreted by the shell+# if necessary when it is used. Examples:+#+# DEFAULT_EDITOR='~/bin/vi',+# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',+# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
@@ -19,15 +23,16 @@ const char *git_editor(void)/* Terminal is dumb but no VISUAL nor EDITOR defined. */error("No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"-"or EDITOR. Tried to fall back to vi but terminal is dumb.\n"+"or EDITOR. Tried to fall back to %s but terminal is dumb.\n""Please set one of these variables to an appropriate\n""editor or run again with options that will not cause an\n"-"editor to be invoked (e.g., -m or -F for git commit).");+"editor to be invoked (e.g., -m or -F for git commit).",+DEFAULT_EDITOR);returnNULL;}if(!editor)-editor="vi";+editor=DEFAULT_EDITOR;returneditor;}
@@ -4,7 +4,26 @@ test_description='GIT_EDITOR, core.editor, and stuff' ../test-lib.sh-foriinGIT_EDITORcore_editorEDITORVISUALvi+unsetEDITORVISUALGIT_EDITOR++test_expect_success'does editor have a simple name (no slashes, etc)?''++editor=$(TERM=vt100gitvarGIT_EDITOR)&&+test-n"$editor"&&+simple=t&&+case"$editor"in+*/*|core_editor|[A-Z]*)+unsetsimple;;+esac++'+iftest-z"${simple+set}"+then+say'skipping editor tests, default editor is not sought on PATH'+test_done+fi++foriinGIT_EDITORcore_editorEDITORVISUAL"$editor"docat>e-$i.sh<<-EOF#!$SHELL_PATH
@@ -44,7 +61,7 @@ test_expect_success 'dumb should error out when falling back on vi' 'TERM=vt100exportTERM-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin"$editor"EDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectunsetEDITORVISUALGIT_EDITOR
@@ -68,7 +85,7 @@ doneunsetEDITORVISUALGIT_EDITOR gitconfig--unset-allcore.editor-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin"$editor"EDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectcase"$i"in
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
From: Junio C Hamano <redacted>
Provide a DEFAULT_PAGER knob so packagers can set the fallback
pager to something appropriate during the build.
Examples:
On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which
forces users to alter their environment (PATH, GIT_PAGER, LESS,
etc) or have a local or global gitconfig before paging works as
expected.
On Debian systems, by policy packages must fall back to the
'pager' command, so that changing the target of the
/usr/bin/pager symlink changes the default pager for all packages
at once.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Previously I suggested that the default pager isn’t being tracked
properly with TRACK_CFLAGS. Actually, since it is included in
BASIC_CFLAGS, it always was. Sorry for the confusion.
Makefile | 11 +++++++++++
pager.c | 6 +++++-
2 files changed, 16 insertions(+), 1 deletions(-)
@@ -201,6 +201,10 @@ all::## Define NO_REGEX if you have no or inferior regex support in your C library.#+# Define DEFAULT_PAGER to a sensible pager command (defaults to "less") if+# you want to use something different. The value will be interpreted by the+# shell at runtime when it is used.+## Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you# want to use something different. The value will be interpreted by the shell# if necessary when it is used. Examples:
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Expose the functionality of launch_editor() for scripts to use.
Signed-off-by: Jonathan Nieder <redacted>
---
As I mentioned in the cover letter, the analogous change for the pager
is a little more tricky. I was wrong to blame Windows for this. The
excellent commit ea27a18 (spawn pager via run_command interface,
2008-07-22) explains all.
The difficulties: the pager receives input from the current process
and the run_pager() function does not take an argument to take input
from somewhere else. Also the pager is not exec()'d directly, so the
current process sticks around uselessly until it quits and it is a
little tricky to find the 'less' exit status for "git var --run" to
use as well.
Documentation/git-var.txt | 10 ++++++++-
var.c | 48 +++++++++++++++++++++++++++++++++++++-------
2 files changed, 49 insertions(+), 9 deletions(-)
@@ -22,6 +25,11 @@ OPTIONS as well. (However, the configuration variables listing functionality is deprecated in favor of 'git config -l'.)+--run variable [args]::+ If the specified logical variable represents a command, run that+ command. For example, `git var --run GIT_EDITOR foo.txt` edits+ foo.txt with the text editor git is configured to use.+ EXAMPLE -------- $ git var GIT_AUTHOR_IDENT
@@ -6,7 +6,8 @@#include"cache.h"#include"exec_cmd.h"-staticconstcharvar_usage[]="git var [-l | <variable>]";+staticconstcharvar_usage[]=+"git var { -l | <variable> | --run <variable> [args] }";staticconstchar*editor(intflag){
@@ -26,16 +27,25 @@ static const char *pager(int flag)returnpgm;}+staticintrun_editor(intargc,constchar*const*argv)+{+if(argc>1)+returnerror("cannot launch editor with more than one file");++returnlaunch_editor(argv[0],NULL,NULL);+}+structgit_var{constchar*name;constchar*(*read)(int);+int(*run)(intargc,constchar*const*argv);};staticstructgit_vargit_vars[]={-{"GIT_COMMITTER_IDENT",git_committer_info},-{"GIT_AUTHOR_IDENT",git_author_info},-{"GIT_EDITOR",editor},-{"GIT_PAGER",pager},-{"",NULL},+{"GIT_COMMITTER_IDENT",git_committer_info,NULL},+{"GIT_AUTHOR_IDENT",git_author_info,NULL},+{"GIT_EDITOR",editor,run_editor},+{"GIT_PAGER",pager,NULL},+{"",NULL,NULL},};staticvoidlist_vars(void)
@@ -59,6 +69,17 @@ static const char *read_var(const char *var)returnval;}+staticintrun_var_cmd(constchar*var,intargc,char**argv)+{+structgit_var*ptr;++for(ptr=git_vars;ptr->read;ptr++)+if(ptr->run&&strcmp(var,ptr->name)==0)+returnptr->run(argc,(constchar*const*)argv);++returnerror("%s is not a variable command",var);+}+staticintshow_config(constchar*var,constchar*value,void*cb){if(value)
@@ -72,12 +93,23 @@ int main(int argc, char **argv){constchar*val;intnongit;++git_extract_argv0_path(argv[0]);++if(argv[1]&&strcmp(argv[1],"--run")==0){+if(argc<=2)+usage(var_usage);++setup_git_directory_gently(&nongit);+git_config(git_default_config,NULL);++returnrun_var_cmd(argv[2],argc-3,argv+3);+}+if(argc!=2){usage(var_usage);}-git_extract_argv0_path(argv[0]);-setup_git_directory_gently(&nongit);val=NULL;
From: Johannes Sixt <hidden> Date: 2016-06-15 22:47:38
Jonathan Nieder schrieb:
From: Johannes Sixt <redacted>
Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.
Signed-off-by: Johannes Sixt <redacted>
Signed-off-by: Jonathan Nieder <redacted>
Thanks for cleaning up behind me. I don't mind if you take authorship, but
if you do keep my name, please make it:
From: Johannes Sixt <redacted>
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor, *terminal;
...
terminal = getenv("TERM");
- if (!editor && (!terminal || !strcmp(terminal, "dumb")))
+ if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
/* Terminal is dumb but no VISUAL nor EDITOR defined. */
- return error(
+ error(
"No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"
"or EDITOR. Tried to fall back to vi but terminal is dumb.\n"
"Please set one of these variables to an appropriate\n"
"editor or run again with options that will not cause an\n"
"editor to be invoked (e.g., -m or -F for git commit).");
+ return NULL;
+ }
I somehow dislike that this huge error message is in git_editor(). The
return value, NULL, should be indication enough for the callers to handle
the situation suitable. In particular, launch_editor() wants to write this
big warning, but 'git var -l' can avoid the error message and write only a
short notice:
GIT_EDITOR=terminal is dumb, but VISUAL and EDITOR unset
This should be
return pgm ? pgm : "terminal is dumb, but VISUAL and EDITOR unset";
otherwise, 'git var -l' later trips over printf("%s", NULL).
-- Hannes
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:38
Johannes Sixt wrote:
Jonathan Nieder schrieb:
quoted
From: Johannes Sixt <redacted>
[...]
Thanks for cleaning up behind me. I don't mind if you take
authorship, but if you do keep my name, please make it:
From: Johannes Sixt <redacted>
Thanks for the catch.
quoted
+ error(
"No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"
"or EDITOR. Tried to fall back to vi but terminal is dumb.\n"
"Please set one of these variables to an appropriate\n"
"editor or run again with options that will not cause an\n"
"editor to be invoked (e.g., -m or -F for git commit).");
+ return NULL;
+ }
I somehow dislike that this huge error message is in git_editor().
Makes sense.
I am a bit uncomfortable with this error in general. It makes some
sense to refuse to use $VISUAL and fall back to $EDITOR if TERM=dumb,
since without this the distinction between VISUAL and EDITOR is not
very useful. But wouldn’t that check be equally useful if GIT_EDITOR
or core.editor is set to vi? Ideally, vi itself would check TERM and
error out, and git would only need to report and handle the exit.
Unfortunately, at least vim is happy to assume a terminal supports
ANSI sequences even if TERM=dumb (e.g., when running from a text
editor like Acme). Unless VISUAL, GIT_EDITOR, and core.editor are
unset, nobody gets the benefit of this check.
Should git error out rather than run $VISUAL when TERM=dumb? How
about $GIT_EDITOR?
The advice about options to avoid invoking an editor is not very
helpful except with 'git commit', so probably only 'git commit' should
print that message.
The return value, NULL, should be indication enough for the callers
to handle the situation suitable.
Good idea.
In particular, launch_editor()
wants to write this big warning, but 'git var -l' can avoid the
error message and write only a short notice:
GIT_EDITOR=terminal is dumb, but VISUAL and EDITOR unset
Maybe 'git var -l' should omit GIT_EDITOR in this situation.
Thanks for the comments,
Jonathan
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Junio C Hamano wrote:
I'll queue these for now probably on 'pu', but with the comments we saw on
the list expect them to be followed up with replacement patches.
Here’s a replacement series. It omits the longer error message when
TERM=dumb and the git var --run experiment because I was not happy
with where either of those were going.
Thanks for all the comments, everyone.
Johannes Sixt (1):
Teach git var about GIT_EDITOR
Jonathan Nieder (6):
Handle more shell metacharacters in editor names
Do not use VISUAL editor on dumb terminals
Teach git var about GIT_PAGER
add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
am -i, git-svn: use "git var GIT_PAGER"
Provide a build time default-editor setting
Junio C Hamano (1):
Provide a build time default-pager setting
Documentation/config.txt | 4 +---
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 ++--
Documentation/git-var.txt | 14 ++++++++++++++
Makefile | 28 ++++++++++++++++++++++++++++
cache.h | 2 ++
contrib/fast-import/git-p4 | 5 +----
editor.c | 32 +++++++++++++++++++++++---------
git-add--interactive.perl | 3 +--
git-am.sh | 5 ++++-
git-send-email.perl | 3 ++-
git-sh-setup.sh | 19 ++++++-------------
git-svn.perl | 11 ++++-------
pager.c | 24 ++++++++++++++++++++----
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
var.c | 31 ++++++++++++++++++++++++++++++-
16 files changed, 163 insertions(+), 55 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Pass the editor name to the shell if it contains any susv3 shell
special character (globs, redirections, variable substitutions,
escapes, etc). This way, the meaning of some characters will not
meaninglessly change when others are added, and git commands
implemented in C and in shell scripts will interpret editor names
in the same way.
This does not make the GIT_EDITOR setting any more expressive,
since one could always use single quotes to force the editor to
be passed to the shell.
Signed-off-by: Jonathan Nieder <redacted>
---
Unchanged from v2.
editor.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
or set to "dumb". Traditionally, VISUAL is set to a screen
editor and EDITOR to a line-based editor, which should be more
useful in that situation.
vim, for example, is happy to assume a terminal supports ANSI
sequences even if TERM is dumb (e.g., when running from a text
editor like Acme). git already refuses to fall back to vi on a
dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
unset, but without this patch, that check is suppressed by
VISUAL=vi.
Signed-off-by: Jonathan Nieder <redacted>
---
This patch eases my discomfort about the error message a little. It
is not actually needed to support any ways of working I engage in.
If stdout is redirected, this is probably still making the wrong
choice; isatty(STDOUT_FILENO) might be a more useful datum to use.
But it does not seem worth complicating the logic further.
editor.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
@@ -4,19 +4,19 @@intlaunch_editor(constchar*path,structstrbuf*buffer,constchar*const*env){-constchar*editor,*terminal;+constchar*editor=getenv("GIT_EDITOR");+constchar*terminal=getenv("TERM");+intterminal_is_dumb=!terminal||!strcmp(terminal,"dumb");-editor=getenv("GIT_EDITOR");if(!editor&&editor_program)editor=editor_program;-if(!editor)+if(!editor&&!terminal_is_dumb)editor=getenv("VISUAL");if(!editor)editor=getenv("EDITOR");-terminal=getenv("TERM");-if(!editor&&(!terminal||!strcmp(terminal,"dumb")))-returnerror("Terminal is dumb but no VISUAL nor EDITOR defined.");+if(!editor&&terminal_is_dumb)+returnerror("terminal is dumb, but EDITOR unset");if(!editor)editor="vi";
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
From: Johannes Sixt <redacted>
Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.
If no satisfactory GIT_EDITOR could be chosen, let "git var -l"
output a warning. This warning goes to stderr so as not to
confuse scripts. Example:
core.logallrefupdates=true
*** Please tell me who you are.
Run
git config --global user.email "you@example.com"
git config --global user.name "Your Name"
to set your account's default identity.
Omit --global to set the identity only in this repository.
GIT_COMMITTER_IDENT=user <user@domain> 1256952739 -0500
GIT_AUTHOR_IDENT=user <user@domain> 1256952739 -0500
warning: GIT_EDITOR: terminal is dumb, but EDITOR unset
Signed-off-by: Johannes Sixt <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 14 ++++++++++++--
var.c | 21 ++++++++++++++++++++-
4 files changed, 41 insertions(+), 3 deletions(-)
@@ -36,6 +36,14 @@ GIT_AUTHOR_IDENT:: GIT_COMMITTER_IDENT:: The person who put a piece of code into git.+GIT_EDITOR::+ Text editor for use by git commands. The value is meant to be+ interpreted by the shell when it is used. Examples: `~/bin/vi`,+ `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"+ --nofork`. The order of preference is the `$GIT_EDITOR`+ environment variable, then `core.editor` configuration, then+ `$VISUAL`, then `$EDITOR`, and then finally 'vi'.+ Diagnostics ----------- You don't exist. Go away!::
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *eneditor=getenv("EDITOR");if(!editor&&terminal_is_dumb)-returnerror("terminal is dumb, but EDITOR unset");+returnNULL;if(!editor)editor="vi";+returneditor;+}++intlaunch_editor(constchar*path,structstrbuf*buffer,constchar*const*env)+{+constchar*editor=git_editor();++if(!editor)+returnerror("terminal is dumb, but EDITOR unset");+if(strcmp(editor,":")){size_tlen=strlen(editor);inti=0;
@@ -8,6 +8,21 @@staticconstcharvar_usage[]="git var [-l | <variable>]";+staticconstchar*editor(intflag)+{+constchar*pgm=git_editor();++if(!pgm){+if(flag&IDENT_ERROR_ON_NO_NAME)+die("terminal is dumb, but EDITOR unset");+if(flag&IDENT_WARN_ON_NO_NAME)+warning("GIT_EDITOR: terminal is dumb, "+"but EDITOR unset");+}++returnpgm;+}+structgit_var{constchar*name;constchar*(*read)(int);
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Expose the command found by setup_pager() for scripts to use.
Scripts can use this to avoid repeating the logic to look for a
proper pager in each command.
Signed-off-by: Jonathan Nieder <redacted>
---
No changes from the last version sent.
Documentation/git-var.txt | 6 ++++++
cache.h | 1 +
pager.c | 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
@@ -44,6 +44,12 @@ GIT_EDITOR:: environment variable, then `core.editor` configuration, then `$VISUAL`, then `$EDITOR`, and then finally 'vi'.+GIT_PAGER::+ Text viewer for use by git commands (e.g., 'less'). The value+ is meant to be interpreted by the shell. The order of preference+ is the `$GIT_PAGER` environment variable, then `core.pager`+ configuration, then `$PAGER`, and then finally 'less'.+ Diagnostics ----------- You don't exist. Go away!::
@@ -60,6 +62,16 @@ void setup_pager(void)if(!pager)pager="less";elseif(!*pager||!strcmp(pager,"cat"))+pager=NULL;++returnpager;+}++voidsetup_pager(void)+{+constchar*pager=git_pager();++if(!pager)return;spawned_pager=1;/* means we are emitting to terminal */
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Use the new "git var GIT_EDITOR" feature to decide what editor to
use, instead of duplicating its logic elsewhere. This should make
the behavior of commands in edge cases (e.g., editor names with
spaces) a little more consistent.
Signed-off-by: Jonathan Nieder <redacted>
---
Documentation/config.txt | 4 +---
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 ++--
contrib/fast-import/git-p4 | 5 +----
git-add--interactive.perl | 3 +--
git-send-email.perl | 3 ++-
git-sh-setup.sh | 19 ++++++-------------
git-svn.perl | 5 ++---
8 files changed, 16 insertions(+), 29 deletions(-)
@@ -387,9 +387,7 @@ core.editor:: Commands such as `commit` and `tag` that lets you edit messages by launching an editor uses the value of this variable when it is set, and the environment variable- `GIT_EDITOR` is not set. The order of preference is- `GIT_EDITOR` environment, `core.editor`, `VISUAL` and- `EDITOR` environment variables and then finally `vi`.+ `GIT_EDITOR` is not set. See linkgit:git-var[1]. core.pager:: The command that git will use to paginate output. Can
@@ -323,7 +323,7 @@ ENVIRONMENT AND CONFIGURATION VARIABLES The editor used to edit the commit log message will be chosen from the GIT_EDITOR environment variable, the core.editor configuration variable, the VISUAL environment variable, or the EDITOR environment variable (in that-order).+order). See linkgit:git-var[1] for details. HOOKS -----
@@ -60,8 +60,8 @@ The --bcc option must be repeated for each user you want on the bcc list. The --cc option must be repeated for each user you want on the cc list. --compose::- Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an- introductory message for the patch series.+ Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])+ to edit an introductory message for the patch series. + When '--compose' is used, git send-email will use the From, Subject, and In-Reply-To headers specified in the message. If the body of the message
@@ -987,8 +987,7 @@ sub edit_hunk_manually {EOFclose$fh;-my$editor=$ENV{GIT_EDITOR}||$repo->config("core.editor")-||$ENV{VISUAL}||$ENV{EDITOR}||"vi";+chomp(my$editor=run_cmd_pipe(qw(git var GIT_EDITOR)));system('sh','-c',$editor.' "$@"',$editor,$hunkfile);if($?!=0){
@@ -99,19 +99,12 @@ set_reflog_action() {} git_editor(){-:"${GIT_EDITOR:=$(gitconfigcore.editor)}"-:"${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"-case"$GIT_EDITOR,$TERM"in-,dumb)-echo>&2"No editor specified in GIT_EDITOR, core.editor, VISUAL,"-echo>&2"or EDITOR. Tried to fall back to vi but terminal is dumb."-echo>&2"Please set one of these variables to an appropriate"-echo>&2"editor or run $0 with options that will not cause an"-echo>&2"editor to be invoked (e.g., -m or -F for git-commit)."-exit1-;;-esac-eval"${GIT_EDITOR:=vi}"'"$@"'+iftest-z"${GIT_EDITOR:+set}"+then+GIT_EDITOR="$(gitvarGIT_EDITOR)"||return$?+fi++eval"$GIT_EDITOR"'"$@"'} is_bare_repository(){
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Use the new "git var GIT_PAGER" command to ask what pager to use.
Without this change, the core.pager configuration is ignored by
these commands.
Signed-off-by: Jonathan Nieder <redacted>
---
git-am.sh | 5 ++++-
git-svn.perl | 6 ++----
2 files changed, 6 insertions(+), 5 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Provide a DEFAULT_EDITOR knob to allow setting the fallback
editor to use instead of vi (when VISUAL, EDITOR, and GIT_EDITOR
are unset). The value can be set at build time according to a
system’s policy. For example, on Debian systems, the default
editor should be the 'editor' command.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Makefile | 17 +++++++++++++++++
editor.c | 6 +++++-
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
3 files changed, 46 insertions(+), 8 deletions(-)
@@ -200,6 +200,14 @@ all::# memory allocators with the nedmalloc allocator written by Niall Douglas.## Define NO_REGEX if you have no or inferior regex support in your C library.+#+# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you+# want to use something different. The value will be interpreted by the shell+# if necessary when it is used. Examples:+#+# DEFAULT_EDITOR='~/bin/vi',+# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',+# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
@@ -4,7 +4,26 @@ test_description='GIT_EDITOR, core.editor, and stuff' ../test-lib.sh-foriinGIT_EDITORcore_editorEDITORVISUALvi+unsetEDITORVISUALGIT_EDITOR++test_expect_success'does editor have a simple name (no slashes, etc)?''++editor=$(TERM=vt100gitvarGIT_EDITOR)&&+test-n"$editor"&&+simple=t&&+case"$editor"in+*/*|core_editor|[A-Z]*)+unsetsimple;;+esac++'+iftest-z"${simple+set}"+then+say'skipping editor tests, default editor is not sought on PATH'+test_done+fi++foriinGIT_EDITORcore_editorEDITORVISUAL"$editor"docat>e-$i.sh<<-EOF#!$SHELL_PATH
@@ -44,7 +61,7 @@ test_expect_success 'dumb should error out when falling back on vi' 'TERM=vt100exportTERM-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin"$editor"EDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectunsetEDITORVISUALGIT_EDITOR
@@ -68,7 +85,7 @@ doneunsetEDITORVISUALGIT_EDITOR gitconfig--unset-allcore.editor-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin"$editor"EDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectcase"$i"in
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
From: Junio C Hamano <redacted>
Provide a DEFAULT_PAGER knob so packagers can set the fallback
pager to something appropriate during the build.
Examples:
On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which
forces users to alter their environment (PATH, GIT_PAGER, LESS,
etc) or have a local or global gitconfig before paging works as
expected.
On Debian systems, by policy packages must fall back to the
'pager' command, so that changing the target of the
/usr/bin/pager symlink changes the default pager for all packages
at once.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Makefile | 11 +++++++++++
pager.c | 6 +++++-
2 files changed, 16 insertions(+), 1 deletions(-)
@@ -201,6 +201,10 @@ all::## Define NO_REGEX if you have no or inferior regex support in your C library.#+# Define DEFAULT_PAGER to a sensible pager command (defaults to "less") if+# you want to use something different. The value will be interpreted by the+# shell at runtime when it is used.+## Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you# want to use something different. The value will be interpreted by the shell# if necessary when it is used. Examples:
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:39
Jonathan Nieder wrote:
Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
or set to "dumb". Traditionally, VISUAL is set to a screen
editor and EDITOR to a line-based editor, which should be more
useful in that situation.
I was too lazy to wait for tests to finish on this one, and lo and
behold, they did not pass.
These additional changes seem to help, and they also add a test to
explain the change in editor behavior. The patch with these changes
squashed is also included in this message, below the scissors mark.
In the controlled environment used for tests, TERM is set to dumb
and ever since commit 02b3566 (test-lib.sh: Add a test_set_editor
function to safely set $VISUAL, 2008-05-04), most tests set VISUAL
when they want to set an editor for git to use. With this patch, they
should be using EDITOR instead.
--- a/t/t7005-editor.sh+++ b/t/t7005-editor.sh
@@ -42,6 +42,16 @@ test_expect_success 'dumb should error out when falling back on vi' 'fi'+test_expect_success'dumb should prefer EDITOR to VISUAL''++EDITOR=./e-EDITOR.sh&&+VISUAL=./e-VISUAL.sh&&+exportEDITORVISUAL&&+gitcommit--amend&&+test"$(gitshow-s--format=%s)"="Edited by EDITOR"++'+TERM=vt100exportTERMforiinviEDITORVISUALcore_editorGIT_EDITOR---a/t/t7501-commit.sh+++b/t/t7501-commit.sh
@@ -107,7 +107,7 @@ chmod 755 editor test_expect_success\"editing message from other commit"\"echo 'hula hula' >file && \-VISUAL=./editorgitcommit-cHEAD^-a"+EDITOR=./editorgitcommit-cHEAD^-a" test_expect_success\"message from stdin"\
@@ -141,10 +141,10 @@ EOF test_expect_success\'editor not invoked if -F is given''echo"moo">file&&-VISUAL=./editorgitcommit-a-Fmsg&&+EDITOR=./editorgitcommit-a-Fmsg&&gitshow-s--pretty=format:"%s"|grep-qgood&&echo"quack">file&&-echo"Another good message."|VISUAL=./editorgitcommit-a-F-&&+echo"Another good message."|EDITOR=./editorgitcommit-a-F-&&gitshow-s--pretty=format:"%s"|grep-qgood'# We could just check the head sha1, but checking each commit makes it---a/t/test-lib.sh+++b/t/test-lib.sh
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5exportGIT_MERGE_VERBOSITYexportGIT_AUTHOR_EMAILGIT_AUTHOR_NAMEexportGIT_COMMITTER_EMAILGIT_COMMITTER_NAME-exportEDITORVISUAL+exportEDITORGIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}# Protect ourselves from common misconfiguration to export
Subject: [PATCH] Do not use VISUAL editor on dumb terminals
Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
or set to "dumb". Traditionally, VISUAL is set to a screen
editor and EDITOR to a line-based editor, which should be more
useful in that situation.
vim, for example, is happy to assume a terminal supports ANSI
sequences even if TERM is dumb (e.g., when running from a text
editor like Acme). git already refuses to fall back to vi on a
dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
unset, but without this patch, that check is suppressed by
VISUAL=vi.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
editor.c | 12 ++++++------
t/t7005-editor.sh | 10 ++++++++++
t/t7501-commit.sh | 8 ++++----
t/test-lib.sh | 8 ++++----
4 files changed, 24 insertions(+), 14 deletions(-)
@@ -4,19 +4,19 @@intlaunch_editor(constchar*path,structstrbuf*buffer,constchar*const*env){-constchar*editor,*terminal;+constchar*editor=getenv("GIT_EDITOR");+constchar*terminal=getenv("TERM");+intterminal_is_dumb=!terminal||!strcmp(terminal,"dumb");-editor=getenv("GIT_EDITOR");if(!editor&&editor_program)editor=editor_program;-if(!editor)+if(!editor&&!terminal_is_dumb)editor=getenv("VISUAL");if(!editor)editor=getenv("EDITOR");-terminal=getenv("TERM");-if(!editor&&(!terminal||!strcmp(terminal,"dumb")))-returnerror("Terminal is dumb but no VISUAL nor EDITOR defined.");+if(!editor&&terminal_is_dumb)+returnerror("terminal is dumb, but EDITOR unset");if(!editor)editor="vi";
@@ -42,6 +42,16 @@ test_expect_success 'dumb should error out when falling back on vi' 'fi'+test_expect_success'dumb should prefer EDITOR to VISUAL''++EDITOR=./e-EDITOR.sh&&+VISUAL=./e-VISUAL.sh&&+exportEDITORVISUAL&&+gitcommit--amend&&+test"$(gitshow-s--format=%s)"="Edited by EDITOR"++'+TERM=vt100exportTERMforiinviEDITORVISUALcore_editorGIT_EDITOR
@@ -107,7 +107,7 @@ chmod 755 editor test_expect_success\"editing message from other commit"\"echo 'hula hula' >file && \-VISUAL=./editorgitcommit-cHEAD^-a"+EDITOR=./editorgitcommit-cHEAD^-a" test_expect_success\"message from stdin"\
@@ -141,10 +141,10 @@ EOF test_expect_success\'editor not invoked if -F is given''echo"moo">file&&-VISUAL=./editorgitcommit-a-Fmsg&&+EDITOR=./editorgitcommit-a-Fmsg&&gitshow-s--pretty=format:"%s"|grep-qgood&&echo"quack">file&&-echo"Another good message."|VISUAL=./editorgitcommit-a-F-&&+echo"Another good message."|EDITOR=./editorgitcommit-a-F-&&gitshow-s--pretty=format:"%s"|grep-qgood'# We could just check the head sha1, but checking each commit makes it
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5exportGIT_MERGE_VERBOSITYexportGIT_AUTHOR_EMAILGIT_AUTHOR_NAMEexportGIT_COMMITTER_EMAILGIT_COMMITTER_NAME-exportEDITORVISUAL+exportEDITORGIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}# Protect ourselves from common misconfiguration to export
--
1.6.5.2
>
> vim, for example, is happy to assume a terminal supports ANSI
> sequences even if TERM is dumb (e.g., when running from a text
> editor like Acme). git already refuses to fall back to vi on a
> dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
> unset, but without this patch, that check is suppressed by
> VISUAL=vi.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> This patch eases my discomfort about the error message a little. It
> is not actually needed to support any ways of working I engage in.
>
> If stdout is redirected, this is probably still making the wrong
> choice; isatty(STDOUT_FILENO) might be a more useful datum to use.
> But it does not seem worth complicating the logic further.
>
> editor.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/editor.c b/editor.c
> index 941c0b2..3f13751 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -4,19 +4,19 @@
>
> int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
> {
> - const char *editor, *terminal;
> + const char *editor = getenv("GIT_EDITOR");
> + const char *terminal = getenv("TERM");
> + int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb");
>
> - editor = getenv("GIT_EDITOR");
> if (!editor && editor_program)
> editor = editor_program;
> - if (!editor)
> + if (!editor && !terminal_is_dumb)
> editor = getenv("VISUAL");
> if (!editor)
> editor = getenv("EDITOR");
>
> - terminal = getenv("TERM");
> - if (!editor && (!terminal || !strcmp(terminal, "dumb")))
> - return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
> + if (!editor && terminal_is_dumb)
> + return error("terminal is dumb, but EDITOR unset");
>
> if (!editor)
> editor = "vi";
> --
> 1.6.5.2
>
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Junio C Hamano wrote:
* jn/editor-pager (2009-10-30) 8 commits
- Provide a build time default-pager setting
- Provide a build time default-editor setting
- am -i, git-svn: use "git var GIT_PAGER"
- add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
- Teach git var about GIT_PAGER
- Teach git var about GIT_EDITOR
- Do not use VISUAL editor on dumb terminals
- Handle more shell metacharacters in editor names
Any comments?
@@ -205,7 +205,7 @@ const char *fmt_ident(const char *name, const char *email,if((warn_on_no_name||error_on_no_name)&&name==git_default_name&&env_hint){fprintf(stderr,env_hint,au_env,co_env);-env_hint=NULL;/* warn only once, for "git var -l" */+env_hint=NULL;/* warn only once */}if(error_on_no_name)die("empty ident %s <%s> not allowed",name,email);
and the corresponding hunk in patch 3 could be safely discarded.
Aside from that, patch 3 has been unsquashed from patch 4, since it is
an independent fix that might be worth ejecting; the Signed-off-by
lines on patches 2 and 4 have been fixed; and the commit message for
patch 4 has been expanded to explain more.
In short, nothing of substance has changed. If you are reminded of
any thoughts on the series, please let me know.
I think it is fair to say every one of these patches except the first
was someone else’s idea. Thanks, everyone.
Jonathan Nieder (8):
Handle more shell metacharacters in editor names
Do not use VISUAL editor on dumb terminals
Suppress warnings from "git var -l"
Teach git var about GIT_EDITOR
Teach git var about GIT_PAGER
add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
am -i, git-svn: use "git var GIT_PAGER"
Provide a build time default-editor setting
Junio C Hamano (1):
Provide a build time default-pager setting
Documentation/config.txt | 4 +--
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 +-
Documentation/git-var.txt | 14 +++++++++++
Makefile | 28 ++++++++++++++++++++++
cache.h | 2 +
contrib/fast-import/git-p4 | 5 +---
editor.c | 32 ++++++++++++++++++-------
git-add--interactive.perl | 3 +-
git-am.sh | 5 +++-
git-send-email.perl | 3 +-
git-sh-setup.sh | 19 +++++----------
git-svn.perl | 11 +++-----
ident.c | 2 +-
pager.c | 24 ++++++++++++++++---
t/t7005-editor.sh | 47 ++++++++++++++++++++++++++++---------
t/t7501-commit.sh | 8 +++---
t/test-lib.sh | 8 +++---
var.c | 26 ++++++++++++++++++++-
19 files changed, 178 insertions(+), 69 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Pass the editor name to the shell if it contains any susv3 shell
special character (globs, redirections, variable substitutions,
escapes, etc). This way, the meaning of some characters will not
meaninglessly change when others are added, and git commands
implemented in C and in shell scripts will interpret editor names
in the same way.
This does not make the GIT_EDITOR setting any more expressive,
since one could always use single quotes to force the editor to
be passed to the shell.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
Unchanged from jn/editor-pager, included only for reference.
editor.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
or set to "dumb". Traditionally, VISUAL is set to a screen
editor and EDITOR to a line-based editor, which should be more
useful in that situation.
vim, for example, is happy to assume a terminal supports ANSI
sequences even if TERM is dumb (e.g., when running from a text
editor like Acme). git already refuses to fall back to vi on a
dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
unset, but without this patch, that check is suppressed by
VISUAL=vi.
Signed-off-by: Jonathan Nieder <redacted>
---
Fixes broken sign-off, patch unchanged.
I am personally most interested in this for usage from a text editor,
but vim does not set TERM=dumb like it probably ought to. A more
realistic everyday example might be "ssh user@domain git commit".
editor.c | 12 ++++++------
t/t7005-editor.sh | 10 ++++++++++
t/t7501-commit.sh | 8 ++++----
t/test-lib.sh | 8 ++++----
4 files changed, 24 insertions(+), 14 deletions(-)
@@ -4,19 +4,19 @@intlaunch_editor(constchar*path,structstrbuf*buffer,constchar*const*env){-constchar*editor,*terminal;+constchar*editor=getenv("GIT_EDITOR");+constchar*terminal=getenv("TERM");+intterminal_is_dumb=!terminal||!strcmp(terminal,"dumb");-editor=getenv("GIT_EDITOR");if(!editor&&editor_program)editor=editor_program;-if(!editor)+if(!editor&&!terminal_is_dumb)editor=getenv("VISUAL");if(!editor)editor=getenv("EDITOR");-terminal=getenv("TERM");-if(!editor&&(!terminal||!strcmp(terminal,"dumb")))-returnerror("Terminal is dumb but no VISUAL nor EDITOR defined.");+if(!editor&&terminal_is_dumb)+returnerror("terminal is dumb, but EDITOR unset");if(!editor)editor="vi";
@@ -42,6 +42,16 @@ test_expect_success 'dumb should error out when falling back on vi' 'fi'+test_expect_success'dumb should prefer EDITOR to VISUAL''++EDITOR=./e-EDITOR.sh&&+VISUAL=./e-VISUAL.sh&&+exportEDITORVISUAL&&+gitcommit--amend&&+test"$(gitshow-s--format=%s)"="Edited by EDITOR"++'+TERM=vt100exportTERMforiinviEDITORVISUALcore_editorGIT_EDITOR
@@ -107,7 +107,7 @@ chmod 755 editor test_expect_success\"editing message from other commit"\"echo 'hula hula' >file && \-VISUAL=./editorgitcommit-cHEAD^-a"+EDITOR=./editorgitcommit-cHEAD^-a" test_expect_success\"message from stdin"\
@@ -141,10 +141,10 @@ EOF test_expect_success\'editor not invoked if -F is given''echo"moo">file&&-VISUAL=./editorgitcommit-a-Fmsg&&+EDITOR=./editorgitcommit-a-Fmsg&&gitshow-s--pretty=format:"%s"|grep-qgood&&echo"quack">file&&-echo"Another good message."|VISUAL=./editorgitcommit-a-F-&&+echo"Another good message."|EDITOR=./editorgitcommit-a-F-&&gitshow-s--pretty=format:"%s"|grep-qgood'# We could just check the head sha1, but checking each commit makes it
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5exportGIT_MERGE_VERBOSITYexportGIT_AUTHOR_EMAILGIT_AUTHOR_NAMEexportGIT_COMMITTER_EMAILGIT_COMMITTER_NAME-exportEDITORVISUAL+exportEDITORGIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}# Protect ourselves from common misconfiguration to export
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
For scripts using "git var -l" to read all logical variables at
once, not all per-variable warnings will be relevant. So suppress
them.
Signed-off-by: Jonathan Nieder <redacted>
---
This is a separate issue that might even deserve to be ejected
from the series.
Changes from jn/editor-pager: unsquashed from the next patch, added
back comment-changing hunk. Of course, there’s no harm in omitting
the comment change, but it describes a change in reality: before this
patch, that code gets run multiple times by "git var -l"; afterwards,
by no one (except possible out-of-tree users).
ident.c | 2 +-
var.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
@@ -205,7 +205,7 @@ const char *fmt_ident(const char *name, const char *email,if((warn_on_no_name||error_on_no_name)&&name==git_default_name&&env_hint){fprintf(stderr,env_hint,au_env,co_env);-env_hint=NULL;/* warn only once, for "git var -l" */+env_hint=NULL;/* warn only once */}if(error_on_no_name)die("empty ident %s <%s> not allowed",name,email);
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.
git_editor(void) uses the logic to decide which editor to use
that used to live in launch_editor(). The function returns NULL
if there is no suitable editor; the caller is expected to issue
an error message when appropriate.
launch_editor() uses git_editor() and gives the error message the
same way as before when EDITOR is not set.
"git var GIT_EDITOR" gives the editor name, or an error message
when there is no appropriate one.
"git var -l" gives GIT_EDITOR=name only if there is an
appropriate editor.
Originally-submitted-by: Johannes Sixt [off-list ref]
Signed-off-by: Jonathan Nieder <redacted>
---
Changes from the version in pu:
* unsquashed with the previous patch;
* replaces Hannes’s sign-off with something more descriptive (see
<http://thread.gmane.org/gmane.comp.version-control.git/131471/focus=131851>);
* nicer commit message based on Junio’s summary.
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 14 ++++++++++++--
var.c | 16 +++++++++++++++-
4 files changed, 36 insertions(+), 3 deletions(-)
@@ -36,6 +36,14 @@ GIT_AUTHOR_IDENT:: GIT_COMMITTER_IDENT:: The person who put a piece of code into git.+GIT_EDITOR::+ Text editor for use by git commands. The value is meant to be+ interpreted by the shell when it is used. Examples: `~/bin/vi`,+ `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"+ --nofork`. The order of preference is the `$GIT_EDITOR`+ environment variable, then `core.editor` configuration, then+ `$VISUAL`, then `$EDITOR`, and then finally 'vi'.+ Diagnostics ----------- You don't exist. Go away!::
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *eneditor=getenv("EDITOR");if(!editor&&terminal_is_dumb)-returnerror("terminal is dumb, but EDITOR unset");+returnNULL;if(!editor)editor="vi";+returneditor;+}++intlaunch_editor(constchar*path,structstrbuf*buffer,constchar*const*env)+{+constchar*editor=git_editor();++if(!editor)+returnerror("Terminal is dumb, but EDITOR unset");+if(strcmp(editor,":")){size_tlen=strlen(editor);inti=0;
@@ -8,6 +8,16 @@staticconstcharvar_usage[]="git var [-l | <variable>]";+staticconstchar*editor(intflag)+{+constchar*pgm=git_editor();++if(!pgm&&flag&IDENT_ERROR_ON_NO_NAME)+die("Terminal is dumb, but EDITOR unset");++returnpgm;+}+structgit_var{constchar*name;constchar*(*read)(int);
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Expose the command found by setup_pager() for scripts to use.
Scripts can use this to avoid repeating the logic to look for a
proper pager in each command.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
The rest of the series is unchanged from pu.
Documentation/git-var.txt | 6 ++++++
cache.h | 1 +
pager.c | 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
@@ -44,6 +44,12 @@ GIT_EDITOR:: environment variable, then `core.editor` configuration, then `$VISUAL`, then `$EDITOR`, and then finally 'vi'.+GIT_PAGER::+ Text viewer for use by git commands (e.g., 'less'). The value+ is meant to be interpreted by the shell. The order of preference+ is the `$GIT_PAGER` environment variable, then `core.pager`+ configuration, then `$PAGER`, and then finally 'less'.+ Diagnostics ----------- You don't exist. Go away!::
@@ -60,6 +62,16 @@ void setup_pager(void)if(!pager)pager="less";elseif(!*pager||!strcmp(pager,"cat"))+pager=NULL;++returnpager;+}++voidsetup_pager(void)+{+constchar*pager=git_pager();++if(!pager)return;spawned_pager=1;/* means we are emitting to terminal */
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Use the new "git var GIT_EDITOR" feature to decide what editor to
use, instead of duplicating its logic elsewhere. This should make
the behavior of commands in edge cases (e.g., editor names with
spaces) a little more consistent.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
Unchanged from pu.
Documentation/config.txt | 4 +---
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 ++--
contrib/fast-import/git-p4 | 5 +----
git-add--interactive.perl | 3 +--
git-send-email.perl | 3 ++-
git-sh-setup.sh | 19 ++++++-------------
git-svn.perl | 5 ++---
8 files changed, 16 insertions(+), 29 deletions(-)
@@ -387,9 +387,7 @@ core.editor:: Commands such as `commit` and `tag` that lets you edit messages by launching an editor uses the value of this variable when it is set, and the environment variable- `GIT_EDITOR` is not set. The order of preference is- `GIT_EDITOR` environment, `core.editor`, `VISUAL` and- `EDITOR` environment variables and then finally `vi`.+ `GIT_EDITOR` is not set. See linkgit:git-var[1]. core.pager:: The command that git will use to paginate output. Can
@@ -323,7 +323,7 @@ ENVIRONMENT AND CONFIGURATION VARIABLES The editor used to edit the commit log message will be chosen from the GIT_EDITOR environment variable, the core.editor configuration variable, the VISUAL environment variable, or the EDITOR environment variable (in that-order).+order). See linkgit:git-var[1] for details. HOOKS -----
@@ -60,8 +60,8 @@ The --bcc option must be repeated for each user you want on the bcc list. The --cc option must be repeated for each user you want on the cc list. --compose::- Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an- introductory message for the patch series.+ Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])+ to edit an introductory message for the patch series. + When '--compose' is used, git send-email will use the From, Subject, and In-Reply-To headers specified in the message. If the body of the message
@@ -987,8 +987,7 @@ sub edit_hunk_manually {EOFclose$fh;-my$editor=$ENV{GIT_EDITOR}||$repo->config("core.editor")-||$ENV{VISUAL}||$ENV{EDITOR}||"vi";+chomp(my$editor=run_cmd_pipe(qw(git var GIT_EDITOR)));system('sh','-c',$editor.' "$@"',$editor,$hunkfile);if($?!=0){
@@ -99,19 +99,12 @@ set_reflog_action() {} git_editor(){-:"${GIT_EDITOR:=$(gitconfigcore.editor)}"-:"${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"-case"$GIT_EDITOR,$TERM"in-,dumb)-echo>&2"No editor specified in GIT_EDITOR, core.editor, VISUAL,"-echo>&2"or EDITOR. Tried to fall back to vi but terminal is dumb."-echo>&2"Please set one of these variables to an appropriate"-echo>&2"editor or run $0 with options that will not cause an"-echo>&2"editor to be invoked (e.g., -m or -F for git-commit)."-exit1-;;-esac-eval"${GIT_EDITOR:=vi}"'"$@"'+iftest-z"${GIT_EDITOR:+set}"+then+GIT_EDITOR="$(gitvarGIT_EDITOR)"||return$?+fi++eval"$GIT_EDITOR"'"$@"'} is_bare_repository(){
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Use the new "git var GIT_PAGER" command to ask what pager to use.
Without this change, the core.pager configuration is ignored by
these commands.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
Unchanged.
git-am.sh | 5 ++++-
git-svn.perl | 6 ++----
2 files changed, 6 insertions(+), 5 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Provide a DEFAULT_EDITOR knob to allow setting the fallback
editor to use instead of vi (when VISUAL, EDITOR, and GIT_EDITOR
are unset). The value can be set at build time according to a
system’s policy. For example, on Debian systems, the default
editor should be the 'editor' command.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <redacted>
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
Unchanged.
Makefile | 17 +++++++++++++++++
editor.c | 6 +++++-
t/t7005-editor.sh | 37 +++++++++++++++++++++++++------------
3 files changed, 47 insertions(+), 13 deletions(-)
@@ -200,6 +200,14 @@ all::# memory allocators with the nedmalloc allocator written by Niall Douglas.## Define NO_REGEX if you have no or inferior regex support in your C library.+#+# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you+# want to use something different. The value will be interpreted by the shell+# if necessary when it is used. Examples:+#+# DEFAULT_EDITOR='~/bin/vi',+# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',+# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'GIT-VERSION-FILE:.FORCE-GIT-VERSION-FILE@$(SHELL_PATH)./GIT-VERSION-GEN
@@ -54,7 +67,7 @@ test_expect_success 'dumb should prefer EDITOR to VISUAL' 'TERM=vt100exportTERM-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin$viEDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectunsetEDITORVISUALGIT_EDITOR
@@ -78,7 +91,7 @@ doneunsetEDITORVISUALGIT_EDITOR gitconfig--unset-allcore.editor-foriinviEDITORVISUALcore_editorGIT_EDITOR+foriin$viEDITORVISUALcore_editorGIT_EDITORdoecho"Edited by $i">expectcase"$i"in
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:47:42
Provide a DEFAULT_PAGER knob so packagers can set the fallback
pager to something appropriate during the build.
Examples:
On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which
forces users to alter their environment (PATH, GIT_PAGER, LESS,
etc) or have a local or global gitconfig before paging works as
expected.
On Debian systems, by policy packages must fall back to the
'pager' command, so that changing the target of the
/usr/bin/pager symlink changes the default pager for all packages
at once.
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Ben Walton <redacted>
Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
As in pu.
Makefile | 11 +++++++++++
pager.c | 6 +++++-
2 files changed, 16 insertions(+), 1 deletions(-)
@@ -201,6 +201,10 @@ all::## Define NO_REGEX if you have no or inferior regex support in your C library.#+# Define DEFAULT_PAGER to a sensible pager command (defaults to "less") if+# you want to use something different. The value will be interpreted by the+# shell at runtime when it is used.+## Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you# want to use something different. The value will be interpreted by the shell# if necessary when it is used. Examples: