* Re: [PATCH 18/19] Refactor git_remote_cvs to a more generic git_remote_helpers
From: Johan Herland @ 2009-10-30 8:42 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <1256839287-19016-19-git-send-email-srabbelier@gmail.com>
On Thursday 29 October 2009, Sverre Rabbelier wrote:
> This in an effort to allow future remote helpers written in python to
> re-use the non-cvs-specific code.
>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
[snip]
> diff --git a/git_remote_helpers/__init__.py
> b/git_remote_helpers/__init__.py new file mode 100644
> index 0000000..38c7b5f
> --- /dev/null
> +++ b/git_remote_helpers/__init__.py
> @@ -0,0 +1,27 @@
> +#!/usr/bin/env python
> +
> +"""Support library package for git remote helpers.
> +
> +Git remote helpers are helper commands that interfaces with a non-git
> +repository to provide automatic import of non-git history into a Git
> +repository.
> +
> +This package provides the support library needed by these helpers..
> +The following modules are included:
> +
> +- cvs/cvs - Interaction with CVS repositories
> +
> +- cvs/symbol_cache - Local CVS symbol cache
> +
> +- cvs/changeset - Collect individual CVS revisions into commits
> +
> +- cvs/commit_states - Map Git commits to CVS states
> +
> +- cvs/revision_map - Map CVS revisions to various metainformation
> +
> +- git/git - Interaction with Git repositories
Since this is Python documentation within a package, I'd rather refer to the
python modules as _modules_ and not files. I.e. please use '.' instead of
'/':
+- cvs.cvs - Interaction with CVS repositories
+
+- cvs.symbol_cache - Local CVS symbol cache
+
+- cvs.changeset - Collect individual CVS revisions into commits
+
+- cvs.commit_states - Map Git commits to CVS states
+
+- cvs.revision_map - Map CVS revisions to various metainformation
+
+- git.git - Interaction with Git repositories
> +
> +- util - General utility functionality use by the other modules in
> + this package, and also used directly by git-remote-cvs.
Probably you should drop the direct reference to git-remote-cvs.
> diff --git a/git_remote_cvs/util.py b/git_remote_helpers/util.py
> similarity index 100%
> rename from git_remote_cvs/util.py
> rename to git_remote_helpers/util.py
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 77ad23e..c7530aa 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -640,9 +640,9 @@ test -d ../templates/blt || {
>
> if test -z "$GIT_TEST_INSTALLED"
> then
> - GITPYTHONLIB="$(pwd)/../git_remote_cvs/build/lib"
> + GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
> export GITPYTHONLIB
> - test -d ../git_remote_cvs/build || {
> + test -d ../git_remote_helpers/build || {
> error "You haven't built git_remote_cvs yet, have you?"
Please also s/git_remote_cvs/git_remote_helpers/ in the error message.
Otherwise, it all looks good :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* [PATCH v2 0/8] Default pager and editor
From: Jonathan Nieder @ 2009-10-30 10:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <7v8weu6idl.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> My "how about" patch on DEFAULT_PAGER might be minimally safe with
>
> make DEFAULT_PAGER="/c/my program/less"
It isn’t, actually, since in pager.c the pager already gets run through
sh if it contains certain shell metacharacters.
> but if you are going to do this for real, you would need to use proper
> quoting in the Makefile (look for _SQ for hints).
Good catch --- thanks.
> > This change makes t7005-editor into a mess. Any ideas for fixing
> > this?
>
> I think the introduction of DEFAULT_EDITOR makes it unfixable; your
> DEFAULT_EDITOR may be set to '/usr/bin/vi' not 'vi'.
>
> Just detect DEFAULT_EDITOR being not the default 'vi' and abort/skip the
> entire test, perhaps?
Yep, unfortunately that looks like the best thing to do. I tried to
salvage some of the test for distros (like Debian) that might override
the default without using an absolute path.
Here’s an updated series. It doesn’t provide git var --run yet since
the Windows exit status magic means that would require either futzing
with the run_pager() implementation or reimplementing cat in var.c.
Thoughts?
Johannes Sixt (1):
Teach git var about GIT_EDITOR
Jonathan Nieder (6):
launch_editor: Longer error message when TERM=dumb
Handle more shell metacharacters in editor name
Teach git var about GIT_PAGER
add -i, send-email, svn, p4, etc: Use "git var GIT_EDITOR"
am -i, git-svn: use $(git var GIT_PAGER) instead of 'less'
Provide a build time default-editor setting
Junio C Hamano (1):
Provide a build time default-pager setting
Documentation/config.txt | 4 +---
Documentation/git-commit.txt | 2 +-
Documentation/git-send-email.txt | 4 ++--
Documentation/git-var.txt | 14 ++++++++++++++
Makefile | 28 ++++++++++++++++++++++++++++
cache.h | 2 ++
contrib/fast-import/git-p4 | 5 +----
editor.c | 33 ++++++++++++++++++++++++++++-----
git-add--interactive.perl | 3 +--
git-am.sh | 5 ++++-
git-send-email.perl | 3 ++-
git-sh-setup.sh | 19 ++++++-------------
git-svn.perl | 11 ++++-------
pager.c | 24 ++++++++++++++++++++----
t/t7005-editor.sh | 31 ++++++++++++++++++++++++-------
var.c | 20 ++++++++++++++++++++
16 files changed, 158 insertions(+), 50 deletions(-)
^ permalink raw reply
* [PATCH 1/8] launch_editor: Longer error message when TERM=dumb
From: Jonathan Nieder @ 2009-10-30 10:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 2/8] Handle more shell metacharacters in editor names
From: Jonathan Nieder @ 2009-10-30 10:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 3/8] Teach git var about GIT_EDITOR
From: Jonathan Nieder @ 2009-10-30 10:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 4/8] Teach git var about GIT_PAGER
From: Jonathan Nieder @ 2009-10-30 10:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 5/8] add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
From: Jonathan Nieder @ 2009-10-30 10:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 6/8] am -i, git-svn: use "git var GIT_PAGER"
From: Jonathan Nieder @ 2009-10-30 10:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 7/8] Provide a build time default-editor setting
From: Jonathan Nieder @ 2009-10-30 10:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH 8/8] Provide a build time default-pager setting
From: Jonathan Nieder @ 2009-10-30 10:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* [PATCH/RFC 9/8] Teach git var to run the editor
From: Jonathan Nieder @ 2009-10-30 10:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030101634.GA1610@progeny.tock>
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
* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: Markus Heidelberg @ 2009-10-30 10:35 UTC (permalink / raw)
To: Jay Soffian
Cc: Charles Bailey, Scott Chacon, git list, Junio C Hamano,
David Aguilar
In-Reply-To: <76718490910292000t7b024b83y68d71b6ff810c15@mail.gmail.com>
Jay Soffian, 30.10.2009:
> On Thu, Oct 29, 2009 at 9:02 PM, Markus Heidelberg
> <markus.heidelberg@web.de> wrote:
> > He didn't mean p4merge on other platforms, but other merge tools on Mac
> > OS X. What about all the other merge tools already in mergetool--lib?
> > Should they get special handling, too?
>
> If someone wants to scratch that itch, then yes. The default diff tool
> for OS X has its helper already in /usr/bin (opendiff). p4merge is
> arguably a better merge tool, and it installs as an app bundle in
> /Applications. I'm not sure about the other diff tools, I haven't
> looked.
>
> > And for Windows we could add C:\Program Files\MergeToolX\tool.exe for
> > every merge tool.
>
> If it makes those tools easier to use with git, and if someone on
> Windows wants to scratch that itch, then yes, we should.
Another possible problem: the user can change the installation
destination on Windows. What's the behaviour of Mac OS here? Is the
instalation path fixed or changeable?
Markus
^ permalink raw reply
* Re: [PATCH] clone: detect extra arguments
From: Jonathan Nieder @ 2009-10-30 11:19 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091029160614.GB7622@sigill.intra.peff.net>
Jeff King wrote:
> Should we maybe be showing the usage in this case?
Sounds reasonable. How about this patch on top?
-- %< --
Subject: [PATCH] clone: print usage on wrong number of arguments
git clone's short usage string is only 22 lines, so an error
message plus usage string still fits comfortably on an 80x24
terminal.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin-clone.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index 76ad581..736d9e1 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -378,10 +378,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
builtin_clone_usage, 0);
if (argc > 2)
- die("Too many arguments.");
+ usage_msg_opt("Too many arguments.",
+ builtin_clone_usage, builtin_clone_options);
if (argc == 0)
- die("You must specify a repository to clone.");
+ usage_msg_opt("You must specify a repository to clone.",
+ builtin_clone_usage, builtin_clone_options);
if (option_mirror)
option_bare = 1;
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: Reece Dunn @ 2009-10-30 11:25 UTC (permalink / raw)
To: markus.heidelberg
Cc: Jay Soffian, Charles Bailey, Scott Chacon, git list,
Junio C Hamano, David Aguilar
In-Reply-To: <200910301135.59831.markus.heidelberg@web.de>
2009/10/30 Markus Heidelberg <markus.heidelberg@web.de>:
> Jay Soffian, 30.10.2009:
>> On Thu, Oct 29, 2009 at 9:02 PM, Markus Heidelberg
>> <markus.heidelberg@web.de> wrote:
>> > He didn't mean p4merge on other platforms, but other merge tools on Mac
>> > OS X. What about all the other merge tools already in mergetool--lib?
>> > Should they get special handling, too?
>>
>> If someone wants to scratch that itch, then yes. The default diff tool
>> for OS X has its helper already in /usr/bin (opendiff). p4merge is
>> arguably a better merge tool, and it installs as an app bundle in
>> /Applications. I'm not sure about the other diff tools, I haven't
>> looked.
>>
>> > And for Windows we could add C:\Program Files\MergeToolX\tool.exe for
>> > every merge tool.
>>
>> If it makes those tools easier to use with git, and if someone on
>> Windows wants to scratch that itch, then yes, we should.
>
> Another possible problem: the user can change the installation
> destination on Windows. What's the behaviour of Mac OS here? Is the
> instalation path fixed or changeable?
For Windows, the program should have an InstallDir or similar registry
value in a fixed place in the registry to point to where it is
installed (something like
HKLM/Software/[Vendor]/[Application]/[Version]).
As for Linux, there is no guarantee that things like p4merge are in
the path either. It could be placed under /opt/perforce or
/home/perforce.
What would be sensible (for all platforms) is:
1/ if [difftool|mergetool].toolname.path is set, use that (is this
documented?)
2/ try looking for the tool in the system path
3/ try some intelligent guessing
4/ if none of these work, print out an error message -- ideally,
this should mention the configuration option in (1)
(3) is what is being discussed. It is good that it will work without
any user configuration (especially for standard tools installed in
standard places), but isn't really a big problem as long as the user
is prompted to configure the tool path. Also, I'm not sure how this
will work with multiple versions of the tools installed (e.g. vim/gvim
and p4merge).
- Reece
^ permalink raw reply
* Re: [PATCH 02/19] Allow fetch to modify refs
From: Sverre Rabbelier @ 2009-10-30 12:22 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0910300151450.14365@iabervon.org>
Heya,
On Thu, Oct 29, 2009 at 22:56, Daniel Barkalow <barkalow@iabervon.org> wrote:
>> +++ b/builtin-clone.c
>> @@ -526,8 +526,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>>
>> refs = transport_get_remote_refs(transport);
>> if (refs) {
>> - mapped_refs = wanted_peer_refs(refs, refspec);
>> - transport_fetch_refs(transport, mapped_refs);
>> + struct ref *ref_cpy = wanted_peer_refs(refs, refspec);
>> + mapped_refs = ref_cpy;
>> + transport_fetch_refs(transport, ref_cpy);
>
> Just drop this hunk; the change doesn't actually do anything. Otherwise,
> the patch matches what I have.
Am I missing something? I thought we need this hunk since mapped_refs
is const, and transport_fetch_refs takes a non-const argument?
builtin-clone.c: In function ‘cmd_clone’:
builtin-clone.c:531: warning: passing argument 2 of
‘transport_fetch_refs’ discards qualifiers from pointer target type
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [UNSTABLE PATCH 03/19] Allow programs to not depend on remotes having urls
From: Sverre Rabbelier @ 2009-10-30 12:24 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0910300157170.14365@iabervon.org>
Heya,
On Thu, Oct 29, 2009 at 23:02, Daniel Barkalow <barkalow@iabervon.org> wrote:
> Perhaps I should try to get the refactor in
> builtin-push to use push_with_options() without any behavior change into
> master ahead of the rest of this series.
I think that's a good idea in general :).
> Anyway, you correctly understood the intended change here.
Ok, nice.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Sverre Rabbelier @ 2009-10-30 12:26 UTC (permalink / raw)
To: Johan Herland; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <200910300921.00561.johan@herland.net>
Heya,
On Fri, Oct 30, 2009 at 01:21, Johan Herland <johan@herland.net> wrote:
> Please drop this patch from the series. The functionality is not needed,
> since we'll use the fast-import "option" command from the sr/gfi-options
> series instead.
In that case I will rebase the series on top of sr/gfi-options then as
soon as I reroll that one. Also, do you need to change anything else
in git-remote-cvs to do that?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 12/19] 1/2: Add Python support library for CVS remote helper
From: Sverre Rabbelier @ 2009-10-30 12:27 UTC (permalink / raw)
To: Johan Herland
Cc: Git List, Johannes Schindelin, Daniel Barkalow, David Aguilar
In-Reply-To: <200910300933.35567.johan@herland.net>
Heya,
On Fri, Oct 30, 2009 at 01:33, Johan Herland <johan@herland.net> wrote:
> Why? Or: why that one, and not the others? Also, you might want to mention
> your contribution in the commit message itself.
What others? And I thought my contributions were so minor it doesn't
really matter that much :).
> It seems the two functions you add (notify() and warn()) have a different
> indentation than the existing code (which uses 4 spaces). Please fix.
That's weird, will fix.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 18/19] Refactor git_remote_cvs to a more generic git_remote_helpers
From: Sverre Rabbelier @ 2009-10-30 12:29 UTC (permalink / raw)
To: Johan Herland; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <200910300942.54101.johan@herland.net>
Heya,
On Fri, Oct 30, 2009 at 01:42, Johan Herland <johan@herland.net> wrote:
>> +- git/git - Interaction with Git repositories
>
> Since this is Python documentation within a package, I'd rather refer to the
> python modules as _modules_ and not files. I.e. please use '.' instead of
> '/':
Ok, will do.
>> +- util - General utility functionality use by the other modules in
>> + this package, and also used directly by git-remote-cvs.
>
> Probably you should drop the direct reference to git-remote-cvs.
Ah, yes, my bad.
>> + test -d ../git_remote_helpers/build || {
>> error "You haven't built git_remote_cvs yet, have you?"
>
> Please also s/git_remote_cvs/git_remote_helpers/ in the error message.
That's weird, I thought I did that, ah well, will fix.
> Otherwise, it all looks good :)
Thanks for the review.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [Vcs-fast-import-devs] What's cooking in git.git (Oct 2009, #01; Wed, 07)
From: Sverre Rabbelier @ 2009-10-30 12:41 UTC (permalink / raw)
To: Shawn O. Pearce, Ian Clatworthy
Cc: vcs-fast-import-devs, git, Johannes Schindelin
In-Reply-To: <4AEA626D.8060804@canonical.com>
Heya,
On Thu, Oct 29, 2009 at 20:50, Ian Clatworthy
<ian.clatworthy@canonical.com> wrote:
>> On Thu, Oct 8, 2009 at 19:39, Shawn O. Pearce <spearce@spearce.org> wrote:
> Sverre Rabbelier wrote:
>> [edited Shawn's message somewhat to be more relevant to vcs-fast-import-dev]
>
> Thanks Sverre. Before I start, sorry for taking so long to reply to this.
Thanks for the review :).
> My strong preference is for:
>
> * feature = anything impacting semantics
> * option = tool-specific with no impact on semantics permitted.
>
> Both features and options ought to OS independent (where possible).
Even better, Shawn, if this LGTY I will reroll the series implementing this.
>>> I think we want to declare features for import-marks and export-marks
>>> and define these as paths to local files which store a VCS specific
>>> formatted mapping of fast-import mark numbers to VCS labels.
>
> Explicitly specifying the local path names worries me though. Consider someone
> using fastimport tools to maintain multiple mirrors in different tools.
> What should the stream look like then? Does it need to change if we want
> an additional mirror.
I think the stream should not have to change, which works if you
define the files to be local to the repo being exported to. That is,
in git the line "feature export-marks=out.marks" would result in a
marks file located in "/path/to/repo/.git/fast-import/out.marks". Or
is that not what you mean?
> +1. By forcing tools to know about options specific to them, we avoid a
> range of bugs processing newer streams with older tools.
It is not possible to change the semantics using options though, what
kind of bugs could arise this way?
> I don't think options should be permitted to change import behavior. In
> other words, we should actively discourage vcs-specific streams
Sounds fair, I reckon that a wiki in addition to the
vcs-fast-import-devs list would not hurt :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC PATCH 06/19] Factor ref updating out of fetch_with_import
From: Sverre Rabbelier @ 2009-10-30 12:57 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0910300221290.14365@iabervon.org>
On Fri, Oct 30, 2009 at 00:10, Daniel Barkalow <barkalow@iabervon.org> wrote:
> Actually, I think the problem is that builtin-clone will always default to
> setting up a refspec of "refs/heads/*:refs/remotes/origin/*", but that
> doesn't actually make any sense if the source repository isn't presented
> as having refs names like "refs/heads/*". The immediate problem that you
> don't get HEAD because it's a symref to something outside the pattern is
> somewhat secondary to the general problem that you don't get anything at
> all, because everything's outside the pattern.
Ah, I didn't realize that as long as HEAD is a symref to something in
refs/heads/* it would be fetched.
> I don't really think that presenting the real refs in refs/<vcs>/*, and
> having the list give symrefs to them from refs/heads/* and refs/tags/*
> really makes sense; I think it would be better to rework fetch_with_import
> and the list provided by a foreign vcs helper such that it can present
> refs with normal names (refs/heads/*, refs/tags/*, etc) if the foreign vcs
> has standards that correspond to branches and tags. Then "clone" would
> work normally.
Perhaps we could introduce an additional phase before import to set
the default refspec? If we could tell git that we want
"refs/heads/*:refs/hg/origin/*" before line 468 "if (option_bare) {",
that would save us a lot of trouble. Does that sound like a good idea?
It would mean we have to move the transport_get part up a bit, but I
don't think that will be a problem. A svn helper for example might
respond the following to the "refspec" command:
"refs/heads/trunkr:refs/svn/origin/trunk"
"refs/tags/*:refs/svn/origin/tags/*"
"refs/heads/*:refs/svn/origin/branches/*"
How does that sound?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 7/8] Provide a build time default-editor setting
From: Jonathan Nieder @ 2009-10-30 13:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091030103558.GH1610@progeny.tock>
Jonathan Nieder wrote:
> Signed-off-by: Ben Walton <bwalton@bwalton@artsci.utoronto.ca>
That should be "Ben Walton <bwalton@artsci.utoronto.ca>", without the
extra bwalton@. Sorry for the trouble.
Regards,
Jonathan
^ permalink raw reply
* Re: [PATCH] clone: detect extra arguments
From: Jeff King @ 2009-10-30 14:45 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20091030111919.GA13242@progeny.tock>
On Fri, Oct 30, 2009 at 06:19:19AM -0500, Jonathan Nieder wrote:
> > Should we maybe be showing the usage in this case?
>
> Sounds reasonable. How about this patch on top?
I do think it's an improvement, but...
> -- %< --
> Subject: [PATCH] clone: print usage on wrong number of arguments
>
> git clone's short usage string is only 22 lines, so an error
> message plus usage string still fits comfortably on an 80x24
> terminal.
The extra blank lines introduced by usage_msg_opt make it 25 lines,
scrolling the message right off of my terminal screen. ;)
But looking at the usage message, there is some potential for cleanup.
So maybe this on top (or between your 1 and 2)?
-- >8 --
Subject: [PATCH] clone: hide "naked" option from usage message
This is just a little-known synonym for bare, and there is
little point in advertising both (we don't even include it
in the manpage). Removing it also makes the usage message
one line shorter, giving just enough room for an information
message in a 24-line terminal.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-clone.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index 736d9e1..ce0d79a 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -51,7 +51,9 @@ static struct option builtin_clone_options[] = {
OPT_BOOLEAN('n', "no-checkout", &option_no_checkout,
"don't create a checkout"),
OPT_BOOLEAN(0, "bare", &option_bare, "create a bare repository"),
- OPT_BOOLEAN(0, "naked", &option_bare, "create a bare repository"),
+ { OPTION_BOOLEAN, 0, "naked", &option_bare, NULL,
+ "create a bare repository",
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
OPT_BOOLEAN(0, "mirror", &option_mirror,
"create a mirror repository (implies bare)"),
OPT_BOOLEAN('l', "local", &option_local,
--
1.6.5.1.143.g1dab74.dirty
^ permalink raw reply related
* Re: [PATCH] clone: detect extra arguments
From: Jeff King @ 2009-10-30 14:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johan Herland, Jonathan Nieder, git
In-Reply-To: <20091030144525.GA22583@coredump.intra.peff.net>
On Fri, Oct 30, 2009 at 10:45:25AM -0400, Jeff King wrote:
> But looking at the usage message, there is some potential for cleanup.
Also, we should probably do this (I did it as a patch on master, though,
as it is an independent fix):
-- >8 --
Subject: [PATCH] clone: fix --recursive usage message
Looks like a mistaken cut-and-paste in e7fed18a.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-clone.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index 5762a6f..436e8da 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -61,7 +61,7 @@ static struct option builtin_clone_options[] = {
OPT_BOOLEAN('s', "shared", &option_shared,
"setup as shared repository"),
OPT_BOOLEAN(0, "recursive", &option_recursive,
- "setup as shared repository"),
+ "initialize submodules in the clone"),
OPT_STRING(0, "template", &option_template, "path",
"path the template repository"),
OPT_STRING(0, "reference", &option_reference, "repo",
--
1.6.5.1.143.g1dab74.dirty
^ permalink raw reply related
* Re: [RFC PATCH v4 11/26] Move WebDAV HTTP push under remote-curl
From: Tay Ray Chuan @ 2009-10-30 15:02 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Clemens Buchacher, Daniel Barkalow, Mike Hommey
In-Reply-To: <1256774448-7625-12-git-send-email-spearce@spearce.org>
Hi,
On Thu, Oct 29, 2009 at 8:00 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> update http tests according to remote-curl capabilities
it would be great if you could mention the $ORIG_HEAD bit:
o Use a variable ($ORIG_HEAD) instead of full SHA-1 name.
--
Cheers,
Ray Chuan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox