* [PATCH 1/8] launch_editor: Longer error message when TERM=dumb
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
@ 2009-10-30 10:20 ` Jonathan Nieder
2009-10-30 10:25 ` [PATCH 2/8] Handle more shell metacharacters in editor names Jonathan Nieder
` (8 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/editor.c b/editor.c
index 4d469d0..316d139 100644
--- a/editor.c
+++ b/editor.c
@@ -16,7 +16,13 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
terminal = getenv("TERM");
if (!editor && (!terminal || !strcmp(terminal, "dumb")))
- return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
+ /* Terminal is dumb but no VISUAL nor EDITOR defined. */
+ return 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).");
if (!editor)
editor = "vi";
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 2/8] Handle more shell metacharacters in editor names
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
2009-10-30 10:20 ` [PATCH 1/8] launch_editor: Longer error message when TERM=dumb Jonathan Nieder
@ 2009-10-30 10:25 ` Jonathan Nieder
2009-10-30 10:26 ` [PATCH 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
` (7 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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<jrnieder@gmail.com>
---
editor.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/editor.c b/editor.c
index 316d139..facd7f2 100644
--- a/editor.c
+++ b/editor.c
@@ -34,7 +34,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
const char *args[6];
struct strbuf arg0 = STRBUF_INIT;
- if (strcspn(editor, "$ \t'") != len) {
+ if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) {
/* there are specials */
strbuf_addf(&arg0, "%s \"$@\"", editor);
args[i++] = "sh";
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 3/8] Teach git var about GIT_EDITOR
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
2009-10-30 10:20 ` [PATCH 1/8] launch_editor: Longer error message when TERM=dumb Jonathan Nieder
2009-10-30 10:25 ` [PATCH 2/8] Handle more shell metacharacters in editor names Jonathan Nieder
@ 2009-10-30 10:26 ` Jonathan Nieder
2009-10-30 20:51 ` Johannes Sixt
2009-10-30 10:29 ` [PATCH 4/8] Teach git var about GIT_PAGER Jonathan Nieder
` (6 subsequent siblings)
9 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
From: Johannes Sixt <j.sixt@viscovery.net>
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 <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index e2f4c09..89e4b4f 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 96840c7..311cfe1 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor(void);
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index facd7f2..9dcf95c 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor, *terminal;
@@ -15,18 +15,30 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
editor = 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. */
- 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;
+ }
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 (!editor)
+ return -1;
+
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index 125c0d1..342dc2c 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,15 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int flag)
+{
+ const char *pgm = git_editor();
+
+ if (!pgm && (flag & IDENT_ERROR_ON_NO_NAME))
+ die("cannot find a suitable editor");
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,6 +24,7 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
2009-10-30 10:26 ` [PATCH 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
@ 2009-10-30 20:51 ` Johannes Sixt
2009-10-30 22:47 ` Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Johannes Sixt @ 2009-10-30 20:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
Jonathan Nieder schrieb:
> From: Johannes Sixt <j.sixt@viscovery.net>
>
> 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 <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
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 <j6t@kdbg.org>
> -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
> +static const char *editor(int flag)
> +{
> + const char *pgm = git_editor();
> +
> + if (!pgm && (flag & IDENT_ERROR_ON_NO_NAME))
> + die("cannot find a suitable editor");
> + return pgm;
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
^ permalink raw reply [flat|nested] 65+ messages in thread* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
2009-10-30 20:51 ` Johannes Sixt
@ 2009-10-30 22:47 ` Jonathan Nieder
2009-10-30 22:43 ` Junio C Hamano
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 22:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
Johannes Sixt wrote:
> Jonathan Nieder schrieb:
>> From: Johannes Sixt <j.sixt@viscovery.net>
[...]
> 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 <j6t@kdbg.org>
Thanks for the catch.
>>+ 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
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
2009-10-30 22:47 ` Jonathan Nieder
@ 2009-10-30 22:43 ` Junio C Hamano
2009-10-31 0:01 ` Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2009-10-30 22:43 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Sixt, Junio C Hamano, Ben Walton, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> 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.
More importantly, that is what people traditionally expected from VISUAL
and EDITOR and we do that only to follow suit.
There is no such tradition for GIT_EDITOR nor core.editor and switching
behaviour based on the name of editor ("vi"? "vim"?...) does not feel
quite right.
^ permalink raw reply [flat|nested] 65+ messages in thread* Re: [PATCH 3/8] Teach git var about GIT_EDITOR
2009-10-30 22:43 ` Junio C Hamano
@ 2009-10-31 0:01 ` Jonathan Nieder
0 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 0:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ben Walton, David Roundy, GIT List
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> 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.
>
> More importantly, that is what people traditionally expected from VISUAL
> and EDITOR and we do that only to follow suit.
Unfortunately, we don’t do that: we currently still use $VISUAL if
TERM=dumb and just refuse to fall back to vi in that case. I’ll add a
patch to fix this.
> There is no such tradition for GIT_EDITOR nor core.editor
Makes sense.
Jonathan
^ permalink raw reply [flat|nested] 65+ messages in thread
* [PATCH 4/8] Teach git var about GIT_PAGER
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (2 preceding siblings ...)
2009-10-30 10:26 ` [PATCH 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
@ 2009-10-30 10:29 ` Jonathan Nieder
2009-10-30 10:32 ` [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR" Jonathan Nieder
` (5 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
Documentation/git-var.txt | 6 ++++++
cache.h | 1 +
| 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 89e4b4f..ef6aa81 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 311cfe1..5aaa4ba 100644
--- a/cache.h
+++ b/cache.h
@@ -751,6 +751,7 @@ extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
extern const char *git_editor(void);
+extern const char *git_pager(void);
struct checkout {
const char *base_dir;
--git a/pager.c b/pager.c
index 86facec..0b63d99 100644
--- a/pager.c
+++ b/pager.c
@@ -44,12 +44,14 @@ static void wait_for_pager_signal(int signo)
raise(signo);
}
-void setup_pager(void)
+const char *git_pager(void)
{
- const char *pager = getenv("GIT_PAGER");
+ const char *pager;
if (!isatty(1))
- return;
+ return NULL;
+
+ pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
git_config(git_default_config, NULL);
@@ -60,6 +62,16 @@ void setup_pager(void)
if (!pager)
pager = "less";
else if (!*pager || !strcmp(pager, "cat"))
+ pager = NULL;
+
+ return pager;
+}
+
+void setup_pager(void)
+{
+ const char *pager = git_pager();
+
+ if (!pager)
return;
spawned_pager = 1; /* means we are emitting to terminal */
diff --git a/var.c b/var.c
index 342dc2c..18dad57 100644
--- a/var.c
+++ b/var.c
@@ -17,6 +17,15 @@ static const char *editor(int flag)
return pgm;
}
+static const char *pager(int flag)
+{
+ const char *pgm = git_pager();
+
+ if (!pgm)
+ pgm = "cat";
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -25,6 +34,7 @@ static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
{ "GIT_EDITOR", editor },
+ { "GIT_PAGER", pager },
{ "", NULL },
};
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (3 preceding siblings ...)
2009-10-30 10:29 ` [PATCH 4/8] Teach git var about GIT_PAGER Jonathan Nieder
@ 2009-10-30 10:32 ` Jonathan Nieder
2009-10-30 10:33 ` [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER" Jonathan Nieder
` (4 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d1e2120..5181b77 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -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
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..3ea80c8 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -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
-----
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 767cf4d..c85d7f4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -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
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index e710219..48059d0 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -729,13 +729,10 @@ class P4Submit(Command):
tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
tmpFile.close()
mtime = os.stat(fileName).st_mtime
- defaultEditor = "vi"
- if platform.system() == "Windows":
- defaultEditor = "notepad"
if os.environ.has_key("P4EDITOR"):
editor = os.environ.get("P4EDITOR")
else:
- editor = os.environ.get("EDITOR", defaultEditor);
+ editor = read_pipe("git var GIT_EDITOR")
system(editor + " " + fileName)
response = "y"
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 69aeaf0..0c74e5c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -987,8 +987,7 @@ sub edit_hunk_manually {
EOF
close $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) {
diff --git a/git-send-email.perl b/git-send-email.perl
index a0279de..4f5da4e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -162,7 +162,8 @@ my $compose_filename;
# Handle interactive edition of files.
my $multiedit;
-my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+my $editor = Git::command_oneline('var', 'GIT_EDITOR');
+
sub do_edit {
if (defined($multiedit) && !$multiedit) {
map {
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..99cceeb 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -99,19 +99,12 @@ set_reflog_action() {
}
git_editor() {
- : "${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 "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}" '"$@"'
+ if test -z "${GIT_EDITOR:+set}"
+ then
+ GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
+ fi
+
+ eval "$GIT_EDITOR" '"$@"'
}
is_bare_repository () {
diff --git a/git-svn.perl b/git-svn.perl
index 6a3b501..42c9a72 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1321,9 +1321,8 @@ sub get_commit_entry {
close $log_fh or croak $!;
if ($_edit || ($type eq 'tree')) {
- my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
- # TODO: strip out spaces, comments, like git-commit.sh
- system($editor, $commit_editmsg);
+ chomp(my $editor = command_oneline(qw(var GIT_EDITOR)));
+ system('sh', '-c', $editor.' "$@"', $editor, $commit_editmsg);
}
rename $commit_editmsg, $commit_msg or croak $!;
{
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER"
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (4 preceding siblings ...)
2009-10-30 10:32 ` [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR" Jonathan Nieder
@ 2009-10-30 10:33 ` Jonathan Nieder
2009-10-30 10:35 ` [PATCH 7/8] Provide a build time default-editor setting Jonathan Nieder
` (3 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
git-am.sh | 5 ++++-
git-svn.perl | 6 ++----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c132f50..2649487 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -649,7 +649,10 @@ do
[eE]*) git_editor "$dotest/final-commit"
action=again ;;
[vV]*) action=again
- LESS=-S ${PAGER:-less} "$dotest/patch" ;;
+ : ${GIT_PAGER=$(git var GIT_PAGER)}
+ : ${LESS=-FRSX}
+ export LESS
+ $GIT_PAGER "$dotest/patch" ;;
*) action=again ;;
esac
done
diff --git a/git-svn.perl b/git-svn.perl
index 42c9a72..c4ca548 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5171,10 +5171,8 @@ sub git_svn_log_cmd {
# adapted from pager.c
sub config_pager {
- $pager ||= $ENV{GIT_PAGER} || $ENV{PAGER};
- if (!defined $pager) {
- $pager = 'less';
- } elsif (length $pager == 0 || $pager eq 'cat') {
+ chomp(my $pager = command_oneline(qw(var GIT_PAGER)));
+ if ($pager eq 'cat') {
$pager = undef;
}
$ENV{GIT_PAGER_IN_USE} = defined($pager);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 7/8] Provide a build time default-editor setting
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (5 preceding siblings ...)
2009-10-30 10:33 ` [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER" Jonathan Nieder
@ 2009-10-30 10:35 ` Jonathan Nieder
2009-10-30 13:17 ` Jonathan Nieder
2009-10-30 10:39 ` [PATCH 8/8] Provide a build time default-pager setting Jonathan Nieder
` (2 subsequent siblings)
9 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 17 +++++++++++++++++
editor.c | 11 ++++++++---
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
3 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/Makefile b/Makefile
index 268aede..625866c 100644
--- a/Makefile
+++ b/Makefile
@@ -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
@@ -1363,6 +1371,15 @@ BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
$(COMPAT_CFLAGS)
LIB_OBJS += $(COMPAT_OBJS)
+# Quote for C
+
+ifdef DEFAULT_EDITOR
+DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))"
+DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
diff --git a/editor.c b/editor.c
index 9dcf95c..fcf35a8 100644
--- a/editor.c
+++ b/editor.c
@@ -2,6 +2,10 @@
#include "strbuf.h"
#include "run-command.h"
+#ifndef DEFAULT_EDITOR
+#define DEFAULT_EDITOR "vi"
+#endif
+
const char *git_editor(void)
{
const char *editor, *terminal;
@@ -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);
return NULL;
}
if (!editor)
- editor = "vi";
+ editor = DEFAULT_EDITOR;
return editor;
}
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..73ba44c 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,26 @@ test_description='GIT_EDITOR, core.editor, and stuff'
. ./test-lib.sh
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'does editor have a simple name (no slashes, etc)?' '
+
+ editor=$(TERM=vt100 git var GIT_EDITOR) &&
+ test -n "$editor" &&
+ simple=t &&
+ case "$editor" in
+ */* | core_editor | [A-Z]*)
+ unset simple;;
+ esac
+
+'
+if test -z "${simple+set}"
+then
+ say 'skipping editor tests, default editor is not sought on PATH'
+ test_done
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL "$editor"
do
cat >e-$i.sh <<-EOF
#!$SHELL_PATH
@@ -12,15 +31,13 @@ do
EOF
chmod +x e-$i.sh
done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+mv "e-$editor.sh" "$editor"
test_expect_success setup '
msg="Hand edited" &&
echo "$msg" >expect &&
- git add vi &&
+ git add "$editor" &&
test_tick &&
git commit -m "$msg" &&
git show -s --pretty=oneline |
@@ -44,7 +61,7 @@ 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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -68,7 +85,7 @@ 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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 8/8] Provide a build time default-pager setting
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (6 preceding siblings ...)
2009-10-30 10:35 ` [PATCH 7/8] Provide a build time default-editor setting Jonathan Nieder
@ 2009-10-30 10:39 ` Jonathan Nieder
2009-10-30 22:59 ` Junio C Hamano
2009-10-30 10:49 ` [PATCH/RFC 9/8] Teach git var to run the editor Jonathan Nieder
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
9 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
From: Junio C Hamano <gitster@pobox.com>
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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 +++++++++++
| 6 +++++-
2 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 625866c..18fc50a 100644
--- a/Makefile
+++ b/Makefile
@@ -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:
@@ -1380,6 +1384,13 @@ DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
endif
+ifdef DEFAULT_PAGER
+DEFAULT_PAGER_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_PAGER)))"
+DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
--git a/pager.c b/pager.c
index 0b63d99..92c03f6 100644
--- a/pager.c
+++ b/pager.c
@@ -2,6 +2,10 @@
#include "run-command.h"
#include "sigchain.h"
+#ifndef DEFAULT_PAGER
+#define DEFAULT_PAGER "less"
+#endif
+
/*
* This is split up from the rest of git so that we can do
* something different on Windows.
@@ -60,7 +64,7 @@ const char *git_pager(void)
if (!pager)
pager = getenv("PAGER");
if (!pager)
- pager = "less";
+ pager = DEFAULT_PAGER;
else if (!*pager || !strcmp(pager, "cat"))
pager = NULL;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH/RFC 9/8] Teach git var to run the editor
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (7 preceding siblings ...)
2009-10-30 10:39 ` [PATCH 8/8] Provide a build time default-pager setting Jonathan Nieder
@ 2009-10-30 10:49 ` Jonathan Nieder
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-30 10:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Expose the functionality of launch_editor() for scripts to use.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index ef6aa81..1bfdb6c 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -8,7 +8,10 @@ git-var - Show a git logical variable
SYNOPSIS
--------
-'git var' [ -l | <variable> ]
+[verse]
+'git var' <variable>
+'git var' -l
+'git var' --run <variable> [ args ]
DESCRIPTION
-----------
@@ -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
diff --git a/var.c b/var.c
index 18dad57..c97b2e6 100644
--- a/var.c
+++ b/var.c
@@ -6,7 +6,8 @@
#include "cache.h"
#include "exec_cmd.h"
-static const char var_usage[] = "git var [-l | <variable>]";
+static const char var_usage[] =
+ "git var { -l | <variable> | --run <variable> [args] }";
static const char *editor(int flag)
{
@@ -26,16 +27,25 @@ static const char *pager(int flag)
return pgm;
}
+static int run_editor(int argc, const char *const *argv)
+{
+ if (argc > 1)
+ return error("cannot launch editor with more than one file");
+
+ return launch_editor(argv[0], NULL, NULL);
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
+ int (*run)(int argc, const char *const *argv);
};
static struct git_var git_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 },
};
static void list_vars(void)
@@ -59,6 +69,17 @@ static const char *read_var(const char *var)
return val;
}
+static int run_var_cmd(const char *var, int argc, char **argv)
+{
+ struct git_var *ptr;
+
+ for (ptr = git_vars; ptr->read; ptr++)
+ if (ptr->run && strcmp(var, ptr->name) == 0)
+ return ptr->run(argc, (const char *const *)argv);
+
+ return error("%s is not a variable command", var);
+}
+
static int show_config(const char *var, const char *value, void *cb)
{
if (value)
@@ -72,12 +93,23 @@ int main(int argc, char **argv)
{
const char *val;
int nongit;
+
+ 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);
+
+ return run_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;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH v3 0/8] Default pager and editor
2009-10-30 10:16 ` [PATCH v2 0/8] Default pager and editor Jonathan Nieder
` (8 preceding siblings ...)
2009-10-30 10:49 ` [PATCH/RFC 9/8] Teach git var to run the editor Jonathan Nieder
@ 2009-10-31 1:20 ` Jonathan Nieder
2009-10-31 1:24 ` [PATCH 1/8] Handle more shell metacharacters in editor names Jonathan Nieder
` (8 more replies)
9 siblings, 9 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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(-)
^ permalink raw reply [flat|nested] 65+ messages in thread* [PATCH 1/8] Handle more shell metacharacters in editor names
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
@ 2009-10-31 1:24 ` Jonathan Nieder
2009-10-31 1:30 ` [PATCH 2/8] Do not use VISUAL editor on dumb terminals Jonathan Nieder
` (7 subsequent siblings)
8 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
Unchanged from v2.
editor.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/editor.c b/editor.c
index 4d469d0..941c0b2 100644
--- a/editor.c
+++ b/editor.c
@@ -28,7 +28,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
const char *args[6];
struct strbuf arg0 = STRBUF_INIT;
- if (strcspn(editor, "$ \t'") != len) {
+ if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) {
/* there are specials */
strbuf_addf(&arg0, "%s \"$@\"", editor);
args[i++] = "sh";
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 2/8] Do not use VISUAL editor on dumb terminals
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
2009-10-31 1:24 ` [PATCH 1/8] Handle more shell metacharacters in editor names Jonathan Nieder
@ 2009-10-31 1:30 ` Jonathan Nieder
2009-10-31 7:46 ` [PATCH v2 " Jonathan Nieder
2009-10-31 1:39 ` [PATCH v2 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
` (6 subsequent siblings)
8 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <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
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH v2 2/8] Do not use VISUAL editor on dumb terminals
2009-10-31 1:30 ` [PATCH 2/8] Do not use VISUAL editor on dumb terminals Jonathan Nieder
@ 2009-10-31 7:46 ` Jonathan Nieder
0 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 7:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 &&
+ export EDITOR VISUAL &&
+ git commit --amend &&
+ test "$(git show -s --format=%s)" = "Edited by EDITOR"
+
+'
+
TERM=vt100
export TERM
for i in vi EDITOR VISUAL core_editor GIT_EDITOR
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -86,7 +86,7 @@ chmod 755 editor
test_expect_success \
"amend commit" \
- "VISUAL=./editor git commit --amend"
+ "EDITOR=./editor git commit --amend"
test_expect_success \
"passing -m and -F" \
@@ -107,7 +107,7 @@ chmod 755 editor
test_expect_success \
"editing message from other commit" \
"echo 'hula hula' >file && \
- VISUAL=./editor git commit -c HEAD^ -a"
+ EDITOR=./editor git commit -c HEAD^ -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=./editor git commit -a -F msg &&
+ EDITOR=./editor git commit -a -F msg &&
git show -s --pretty=format:"%s" | grep -q good &&
echo "quack" >file &&
- echo "Another good message." | VISUAL=./editor git commit -a -F - &&
+ echo "Another good message." | EDITOR=./editor git commit -a -F - &&
git show -s --pretty=format:"%s" | grep -q good
'
# We could just check the head sha1, but checking each commit makes it
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -30,7 +30,7 @@ TZ=UTC
TERM=dumb
export LANG LC_ALL PAGER TERM TZ
EDITOR=:
-VISUAL=:
+unset VISUAL
unset GIT_EDITOR
unset AUTHOR_DATE
unset AUTHOR_EMAIL
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5
export GIT_MERGE_VERBOSITY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
-export EDITOR VISUAL
+export EDITOR
GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# Protect ourselves from common misconfiguration to export
@@ -207,8 +207,8 @@ trap 'die' EXIT
test_set_editor () {
FAKE_EDITOR="$1"
export FAKE_EDITOR
- VISUAL='"$FAKE_EDITOR"'
- export VISUAL
+ EDITOR='"$FAKE_EDITOR"'
+ export EDITOR
}
test_tick () {
-- %< --
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 <jrnieder@gmail.com>
Signed-off-by: Jonathan Nieder <jrn@progeny.tock>
---
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(-)
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";
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..a95fe19 100755
--- 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 &&
+ export EDITOR VISUAL &&
+ git commit --amend &&
+ test "$(git show -s --format=%s)" = "Edited by EDITOR"
+
+'
+
TERM=vt100
export TERM
for i in vi EDITOR VISUAL core_editor GIT_EDITOR
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d2de576..a603f6d 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -86,7 +86,7 @@ chmod 755 editor
test_expect_success \
"amend commit" \
- "VISUAL=./editor git commit --amend"
+ "EDITOR=./editor git commit --amend"
test_expect_success \
"passing -m and -F" \
@@ -107,7 +107,7 @@ chmod 755 editor
test_expect_success \
"editing message from other commit" \
"echo 'hula hula' >file && \
- VISUAL=./editor git commit -c HEAD^ -a"
+ EDITOR=./editor git commit -c HEAD^ -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=./editor git commit -a -F msg &&
+ EDITOR=./editor git commit -a -F msg &&
git show -s --pretty=format:"%s" | grep -q good &&
echo "quack" >file &&
- echo "Another good message." | VISUAL=./editor git commit -a -F - &&
+ echo "Another good message." | EDITOR=./editor git commit -a -F - &&
git show -s --pretty=format:"%s" | grep -q good
'
# We could just check the head sha1, but checking each commit makes it
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..ec3336a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -30,7 +30,7 @@ TZ=UTC
TERM=dumb
export LANG LC_ALL PAGER TERM TZ
EDITOR=:
-VISUAL=:
+unset VISUAL
unset GIT_EDITOR
unset AUTHOR_DATE
unset AUTHOR_EMAIL
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5
export GIT_MERGE_VERBOSITY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
-export EDITOR VISUAL
+export EDITOR
GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# Protect ourselves from common misconfiguration to export
@@ -207,8 +207,8 @@ trap 'die' EXIT
test_set_editor () {
FAKE_EDITOR="$1"
export FAKE_EDITOR
- VISUAL='"$FAKE_EDITOR"'
- export VISUAL
+ EDITOR='"$FAKE_EDITOR"'
+ export EDITOR
}
test_tick () {
--
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
>
^ permalink raw reply related [flat|nested] 65+ messages in thread
* [PATCH v2 3/8] Teach git var about GIT_EDITOR
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
2009-10-31 1:24 ` [PATCH 1/8] Handle more shell metacharacters in editor names Jonathan Nieder
2009-10-31 1:30 ` [PATCH 2/8] Do not use VISUAL editor on dumb terminals Jonathan Nieder
@ 2009-10-31 1:39 ` Jonathan Nieder
2009-10-31 2:01 ` Junio C Hamano
2009-10-31 19:40 ` [PATCH v2 3/8] " Johannes Sixt
2009-10-31 1:41 ` [PATCH 4/8] Teach git var about GIT_PAGER Jonathan Nieder
` (5 subsequent siblings)
8 siblings, 2 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
From: Johannes Sixt <j6t@kdbg.org>
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 <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 14 ++++++++++++--
var.c | 21 ++++++++++++++++++++-
4 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index e2f4c09..89e4b4f 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 96840c7..311cfe1 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor(void);
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index 3f13751..4f98b72 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
const char *terminal = getenv("TERM");
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
editor = getenv("EDITOR");
if (!editor && terminal_is_dumb)
- return error("terminal is dumb, but EDITOR unset");
+ return 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 (!editor)
+ return error("terminal is dumb, but EDITOR unset");
+
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index 125c0d1..399f409 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,21 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int flag)
+{
+ const char *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");
+ }
+
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,14 +30,18 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
static void list_vars(void)
{
struct git_var *ptr;
+ const char *val;
+
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(IDENT_WARN_ON_NO_NAME));
+ if ((val = ptr->read(IDENT_WARN_ON_NO_NAME)))
+ printf("%s=%s\n", ptr->name, val);
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
2009-10-31 1:39 ` [PATCH v2 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
@ 2009-10-31 2:01 ` Junio C Hamano
2009-10-31 2:23 ` Jonathan Nieder
2009-10-31 19:40 ` [PATCH v2 3/8] " Johannes Sixt
1 sibling, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2009-10-31 2:01 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Ben Walton, Johannes Sixt, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> From: Johannes Sixt <j6t@kdbg.org>
>
> 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
Sorry, I cannot grok this example. Is it supposed to be a transcript
of a user session? What did the user type?
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
2009-10-31 2:01 ` Junio C Hamano
@ 2009-10-31 2:23 ` Jonathan Nieder
2009-10-31 2:34 ` Junio C Hamano
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 2:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Junio C Hamano wrote:
>> 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
>
> Sorry, I cannot grok this example. Is it supposed to be a transcript
> of a user session? What did the user type?
Oh, sorry about that. The user typed 'git var -l', and that is all
output from that. More realistic examples:
$ # what scripts see
$ git var -l 2>/dev/null
gc.auto=0
rerere.enabled
merge.log
merge.conflictstyle=diff3
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
remote.origin.url=git://repo.or.cz/git
branch.master.remote=origin
branch.master.merge=refs/heads/master
GIT_COMMITTER_IDENT=user <user@domain> 1256952739 -0500
GIT_AUTHOR_IDENT=user <user@domain> 1256952739 -0500
$
$ # what scripts pass on to the user
$ git var -l >/dev/null
*** 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.
warning: GIT_EDITOR: terminal is dumb, but EDITOR unset
$
At least, that is what I was imagining (that’s one way to use git var,
anyway).
Would a more friendly message be helpful here? I am not sure how 'git
var -l' gets used. I never liked using it directly myself, mostly
because the long list of configuration items can be overwhelming.
Jonathan
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
2009-10-31 2:23 ` Jonathan Nieder
@ 2009-10-31 2:34 ` Junio C Hamano
2009-10-31 4:00 ` Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2009-10-31 2:34 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>
>>> 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
>>
>> Sorry, I cannot grok this example. Is it supposed to be a transcript
>> of a user session? What did the user type?
>
> Oh, sorry about that. The user typed 'git var -l', and that is all
> output from that. More realistic examples:
>
> $ # what scripts see
> $ git var -l 2>/dev/null
> gc.auto=0
> rerere.enabled
> merge.log
> merge.conflictstyle=diff3
> core.repositoryformatversion=0
> core.filemode=true
> core.bare=false
> core.logallrefupdates=true
> remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
> remote.origin.url=git://repo.or.cz/git
> branch.master.remote=origin
> branch.master.merge=refs/heads/master
> GIT_COMMITTER_IDENT=user <user@domain> 1256952739 -0500
> GIT_AUTHOR_IDENT=user <user@domain> 1256952739 -0500
> $
> $ # what scripts pass on to the user
> $ git var -l >/dev/null
>
> *** 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.
>
> warning: GIT_EDITOR: terminal is dumb, but EDITOR unset
> $
This is more readable.
But the user did not even ask for GIT_EDITOR. Should it even mention
"unusable"? or should it just say something like
GIT_EDITOR=
without complaining?
For that matter, I also wonder if we can squelch the user.email one when
we are only listing the variables (I know it is not part of this topic,
but I can still wonder).
> Would a more friendly message be helpful here? I am not sure how 'git
> var -l' gets used. I never liked using it directly myself, mostly
> because the long list of configuration items can be overwhelming.
I think people run "git var -l", store the results in variables (think
Perl or Python script) and read from there, instead of making separate
invocations of "git var" for individual variables.
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
2009-10-31 2:34 ` Junio C Hamano
@ 2009-10-31 4:00 ` Jonathan Nieder
2009-10-31 4:04 ` [PATCH v3] " Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 4:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Junio C Hamano wrote:
> But the user did not even ask for GIT_EDITOR. Should it even mention
> "unusable"? or should it just say something like
>
> GIT_EDITOR=
>
> without complaining?
>
> For that matter, I also wonder if we can squelch the user.email one when
> we are only listing the variables (I know it is not part of this topic,
> but I can still wonder).
[...]
> I think people run "git var -l", store the results in variables (think
> Perl or Python script) and read from there, instead of making separate
> invocations of "git var" for individual variables.
In that case, most variable-specific warnings should be suppressed as
irrelevant. So squelching the warnings makes sense.
How about this patch? With the "git var GIT_EDITOR" patch applied on
top, "git var -l" silently omits the GIT_EDITOR variable when a suitable
editor cannot be found.
-- %< --
Subject: Suppress warnings from "git var -l"
For scripts using "git var -l" to read all logical variables at
once, not all per-variable warnings will be relevant. Suppress
them.
The git source tree does not include any scripts using "git var
-l", so this change should not affect other git commands.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
ident.c | 2 +-
var.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ident.c b/ident.c
index 99f1c85..26409b2 100644
--- a/ident.c
+++ b/ident.c
@@ -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);
diff --git a/var.c b/var.c
index 125c0d1..dacbaab 100644
--- a/var.c
+++ b/var.c
@@ -22,7 +22,7 @@ static void list_vars(void)
{
struct git_var *ptr;
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(IDENT_WARN_ON_NO_NAME));
+ printf("%s=%s\n", ptr->name, ptr->read(0));
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH v3] Teach git var about GIT_EDITOR
2009-10-31 4:00 ` Jonathan Nieder
@ 2009-10-31 4:04 ` Jonathan Nieder
2009-10-31 4:53 ` Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 4:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
From: Johannes Sixt <j6t@kdbg.org>
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 <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:
> How about this patch? With the "git var GIT_EDITOR" patch applied on
> top, "git var -l" silently omits the GIT_EDITOR variable when a suitable
> editor cannot be found.
>
> -- %< --
> Subject: Suppress warnings from "git var -l"
>
> For scripts using "git var -l" to read all logical variables at
> once, not all per-variable warnings will be relevant. Suppress
> them.
Here’s the "git var GIT_EDITOR" patch again, rebased on top of the
aforementioned patch. The rest of the series should apply without
changes.
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 14 ++++++++++++--
var.c | 16 +++++++++++++++-
4 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index e2f4c09..89e4b4f 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 96840c7..311cfe1 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor(void);
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index 3f13751..4f98b72 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
const char *terminal = getenv("TERM");
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
editor = getenv("EDITOR");
if (!editor && terminal_is_dumb)
- return error("terminal is dumb, but EDITOR unset");
+ return 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 (!editor)
+ return error("terminal is dumb, but EDITOR unset");
+
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index dacbaab..12a8512 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,16 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int flag)
+{
+ const char *pgm = git_editor();
+
+ if (!pgm && flag & IDENT_ERROR_ON_NO_NAME)
+ die("terminal is dumb, but VISUAL and EDITOR unset");
+
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,14 +25,18 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
static void list_vars(void)
{
struct git_var *ptr;
+ const char *val;
+
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(0));
+ if ((val = ptr->read(0))
+ printf("%s=%s\n", ptr->name, val);
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH v3] Teach git var about GIT_EDITOR
2009-10-31 4:04 ` [PATCH v3] " Jonathan Nieder
@ 2009-10-31 4:53 ` Jonathan Nieder
2009-10-31 7:56 ` [PATCH v4] " Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 4:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
A typo fix --- sorry for the noise.
Jonathan Nieder wrote:
> --- a/var.c
> +++ b/var.c
> @@ -8,6 +8,16 @@
>
> static const char var_usage[] = "git var [-l | <variable>]";
>
> +static const char *editor(int flag)
> +{
> + const char *pgm = git_editor();
> +
> + if (!pgm && flag & IDENT_ERROR_ON_NO_NAME)
> + die("terminal is dumb, but VISUAL and EDITOR unset");
Agh... s/VISUAL and //.
All right, time to sleep. Apologies for all the mistakes, and thanks
for the help catching them.
Kind regards,
Jonathan
^ permalink raw reply [flat|nested] 65+ messages in thread* [PATCH v4] Teach git var about GIT_EDITOR
2009-10-31 4:53 ` Jonathan Nieder
@ 2009-10-31 7:56 ` Jonathan Nieder
2009-11-01 4:29 ` Junio C Hamano
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 7:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
There was another typo in the patch I sent. The paper-bag fix:
diff -u b/var.c b/var.c
--- b/var.c
+++ b/var.c
@@ -35,7 +35,7 @@
const char *val;
for (ptr = git_vars; ptr->read; ptr++)
- if ((val = ptr->read(0))
+ if ((val = ptr->read(0)))
printf("%s=%s\n", ptr->name, val);
}
Here’s an updated patch. This one shouldn’t have any bugs (yeah, right).
Good night again,
Jonathan
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 14 ++++++++++++--
var.c | 16 +++++++++++++++-
4 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index e2f4c09..89e4b4f 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 96840c7..311cfe1 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor(void);
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index 3f13751..4f98b72 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
const char *terminal = getenv("TERM");
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
editor = getenv("EDITOR");
if (!editor && terminal_is_dumb)
- return error("terminal is dumb, but EDITOR unset");
+ return 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 (!editor)
+ return error("terminal is dumb, but EDITOR unset");
+
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index dacbaab..a303757 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,16 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int flag)
+{
+ const char *pgm = git_editor();
+
+ if (!pgm && flag & IDENT_ERROR_ON_NO_NAME)
+ die("terminal is dumb, but EDITOR unset");
+
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,14 +25,18 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
static void list_vars(void)
{
struct git_var *ptr;
+ const char *val;
+
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(0));
+ if ((val = ptr->read(0)))
+ printf("%s=%s\n", ptr->name, val);
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH v4] Teach git var about GIT_EDITOR
2009-10-31 7:56 ` [PATCH v4] " Jonathan Nieder
@ 2009-11-01 4:29 ` Junio C Hamano
0 siblings, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2009-11-01 4:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> 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.
Ok, so the idea is...
* 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.
The above all look sensible, but IIRC, the true "vi" fell back on "ex"
mode on dumb terminals and was usable as a line editor, so we should be
able to run it even on dumb terminals. I do not know about vi-clones
though.
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
2009-10-31 1:39 ` [PATCH v2 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
2009-10-31 2:01 ` Junio C Hamano
@ 2009-10-31 19:40 ` Johannes Sixt
1 sibling, 0 replies; 65+ messages in thread
From: Johannes Sixt @ 2009-10-31 19:40 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
Jonathan Nieder schrieb:
> From: Johannes Sixt <j6t@kdbg.org>
>
> Expose the command used by launch_editor() for scripts to use...
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
This patch has grown far beyond my original submission. I can't validly
sign it off anymore. Please take authorship ;)
-- Hannes
^ permalink raw reply [flat|nested] 65+ messages in thread
* [PATCH 4/8] Teach git var about GIT_PAGER
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
` (2 preceding siblings ...)
2009-10-31 1:39 ` [PATCH v2 3/8] Teach git var about GIT_EDITOR Jonathan Nieder
@ 2009-10-31 1:41 ` Jonathan Nieder
2009-10-31 1:42 ` [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR" Jonathan Nieder
` (4 subsequent siblings)
8 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
No changes from the last version sent.
Documentation/git-var.txt | 6 ++++++
cache.h | 1 +
| 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 89e4b4f..ef6aa81 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 311cfe1..5aaa4ba 100644
--- a/cache.h
+++ b/cache.h
@@ -751,6 +751,7 @@ extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
extern const char *git_editor(void);
+extern const char *git_pager(void);
struct checkout {
const char *base_dir;
--git a/pager.c b/pager.c
index 86facec..0b63d99 100644
--- a/pager.c
+++ b/pager.c
@@ -44,12 +44,14 @@ static void wait_for_pager_signal(int signo)
raise(signo);
}
-void setup_pager(void)
+const char *git_pager(void)
{
- const char *pager = getenv("GIT_PAGER");
+ const char *pager;
if (!isatty(1))
- return;
+ return NULL;
+
+ pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
git_config(git_default_config, NULL);
@@ -60,6 +62,16 @@ void setup_pager(void)
if (!pager)
pager = "less";
else if (!*pager || !strcmp(pager, "cat"))
+ pager = NULL;
+
+ return pager;
+}
+
+void setup_pager(void)
+{
+ const char *pager = git_pager();
+
+ if (!pager)
return;
spawned_pager = 1; /* means we are emitting to terminal */
diff --git a/var.c b/var.c
index 399f409..facec11 100644
--- a/var.c
+++ b/var.c
@@ -23,6 +23,15 @@ static const char *editor(int flag)
return pgm;
}
+static const char *pager(int flag)
+{
+ const char *pgm = git_pager();
+
+ if (!pgm)
+ pgm = "cat";
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -31,6 +40,7 @@ static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
{ "GIT_EDITOR", editor },
+ { "GIT_PAGER", pager },
{ "", NULL },
};
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
` (3 preceding siblings ...)
2009-10-31 1:41 ` [PATCH 4/8] Teach git var about GIT_PAGER Jonathan Nieder
@ 2009-10-31 1:42 ` Jonathan Nieder
2009-10-31 1:43 ` [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER" Jonathan Nieder
` (3 subsequent siblings)
8 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d1e2120..5181b77 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -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
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..3ea80c8 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -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
-----
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 767cf4d..c85d7f4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -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
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index e710219..48059d0 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -729,13 +729,10 @@ class P4Submit(Command):
tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
tmpFile.close()
mtime = os.stat(fileName).st_mtime
- defaultEditor = "vi"
- if platform.system() == "Windows":
- defaultEditor = "notepad"
if os.environ.has_key("P4EDITOR"):
editor = os.environ.get("P4EDITOR")
else:
- editor = os.environ.get("EDITOR", defaultEditor);
+ editor = read_pipe("git var GIT_EDITOR")
system(editor + " " + fileName)
response = "y"
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 69aeaf0..0c74e5c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -987,8 +987,7 @@ sub edit_hunk_manually {
EOF
close $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) {
diff --git a/git-send-email.perl b/git-send-email.perl
index a0279de..4f5da4e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -162,7 +162,8 @@ my $compose_filename;
# Handle interactive edition of files.
my $multiedit;
-my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+my $editor = Git::command_oneline('var', 'GIT_EDITOR');
+
sub do_edit {
if (defined($multiedit) && !$multiedit) {
map {
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..99cceeb 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -99,19 +99,12 @@ set_reflog_action() {
}
git_editor() {
- : "${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 "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}" '"$@"'
+ if test -z "${GIT_EDITOR:+set}"
+ then
+ GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
+ fi
+
+ eval "$GIT_EDITOR" '"$@"'
}
is_bare_repository () {
diff --git a/git-svn.perl b/git-svn.perl
index 6a3b501..42c9a72 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1321,9 +1321,8 @@ sub get_commit_entry {
close $log_fh or croak $!;
if ($_edit || ($type eq 'tree')) {
- my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
- # TODO: strip out spaces, comments, like git-commit.sh
- system($editor, $commit_editmsg);
+ chomp(my $editor = command_oneline(qw(var GIT_EDITOR)));
+ system('sh', '-c', $editor.' "$@"', $editor, $commit_editmsg);
}
rename $commit_editmsg, $commit_msg or croak $!;
{
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER"
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
` (4 preceding siblings ...)
2009-10-31 1:42 ` [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR" Jonathan Nieder
@ 2009-10-31 1:43 ` Jonathan Nieder
2009-10-31 1:44 ` [PATCH 7/8] Provide a build time default-editor setting Jonathan Nieder
` (2 subsequent siblings)
8 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
git-am.sh | 5 ++++-
git-svn.perl | 6 ++----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c132f50..2649487 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -649,7 +649,10 @@ do
[eE]*) git_editor "$dotest/final-commit"
action=again ;;
[vV]*) action=again
- LESS=-S ${PAGER:-less} "$dotest/patch" ;;
+ : ${GIT_PAGER=$(git var GIT_PAGER)}
+ : ${LESS=-FRSX}
+ export LESS
+ $GIT_PAGER "$dotest/patch" ;;
*) action=again ;;
esac
done
diff --git a/git-svn.perl b/git-svn.perl
index 42c9a72..c4ca548 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5171,10 +5171,8 @@ sub git_svn_log_cmd {
# adapted from pager.c
sub config_pager {
- $pager ||= $ENV{GIT_PAGER} || $ENV{PAGER};
- if (!defined $pager) {
- $pager = 'less';
- } elsif (length $pager == 0 || $pager eq 'cat') {
+ chomp(my $pager = command_oneline(qw(var GIT_PAGER)));
+ if ($pager eq 'cat') {
$pager = undef;
}
$ENV{GIT_PAGER_IN_USE} = defined($pager);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 7/8] Provide a build time default-editor setting
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
` (5 preceding siblings ...)
2009-10-31 1:43 ` [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER" Jonathan Nieder
@ 2009-10-31 1:44 ` Jonathan Nieder
2009-10-31 2:09 ` Junio C Hamano
2009-10-31 1:45 ` [PATCH 8/8] Provide a build time default-pager setting Jonathan Nieder
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
8 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 17 +++++++++++++++++
editor.c | 6 +++++-
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
3 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 268aede..625866c 100644
--- a/Makefile
+++ b/Makefile
@@ -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
@@ -1363,6 +1371,15 @@ BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
$(COMPAT_CFLAGS)
LIB_OBJS += $(COMPAT_OBJS)
+# Quote for C
+
+ifdef DEFAULT_EDITOR
+DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))"
+DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
diff --git a/editor.c b/editor.c
index 4f98b72..2aac807 100644
--- a/editor.c
+++ b/editor.c
@@ -2,6 +2,10 @@
#include "strbuf.h"
#include "run-command.h"
+#ifndef DEFAULT_EDITOR
+#define DEFAULT_EDITOR "vi"
+#endif
+
const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
@@ -19,7 +23,7 @@ const char *git_editor(void)
return NULL;
if (!editor)
- editor = "vi";
+ editor = DEFAULT_EDITOR;
return editor;
}
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..73ba44c 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,26 @@ test_description='GIT_EDITOR, core.editor, and stuff'
. ./test-lib.sh
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'does editor have a simple name (no slashes, etc)?' '
+
+ editor=$(TERM=vt100 git var GIT_EDITOR) &&
+ test -n "$editor" &&
+ simple=t &&
+ case "$editor" in
+ */* | core_editor | [A-Z]*)
+ unset simple;;
+ esac
+
+'
+if test -z "${simple+set}"
+then
+ say 'skipping editor tests, default editor is not sought on PATH'
+ test_done
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL "$editor"
do
cat >e-$i.sh <<-EOF
#!$SHELL_PATH
@@ -12,15 +31,13 @@ do
EOF
chmod +x e-$i.sh
done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+mv "e-$editor.sh" "$editor"
test_expect_success setup '
msg="Hand edited" &&
echo "$msg" >expect &&
- git add vi &&
+ git add "$editor" &&
test_tick &&
git commit -m "$msg" &&
git show -s --pretty=oneline |
@@ -44,7 +61,7 @@ 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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -68,7 +85,7 @@ 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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH 7/8] Provide a build time default-editor setting
2009-10-31 1:44 ` [PATCH 7/8] Provide a build time default-editor setting Jonathan Nieder
@ 2009-10-31 2:09 ` Junio C Hamano
2009-10-31 3:26 ` Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2009-10-31 2:09 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> +test_expect_success 'does editor have a simple name (no slashes, etc)?' '
> +
> + editor=$(TERM=vt100 git var GIT_EDITOR) &&
> + test -n "$editor" &&
> + simple=t &&
> + case "$editor" in
> + */* | core_editor | [A-Z]*)
Hmm, what are the latter two cases designed to catch?
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH 7/8] Provide a build time default-editor setting
2009-10-31 2:09 ` Junio C Hamano
@ 2009-10-31 3:26 ` Jonathan Nieder
2009-10-31 19:51 ` Junio C Hamano
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 3:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> +test_expect_success 'does editor have a simple name (no slashes, etc)?' '
>> +
>> + editor=$(TERM=vt100 git var GIT_EDITOR) &&
>> + test -n "$editor" &&
>> + simple=t &&
>> + case "$editor" in
>> + */* | core_editor | [A-Z]*)
>
> Hmm, what are the latter two cases designed to catch?
Both are meant to allow the test to work without too many changes.
The core_editor case is a little pedantic, since it is unlikely to
actually come up in practice. With a default editor of core_editor,
the initial loop will overwrite e-core_editor.sh (to be used through
the core.editor configuration) with e-core_editor.sh (to be used as a
fallback editor) before renaming it to core_editor.
I missed some other cases: If editor is .git, e-GIT_EDITOR.sh, etc,
the mv will still misbehave.
The [A-Z]* test is to avoid changing the loop around line 86:
| unset EDITOR VISUAL GIT_EDITOR
| git config --unset-all core.editor
| for i in "$editor" EDITOR VISUAL core_editor GIT_EDITOR
| do
| echo "Edited by $i" >expect
| case "$i" in
| core_editor)
| git config core.editor ./e-core_editor.sh
| ;;
| [A-Z]*)
| eval "$i=./e-$i.sh"
| export $i
| ;;
| esac
| test_expect_success "Using $i (override)" '
| git --exec-path=. commit --amend &&
| git show -s --pretty=oneline |
| sed -e "s/^[0-9a-f]* //" >actual &&
| diff actual expect
| '
| done
which I do not think is worth making more complicated.
Maybe it would be better to just check for an editor consisting only
of alphabetical characters. Perhaps something like the following:
-- %< --
From: Jonathan Nieder <jrnieder@gmail.com>
Subject: [PATCH] Provide a build time default-editor setting
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 17 +++++++++++++++++
editor.c | 6 +++++-
t/t7005-editor.sh | 27 ++++++++++++++++++++-------
3 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 268aede..625866c 100644
--- a/Makefile
+++ b/Makefile
@@ -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
@@ -1363,6 +1371,15 @@ BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
$(COMPAT_CFLAGS)
LIB_OBJS += $(COMPAT_OBJS)
+# Quote for C
+
+ifdef DEFAULT_EDITOR
+DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))"
+DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
diff --git a/editor.c b/editor.c
index 4f98b72..2aac807 100644
--- a/editor.c
+++ b/editor.c
@@ -2,6 +2,10 @@
#include "strbuf.h"
#include "run-command.h"
+#ifndef DEFAULT_EDITOR
+#define DEFAULT_EDITOR "vi"
+#endif
+
const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
@@ -19,7 +23,7 @@ const char *git_editor(void)
return NULL;
if (!editor)
- editor = "vi";
+ editor = DEFAULT_EDITOR;
return editor;
}
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..13c37de 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,22 @@ test_description='GIT_EDITOR, core.editor, and stuff'
. ./test-lib.sh
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'determine default editor' '
+
+ editor=$(TERM=vt100 git var GIT_EDITOR) &&
+ test -n "$editor"
+
+'
+
+if ! test -z "$(printf '%s\n' "$editor" | sed '/^[a-z]*$/d')"
+then
+ say 'skipping editor tests, default editor name too complicated'
+ test_done
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL "$editor"
do
cat >e-$i.sh <<-EOF
#!$SHELL_PATH
@@ -12,15 +27,13 @@ do
EOF
chmod +x e-$i.sh
done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+mv "e-$editor.sh" "$editor"
test_expect_success setup '
msg="Hand edited" &&
echo "$msg" >expect &&
- git add vi &&
+ git add "$editor" &&
test_tick &&
git commit -m "$msg" &&
git show -s --pretty=oneline |
@@ -44,7 +57,7 @@ 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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -68,7 +81,7 @@ 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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH 7/8] Provide a build time default-editor setting
2009-10-31 3:26 ` Jonathan Nieder
@ 2009-10-31 19:51 ` Junio C Hamano
2009-10-31 21:21 ` Jonathan Nieder
0 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2009-10-31 19:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> +test_expect_success 'does editor have a simple name (no slashes, etc)?' '
>>> +
>>> + editor=$(TERM=vt100 git var GIT_EDITOR) &&
>>> + test -n "$editor" &&
>>> + simple=t &&
>>> + case "$editor" in
>>> + */* | core_editor | [A-Z]*)
>>
>> Hmm, what are the latter two cases designed to catch?
>
> Both are meant to allow the test to work without too many changes.
Honestly speaking, my preference is to see if the built-in editor is
exactly spelled as 'v' 'i', and skip this test altogether if it isn't.
Then the patch only needs to insert these lines (and reword "default
editor name too complicated" to "using customized default editor") without
touching the rest. It simply does not look worth the complication.
You _might_ be able to skip only the "vi" part of the test when you see
that the built-in default is customized, though. I didn't look closely
enough.
> diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
> ...
> +unset EDITOR VISUAL GIT_EDITOR
> +
> +test_expect_success 'determine default editor' '
> +
> + editor=$(TERM=vt100 git var GIT_EDITOR) &&
> + test -n "$editor"
> +
> +'
> +
> +if ! test -z "$(printf '%s\n' "$editor" | sed '/^[a-z]*$/d')"
> +then
> + say 'skipping editor tests, default editor name too complicated'
> + test_done
> +fi
> +
^ permalink raw reply [flat|nested] 65+ messages in thread
* Re: [PATCH 7/8] Provide a build time default-editor setting
2009-10-31 19:51 ` Junio C Hamano
@ 2009-10-31 21:21 ` Jonathan Nieder
2009-11-01 4:29 ` Junio C Hamano
0 siblings, 1 reply; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 21:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Junio C Hamano wrote:
> Honestly speaking, my preference is to see if the built-in editor is
> exactly spelled as 'v' 'i', and skip this test altogether if it isn't.
That does make sense. But let me try one last time, before doing
that. (I should have sat down and thought this through carefully
before sending the first version --- sorry.)
Though the first two iterations of the patch were pretty ugly, the
third was just 's/vi/"$editor"/g' after setting editor and bailing out
if it does not consist of lowercase letters. As you mentioned, it
makes more sense to skip only the "vi" part of the test.
Tested with DEFAULT_EDITOR=vi, vim, /usr/bin/nonexistent.
t/t7005-editor.sh | 37 +++++++++++++++++++++++++------------
1 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index a95fe19..5257f4d 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,21 @@ test_description='GIT_EDITOR, core.editor, and stuff'
. ./test-lib.sh
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'determine default editor' '
+
+ vi=$(TERM=vt100 git var GIT_EDITOR) &&
+ test -n "$vi"
+
+'
+
+if ! expr "$vi" : '^[a-z]*$' >/dev/null
+then
+ vi=
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
do
cat >e-$i.sh <<-EOF
#!$SHELL_PATH
@@ -12,19 +26,18 @@ do
EOF
chmod +x e-$i.sh
done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+
+if ! test -z "$vi"
+then
+ mv e-$vi.sh $vi
+fi
test_expect_success setup '
- msg="Hand edited" &&
+ msg="Hand-edited" &&
+ test_commit "$msg" &&
echo "$msg" >expect &&
- git add vi &&
- test_tick &&
- git commit -m "$msg" &&
- git show -s --pretty=oneline |
- sed -e "s/^[0-9a-f]* //" >actual &&
+ git show -s --format=%s > actual &&
diff actual expect
'
@@ -54,7 +67,7 @@ test_expect_success 'dumb should prefer EDITOR to VISUAL' '
TERM=vt100
export TERM
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -78,7 +91,7 @@ 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 $vi EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
^ permalink raw reply related [flat|nested] 65+ messages in thread
* Re: [PATCH 7/8] Provide a build time default-editor setting
2009-10-31 21:21 ` Jonathan Nieder
@ 2009-11-01 4:29 ` Junio C Hamano
0 siblings, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2009-11-01 4:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Jonathan Nieder <jrnieder@gmail.com> writes:
> +unset EDITOR VISUAL GIT_EDITOR
> +
> +test_expect_success 'determine default editor' '
> +
> + vi=$(TERM=vt100 git var GIT_EDITOR) &&
> + test -n "$vi"
> +
> +'
> +
> +if ! expr "$vi" : '^[a-z]*$' >/dev/null
> +then
> + vi=
> +fi
> +
> +for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
> ...
> -for i in vi EDITOR VISUAL core_editor GIT_EDITOR
> +for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
> do
> echo "Edited by $i" >expect
> unset EDITOR VISUAL GIT_EDITOR
> @@ -78,7 +91,7 @@ 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 $vi EDITOR VISUAL core_editor GIT_EDITOR
> do
Beautiful ;-)
^ permalink raw reply [flat|nested] 65+ messages in thread
* [PATCH 8/8] Provide a build time default-pager setting
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
` (6 preceding siblings ...)
2009-10-31 1:44 ` [PATCH 7/8] Provide a build time default-editor setting Jonathan Nieder
@ 2009-10-31 1:45 ` Jonathan Nieder
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
8 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-10-31 1:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
From: Junio C Hamano <gitster@pobox.com>
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 11 +++++++++++
| 6 +++++-
2 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 625866c..18fc50a 100644
--- a/Makefile
+++ b/Makefile
@@ -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:
@@ -1380,6 +1384,13 @@ DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
endif
+ifdef DEFAULT_PAGER
+DEFAULT_PAGER_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_PAGER)))"
+DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
--git a/pager.c b/pager.c
index 0b63d99..92c03f6 100644
--- a/pager.c
+++ b/pager.c
@@ -2,6 +2,10 @@
#include "run-command.h"
#include "sigchain.h"
+#ifndef DEFAULT_PAGER
+#define DEFAULT_PAGER "less"
+#endif
+
/*
* This is split up from the rest of git so that we can do
* something different on Windows.
@@ -60,7 +64,7 @@ const char *git_pager(void)
if (!pager)
pager = getenv("PAGER");
if (!pager)
- pager = "less";
+ pager = DEFAULT_PAGER;
else if (!*pager || !strcmp(pager, "cat"))
pager = NULL;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH v4 0/9] Default pager and editor
2009-10-31 1:20 ` [PATCH v3 0/8] Default pager and editor Jonathan Nieder
` (7 preceding siblings ...)
2009-10-31 1:45 ` [PATCH 8/8] Provide a build time default-pager setting Jonathan Nieder
@ 2009-11-11 23:51 ` Jonathan Nieder
2009-11-11 23:52 ` [PATCH 1/9] Handle more shell metacharacters in editor names Jonathan Nieder
` (9 more replies)
8 siblings, 10 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-11 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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?
Here’s a reroll. The interdiff is very small:
diff --git a/ident.c b/ident.c
index 99f1c85..26409b2 100644
--- a/ident.c
+++ b/ident.c
@@ -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(-)
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 1/9] Handle more shell metacharacters in editor names
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
@ 2009-11-11 23:52 ` Jonathan Nieder
2009-11-11 23:56 ` [PATCH 2/9] Do not use VISUAL editor on dumb terminals Jonathan Nieder
` (8 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-11 23:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Unchanged from jn/editor-pager, included only for reference.
editor.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/editor.c b/editor.c
index 4d469d0..941c0b2 100644
--- a/editor.c
+++ b/editor.c
@@ -28,7 +28,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
const char *args[6];
struct strbuf arg0 = STRBUF_INIT;
- if (strcspn(editor, "$ \t'") != len) {
+ if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) {
/* there are specials */
strbuf_addf(&arg0, "%s \"$@\"", editor);
args[i++] = "sh";
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 2/9] Do not use VISUAL editor on dumb terminals
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
2009-11-11 23:52 ` [PATCH 1/9] Handle more shell metacharacters in editor names Jonathan Nieder
@ 2009-11-11 23:56 ` Jonathan Nieder
2009-11-11 23:57 ` [PATCH 3/9] Suppress warnings from "git var -l" Jonathan Nieder
` (7 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-11 23:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
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(-)
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";
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..a95fe19 100755
--- 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 &&
+ export EDITOR VISUAL &&
+ git commit --amend &&
+ test "$(git show -s --format=%s)" = "Edited by EDITOR"
+
+'
+
TERM=vt100
export TERM
for i in vi EDITOR VISUAL core_editor GIT_EDITOR
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d2de576..a603f6d 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -86,7 +86,7 @@ chmod 755 editor
test_expect_success \
"amend commit" \
- "VISUAL=./editor git commit --amend"
+ "EDITOR=./editor git commit --amend"
test_expect_success \
"passing -m and -F" \
@@ -107,7 +107,7 @@ chmod 755 editor
test_expect_success \
"editing message from other commit" \
"echo 'hula hula' >file && \
- VISUAL=./editor git commit -c HEAD^ -a"
+ EDITOR=./editor git commit -c HEAD^ -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=./editor git commit -a -F msg &&
+ EDITOR=./editor git commit -a -F msg &&
git show -s --pretty=format:"%s" | grep -q good &&
echo "quack" >file &&
- echo "Another good message." | VISUAL=./editor git commit -a -F - &&
+ echo "Another good message." | EDITOR=./editor git commit -a -F - &&
git show -s --pretty=format:"%s" | grep -q good
'
# We could just check the head sha1, but checking each commit makes it
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..ec3336a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -30,7 +30,7 @@ TZ=UTC
TERM=dumb
export LANG LC_ALL PAGER TERM TZ
EDITOR=:
-VISUAL=:
+unset VISUAL
unset GIT_EDITOR
unset AUTHOR_DATE
unset AUTHOR_EMAIL
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5
export GIT_MERGE_VERBOSITY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
-export EDITOR VISUAL
+export EDITOR
GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# Protect ourselves from common misconfiguration to export
@@ -207,8 +207,8 @@ trap 'die' EXIT
test_set_editor () {
FAKE_EDITOR="$1"
export FAKE_EDITOR
- VISUAL='"$FAKE_EDITOR"'
- export VISUAL
+ EDITOR='"$FAKE_EDITOR"'
+ export EDITOR
}
test_tick () {
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 3/9] Suppress warnings from "git var -l"
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
2009-11-11 23:52 ` [PATCH 1/9] Handle more shell metacharacters in editor names Jonathan Nieder
2009-11-11 23:56 ` [PATCH 2/9] Do not use VISUAL editor on dumb terminals Jonathan Nieder
@ 2009-11-11 23:57 ` Jonathan Nieder
2009-11-12 0:01 ` [PATCH 4/9] Teach git var about GIT_EDITOR Jonathan Nieder
` (6 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-11 23:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/ident.c b/ident.c
index 99f1c85..26409b2 100644
--- a/ident.c
+++ b/ident.c
@@ -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);
diff --git a/var.c b/var.c
index 125c0d1..dacbaab 100644
--- a/var.c
+++ b/var.c
@@ -22,7 +22,7 @@ static void list_vars(void)
{
struct git_var *ptr;
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(IDENT_WARN_ON_NO_NAME));
+ printf("%s=%s\n", ptr->name, ptr->read(0));
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 4/9] Teach git var about GIT_EDITOR
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (2 preceding siblings ...)
2009-11-11 23:57 ` [PATCH 3/9] Suppress warnings from "git var -l" Jonathan Nieder
@ 2009-11-12 0:01 ` Jonathan Nieder
2009-11-12 0:02 ` [PATCH 5/9] Teach git var about GIT_PAGER Jonathan Nieder
` (5 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-12 0:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index e2f4c09..89e4b4f 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 96840c7..311cfe1 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor(void);
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index 3f13751..70618f1 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
const char *terminal = getenv("TERM");
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
editor = getenv("EDITOR");
if (!editor && terminal_is_dumb)
- return error("terminal is dumb, but EDITOR unset");
+ return 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 (!editor)
+ return error("Terminal is dumb, but EDITOR unset");
+
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index dacbaab..b502487 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,16 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int flag)
+{
+ const char *pgm = git_editor();
+
+ if (!pgm && flag & IDENT_ERROR_ON_NO_NAME)
+ die("Terminal is dumb, but EDITOR unset");
+
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,14 +25,18 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
static void list_vars(void)
{
struct git_var *ptr;
+ const char *val;
+
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(0));
+ if ((val = ptr->read(0)))
+ printf("%s=%s\n", ptr->name, val);
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 5/9] Teach git var about GIT_PAGER
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (3 preceding siblings ...)
2009-11-12 0:01 ` [PATCH 4/9] Teach git var about GIT_EDITOR Jonathan Nieder
@ 2009-11-12 0:02 ` Jonathan Nieder
2009-11-12 0:02 ` [PATCH 6/9] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR" Jonathan Nieder
` (4 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-12 0:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
The rest of the series is unchanged from pu.
Documentation/git-var.txt | 6 ++++++
cache.h | 1 +
| 18 +++++++++++++++---
var.c | 10 ++++++++++
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 89e4b4f..ef6aa81 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -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!::
diff --git a/cache.h b/cache.h
index 311cfe1..5aaa4ba 100644
--- a/cache.h
+++ b/cache.h
@@ -751,6 +751,7 @@ extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
extern const char *git_editor(void);
+extern const char *git_pager(void);
struct checkout {
const char *base_dir;
--git a/pager.c b/pager.c
index 86facec..0b63d99 100644
--- a/pager.c
+++ b/pager.c
@@ -44,12 +44,14 @@ static void wait_for_pager_signal(int signo)
raise(signo);
}
-void setup_pager(void)
+const char *git_pager(void)
{
- const char *pager = getenv("GIT_PAGER");
+ const char *pager;
if (!isatty(1))
- return;
+ return NULL;
+
+ pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
git_config(git_default_config, NULL);
@@ -60,6 +62,16 @@ void setup_pager(void)
if (!pager)
pager = "less";
else if (!*pager || !strcmp(pager, "cat"))
+ pager = NULL;
+
+ return pager;
+}
+
+void setup_pager(void)
+{
+ const char *pager = git_pager();
+
+ if (!pager)
return;
spawned_pager = 1; /* means we are emitting to terminal */
diff --git a/var.c b/var.c
index b502487..d9892f8 100644
--- a/var.c
+++ b/var.c
@@ -18,6 +18,15 @@ static const char *editor(int flag)
return pgm;
}
+static const char *pager(int flag)
+{
+ const char *pgm = git_pager();
+
+ if (!pgm)
+ pgm = "cat";
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -26,6 +35,7 @@ static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
{ "GIT_EDITOR", editor },
+ { "GIT_PAGER", pager },
{ "", NULL },
};
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 6/9] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (4 preceding siblings ...)
2009-11-12 0:02 ` [PATCH 5/9] Teach git var about GIT_PAGER Jonathan Nieder
@ 2009-11-12 0:02 ` Jonathan Nieder
2009-11-12 0:03 ` [PATCH 7/9] am -i, git-svn: use "git var GIT_PAGER" Jonathan Nieder
` (3 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-12 0:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d1e2120..5181b77 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -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
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..3ea80c8 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -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
-----
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 767cf4d..c85d7f4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -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
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index e710219..48059d0 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -729,13 +729,10 @@ class P4Submit(Command):
tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
tmpFile.close()
mtime = os.stat(fileName).st_mtime
- defaultEditor = "vi"
- if platform.system() == "Windows":
- defaultEditor = "notepad"
if os.environ.has_key("P4EDITOR"):
editor = os.environ.get("P4EDITOR")
else:
- editor = os.environ.get("EDITOR", defaultEditor);
+ editor = read_pipe("git var GIT_EDITOR")
system(editor + " " + fileName)
response = "y"
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 69aeaf0..0c74e5c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -987,8 +987,7 @@ sub edit_hunk_manually {
EOF
close $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) {
diff --git a/git-send-email.perl b/git-send-email.perl
index a0279de..4f5da4e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -162,7 +162,8 @@ my $compose_filename;
# Handle interactive edition of files.
my $multiedit;
-my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+my $editor = Git::command_oneline('var', 'GIT_EDITOR');
+
sub do_edit {
if (defined($multiedit) && !$multiedit) {
map {
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..99cceeb 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -99,19 +99,12 @@ set_reflog_action() {
}
git_editor() {
- : "${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 "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}" '"$@"'
+ if test -z "${GIT_EDITOR:+set}"
+ then
+ GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
+ fi
+
+ eval "$GIT_EDITOR" '"$@"'
}
is_bare_repository () {
diff --git a/git-svn.perl b/git-svn.perl
index 6a3b501..42c9a72 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1321,9 +1321,8 @@ sub get_commit_entry {
close $log_fh or croak $!;
if ($_edit || ($type eq 'tree')) {
- my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
- # TODO: strip out spaces, comments, like git-commit.sh
- system($editor, $commit_editmsg);
+ chomp(my $editor = command_oneline(qw(var GIT_EDITOR)));
+ system('sh', '-c', $editor.' "$@"', $editor, $commit_editmsg);
}
rename $commit_editmsg, $commit_msg or croak $!;
{
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 7/9] am -i, git-svn: use "git var GIT_PAGER"
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (5 preceding siblings ...)
2009-11-12 0:02 ` [PATCH 6/9] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR" Jonathan Nieder
@ 2009-11-12 0:03 ` Jonathan Nieder
2009-11-12 0:03 ` [PATCH 8/9] Provide a build time default-editor setting Jonathan Nieder
` (2 subsequent siblings)
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-12 0:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Unchanged.
git-am.sh | 5 ++++-
git-svn.perl | 6 ++----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c132f50..2649487 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -649,7 +649,10 @@ do
[eE]*) git_editor "$dotest/final-commit"
action=again ;;
[vV]*) action=again
- LESS=-S ${PAGER:-less} "$dotest/patch" ;;
+ : ${GIT_PAGER=$(git var GIT_PAGER)}
+ : ${LESS=-FRSX}
+ export LESS
+ $GIT_PAGER "$dotest/patch" ;;
*) action=again ;;
esac
done
diff --git a/git-svn.perl b/git-svn.perl
index 42c9a72..c4ca548 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5171,10 +5171,8 @@ sub git_svn_log_cmd {
# adapted from pager.c
sub config_pager {
- $pager ||= $ENV{GIT_PAGER} || $ENV{PAGER};
- if (!defined $pager) {
- $pager = 'less';
- } elsif (length $pager == 0 || $pager eq 'cat') {
+ chomp(my $pager = command_oneline(qw(var GIT_PAGER)));
+ if ($pager eq 'cat') {
$pager = undef;
}
$ENV{GIT_PAGER_IN_USE} = defined($pager);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 8/9] Provide a build time default-editor setting
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (6 preceding siblings ...)
2009-11-12 0:03 ` [PATCH 7/9] am -i, git-svn: use "git var GIT_PAGER" Jonathan Nieder
@ 2009-11-12 0:03 ` Jonathan Nieder
2009-11-12 0:04 ` [PATCH 9/9] Provide a build time default-pager setting Jonathan Nieder
2009-11-15 9:04 ` [PATCH v4 0/9] Default pager and editor Junio C Hamano
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-12 0:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Unchanged.
Makefile | 17 +++++++++++++++++
editor.c | 6 +++++-
t/t7005-editor.sh | 37 +++++++++++++++++++++++++------------
3 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/Makefile b/Makefile
index 268aede..625866c 100644
--- a/Makefile
+++ b/Makefile
@@ -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
@@ -1363,6 +1371,15 @@ BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
$(COMPAT_CFLAGS)
LIB_OBJS += $(COMPAT_OBJS)
+# Quote for C
+
+ifdef DEFAULT_EDITOR
+DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))"
+DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
diff --git a/editor.c b/editor.c
index 70618f1..615f575 100644
--- a/editor.c
+++ b/editor.c
@@ -2,6 +2,10 @@
#include "strbuf.h"
#include "run-command.h"
+#ifndef DEFAULT_EDITOR
+#define DEFAULT_EDITOR "vi"
+#endif
+
const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
@@ -19,7 +23,7 @@ const char *git_editor(void)
return NULL;
if (!editor)
- editor = "vi";
+ editor = DEFAULT_EDITOR;
return editor;
}
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index a95fe19..5257f4d 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,21 @@ test_description='GIT_EDITOR, core.editor, and stuff'
. ./test-lib.sh
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'determine default editor' '
+
+ vi=$(TERM=vt100 git var GIT_EDITOR) &&
+ test -n "$vi"
+
+'
+
+if ! expr "$vi" : '^[a-z]*$' >/dev/null
+then
+ vi=
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
do
cat >e-$i.sh <<-EOF
#!$SHELL_PATH
@@ -12,19 +26,18 @@ do
EOF
chmod +x e-$i.sh
done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+
+if ! test -z "$vi"
+then
+ mv e-$vi.sh $vi
+fi
test_expect_success setup '
- msg="Hand edited" &&
+ msg="Hand-edited" &&
+ test_commit "$msg" &&
echo "$msg" >expect &&
- git add vi &&
- test_tick &&
- git commit -m "$msg" &&
- git show -s --pretty=oneline |
- sed -e "s/^[0-9a-f]* //" >actual &&
+ git show -s --format=%s > actual &&
diff actual expect
'
@@ -54,7 +67,7 @@ test_expect_success 'dumb should prefer EDITOR to VISUAL' '
TERM=vt100
export TERM
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -78,7 +91,7 @@ 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 $vi EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* [PATCH 9/9] Provide a build time default-pager setting
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (7 preceding siblings ...)
2009-11-12 0:03 ` [PATCH 8/9] Provide a build time default-editor setting Jonathan Nieder
@ 2009-11-12 0:04 ` Jonathan Nieder
2009-11-15 9:04 ` [PATCH v4 0/9] Default pager and editor Junio C Hamano
9 siblings, 0 replies; 65+ messages in thread
From: Jonathan Nieder @ 2009-11-12 0:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
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 <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
As in pu.
Makefile | 11 +++++++++++
| 6 +++++-
2 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 625866c..18fc50a 100644
--- a/Makefile
+++ b/Makefile
@@ -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:
@@ -1380,6 +1384,13 @@ DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
endif
+ifdef DEFAULT_PAGER
+DEFAULT_PAGER_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_PAGER)))"
+DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'
+endif
+
ALL_CFLAGS += $(BASIC_CFLAGS)
ALL_LDFLAGS += $(BASIC_LDFLAGS)
--git a/pager.c b/pager.c
index 0b63d99..92c03f6 100644
--- a/pager.c
+++ b/pager.c
@@ -2,6 +2,10 @@
#include "run-command.h"
#include "sigchain.h"
+#ifndef DEFAULT_PAGER
+#define DEFAULT_PAGER "less"
+#endif
+
/*
* This is split up from the rest of git so that we can do
* something different on Windows.
@@ -60,7 +64,7 @@ const char *git_pager(void)
if (!pager)
pager = getenv("PAGER");
if (!pager)
- pager = "less";
+ pager = DEFAULT_PAGER;
else if (!*pager || !strcmp(pager, "cat"))
pager = NULL;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 65+ messages in thread* Re: [PATCH v4 0/9] Default pager and editor
2009-11-11 23:51 ` [PATCH v4 0/9] Default pager and editor Jonathan Nieder
` (8 preceding siblings ...)
2009-11-12 0:04 ` [PATCH 9/9] Provide a build time default-pager setting Jonathan Nieder
@ 2009-11-15 9:04 ` Junio C Hamano
9 siblings, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2009-11-15 9:04 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
Thanks, re-queued.
I've been sick and in bed for the past few days, so apologies for a late
reply.
^ permalink raw reply [flat|nested] 65+ messages in thread