* 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: [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] 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: 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
* [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
* [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 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 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 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 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 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 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 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 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
* 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
* Re: My custom cccmd
From: Felipe Contreras @ 2009-10-30 8:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzl7c4i81.fsf@alter.siamese.dyndns.org>
On Tue, Oct 27, 2009 at 11:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> I explored this a bit more and I've come to the conclusion that we
>> actually don't wand to discard the previous commits in the patch
>> series. Let's think about this example:
>> 0001 <- John
>> 0002 <- Me overriding some changes from John
>>
>> In this case we want John to appear in the CC list of 0002, because we
>> are changing his code.
>
> There are two cases: your patch partially overrides his code, and your
> patch completely rewrites/removes his code.
>
> Obviously in the former case you do want to Cc, but there are parts of his
> change that remain in the final result, so this case does not matter in
> this discussion. You will Cc him because of these remaining lines anyway.
>
> In the latter case, the only contribution that remains from him is a
> commit with his log message that does not help describing anything in the
> end result, cluttering the history.
>
> In such a case, I would imagine that the reviewers would want to see a
> cleaned up series that does not have his patch that is irrelevant for
> understanding the final result. John might want to know about it, if only
> to raise objection to the way how you arranged your series. For that
> reason, I think it makes sense to Cc him.
>
> But it is a rather a convoluted logic, if you ask me. You find John and
> Cc him, primarily so that he can point out that you should be redoing the
> series not to have his patch as an intermediate state in the series to
> begin with, because his commit does not contribute to the end result.
>
> It might make more sense if your tool told you about such a case directly,
> rather than helping you find John so that he can tell you ;-).
But that's not the purpose of the cccmd tool.
I agree that such "patch series simplificator" tool would be very
useful, but that's out of scope for this. Isn't it?
--
Felipe Contreras
^ permalink raw reply
* Re: keeping track of where a patch begins
From: Thomas Rast @ 2009-10-30 8:37 UTC (permalink / raw)
To: pascal; +Cc: Junio C Hamano, Nicolas Pitre, E R, git, Jeff King
In-Reply-To: <4AEA94EB.8080304@obry.net>
Pascal Obry wrote:
>
> Le 22/10/2009 10:27, Thomas Rast a écrit :
> > I think this not only changes the model of branches, but also commits,
> > to some extent. Currently, commit have no intrinsic branch
> > membership
[...]
> To me there is case where it is important to know which are the commits
> done on a topic branch for example. When working on multiple topic it is
> difficult to remember which commits have been done on this specific
> branch. This is needed to rebase onto:
>
> $ git rebase --onto somebranch <topic_base> <topic_head>
>
> A common idiom, but one as to think hard (& right) to properly get the
> topic_base today.
But how frequently do your topics start on other topics? Otherwise
they will start on an integration or maint branch, and in the git.git
model where each integration branch is contained in the next "less
stable" one, that means you can just specify 'pu' or equivalent.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 12/19] 1/2: Add Python support library for CVS remote helper
From: Johan Herland @ 2009-10-30 8:33 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Git List, Johannes Schindelin, Daniel Barkalow, David Aguilar
In-Reply-To: <1256839287-19016-13-git-send-email-srabbelier@gmail.com>
On Thursday 29 October 2009, Sverre Rabbelier wrote:
> From: Johan Herland <johan@herland.net>
>
> This patch introduces parts of a Python package called "git_remote_cvs"
> containing the building blocks of the CVS remote helper.
> The CVS remote helper itself is NOT part of this patch.
>
> This patch has been improved by the following contributions:
> - David Aguilar: Lots of Python coding style fixes
>
> Cc: David Aguilar <davvid@gmail.com>
> Signed-off-by: Johan Herland <johan@herland.net>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
> This has my patch to util.py squashed in.
Why? Or: why that one, and not the others? Also, you might want to mention
your contribution in the commit message itself.
> diff --git a/git_remote_cvs/util.py b/git_remote_cvs/util.py
> new file mode 100644
> index 0000000..d3ca487
> --- /dev/null
> +++ b/git_remote_cvs/util.py
> @@ -0,0 +1,194 @@
[snip]
> +
> +def notify(msg, *args):
> + """Print a message to stderr."""
> + print >> sys.stderr, msg % args
> +
> +def debug (msg, *args):
> + """Print a debug message to stderr when DEBUG is enabled."""
> + if DEBUG:
> + print >> sys.stderr, msg % args
> +
> +def error (msg, *args):
> + """Print an error message to stderr."""
> + print >> sys.stderr, "ERROR:", msg % args
> +
> +def warn(msg, *args):
> + """Print a warning message to stderr."""
> + print >> sys.stderr, "warning:", msg % args
> +
> +def die (msg, *args):
> + """Print as error message to stderr and exit the program."""
> + error(msg, *args)
> + sys.exit(1)
> +
> +
It seems the two functions you add (notify() and warn()) have a different
indentation than the existing code (which uses 4 spaces). Please fix.
(When I first introduced these Python patches, there was a discussion on the
differences in indentation/style between the Git C code, and Python code,
and it was decided to follow the Python conventions, to make the code more
inviting to the Python community.)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: keeping track of where a patch begins
From: Pascal Obry @ 2009-10-30 7:25 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, Nicolas Pitre, E R, git, Jeff King
In-Reply-To: <200910221027.32739.trast@student.ethz.ch>
Le 22/10/2009 10:27, Thomas Rast a écrit :
> I think this not only changes the model of branches, but also commits,
> to some extent. Currently, commit have no intrinsic branch
> membership; if you say
>
> git branch foo bar
>
> you cannot distinguish whether the commits on 'bar' were created on
> 'foo' or on 'bar'. (By git's means; of course the decision would
> favour 'master' if I had used that instead.)
I have been looking for a way to know that. I've even post a question
about this on this mailing-list long time ago IIRC.
To me there is case where it is important to know which are the commits
done on a topic branch for example. When working on multiple topic it is
difficult to remember which commits have been done on this specific
branch. This is needed to rebase onto:
$ git rebase --onto somebranch <topic_base> <topic_head>
A common idiom, but one as to think hard (& right) to properly get the
topic_base today.
Just my 2 cents!
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net - http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B
^ permalink raw reply
* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Johan Herland @ 2009-10-30 8:21 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <1256839287-19016-11-git-send-email-srabbelier@gmail.com>
On Thursday 29 October 2009, Sverre Rabbelier wrote:
> From: Johan Herland <johan@herland.net>
>
> The 'marks' capability is reported by the remote helper if it requires
> the fast-import marks database to loaded/saved by any git-fast-import
> process that is provided by the transport machinery. The feature is
> advertised along with exactly one argument: the location of the file
> containing the marks database.
>
> Signed-off-by: Johan Herland <johan@herland.net>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>
> Unchanged.
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.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Gisle Aas @ 2009-10-30 8:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vws2d4y3d.fsf@alter.siamese.dyndns.org>
On Thu, Oct 29, 2009 at 23:47, Junio C Hamano <gitster@pobox.com> wrote:
> Gisle Aas <gisle@aas.no> writes:
>
>> Also make the examples show what 'git describe' actually outputs
>> currently. I guess the default --abbrev value has been changed from 4
>> to 7 at some point.
>
> Some are good changes, but I do not think the example with --abbrev=4 is.
>
> $ git describe 975bf9cf5ad5d440f98f464ae8124609a4835ce1
> v1.3.2-216-g975bf9c
> $ git describe 975b31dc6e12fba8f7b067ddbe32230995e05400
> v1.0.0-21-g975b31d
>
> Next time somebody adds a new object whose name happens to begin with
> 975b3 you would need to update the example output.
Yeah, I know, but I don't think that's a big deal. So do you want an
updated patch for that? We could either simply remove this example or
make it use --abbrev=10 or something like that.
--Gisle
>
>> Signed-off-by: Gisle Aas <gisle@aas.no>
>> ---
>> Documentation/git-describe.txt | 12 +++++++-----
>> 1 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
>> index b231dbb..743eb95 100644
>> --- a/Documentation/git-describe.txt
>> +++ b/Documentation/git-describe.txt
>> @@ -44,7 +44,9 @@ OPTIONS
>>
>> --abbrev=<n>::
>> Instead of using the default 7 hexadecimal digits as the
>> - abbreviated object name, use <n> digits.
>> + abbreviated object name, use <n> digits or as many digits
>> + are needed to form a unique object name. An <n> of 0
>> + will suppress long format, only showing the closest tag.
>>
>> --candidates=<n>::
>> Instead of considering only the 10 most recent tags as
>> @@ -68,8 +70,8 @@ OPTIONS
>> This is useful when you want to see parts of the commit object name
>> in "describe" output, even when the commit in question happens to be
>> a tagged version. Instead of just emitting the tag name, it will
>> - describe such a commit as v1.2-0-deadbeef (0th commit since tag v1.2
>> - that points at object deadbeef....).
>> + describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2
>> + that points at object deadbee....).
>>
>> --match <pattern>::
>> Only consider tags matching the given pattern (can be used to avoid
>> @@ -106,10 +108,10 @@ With --all, the command can use branch heads as
>> references, so
>> the output shows the reference path as well:
>>
>> [torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2
>> - tags/v1.0.0-21-g975b
>> + tags/v1.0.0-21-g975b3
>>
>> [torvalds@g5 git]$ git describe --all HEAD^
>> - heads/lt/describe-7-g975b
>> + heads/lt/describe-7-g975b31d
>>
>> With --abbrev set to 0, the command can be used to find the
>> closest tagname without any suffix:
>> --
>> 1.6.2.95.g934f7
>
^ permalink raw reply
* Re: [RFC PATCH 06/19] Factor ref updating out of fetch_with_import
From: Daniel Barkalow @ 2009-10-30 7:10 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1256839287-19016-7-git-send-email-srabbelier@gmail.com>
On Thu, 29 Oct 2009, Sverre Rabbelier wrote:
> Also allow the new update_refs to actually update the refs set, this
> way the remote helper can set the value of previously unknown refs.
>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
> Daniel, if we can get wanted_peer_refs to keep HEAD as a
> wanted ref somehow this patch could be a lot simpler.
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.
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.
Or, perhaps, there should be some other way of allowing git users to take
advantage of foreign vcs standards; I don't have a good perspective on
this, since I only presently use git and a foreign vcs without any useful
standards. But I think it should be such that the transport user sees as
close as possible to a native git layout, and clone continues to rely on
the layout being normal, instead of presenting a weird layout mapped into
a normal layout or something like that and working around transport users
not expecting this.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: git svn show-ignore is excrutiatingly slow
From: Eric Wong @ 2009-10-30 6:39 UTC (permalink / raw)
To: Adam Spiers; +Cc: git mailing list
In-Reply-To: <20091028174307.GA5691@atlantic.linksys.moosehall>
Adam Spiers <git@adamspiers.org> wrote:
> Mail-Followup-To: git mailing list <git@vger.kernel.org>
Hi Adam, MFT is frowned upon here.
> Something is badly wrong here ...
>
> $ cd $svn_wd
> $ time svn propget -R svn:ignore >/dev/null
> svn propget -R svn:ignore > /dev/null 0.28s user 0.20s system 98% cpu 0.490 total
> $ cd $git_wd
> $ time git svn show-ignore > show-ignore.out
> git svn show-ignore > show-ignore.out 20.52s user 33.69s system 1% cpu 1:23:42.17 total
>
> That's 10,000 times slower for what is effectively the same source
> tree! Admittedly the svn propget was a "warm" run and took longer the
> first time around, but even so there are several orders of magnitude
> difference.
It's the difference between reading locally vs remotely. git svn always
looks at the remote repository for this information. In your example,
svn was looking at the working copy where it already has that
information in an easily accessible form.
Try running svn against your repository URL instead for a comparison:
time svn propget -R svn:ignore $SVN_URL >/dev/null
> I had a quick look at the code and it seemed to be doing the svn tree
> recursion itself via Git::SVN::prop_walk(), which might explain why.
> However I did not have time to dig deeper, so would welcome any ideas.
It would be possible to reconstruct the information given untouched
copies of unhandled.log inside $GIT_DIR/svn/**. On the other hand, it
does require running through the history of the project and I don't
think it's worth it for an operation that should be rarely needed. I
designed the output of "git svn show-ignore" be stored in
$GIT_DIR/info/exclude
--
Eric Wong
^ permalink raw reply
* Re: [UNSTABLE PATCH 03/19] Allow programs to not depend on remotes having urls
From: Daniel Barkalow @ 2009-10-30 6:02 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1256839287-19016-4-git-send-email-srabbelier@gmail.com>
On Thu, 29 Oct 2009, Sverre Rabbelier wrote:
> From: Daniel Barkalow <barkalow@iabervon.org>
>
> For fetch and ls-remote, which use the first url of a remote, have
> transport_get() determine this by passing a remote and passing NULL
> for the url. For push, which uses every url of a remote, use each url
> in turn if there are any, and use NULL if there are none.
>
> This will allow the transport code to do something different if the
> location is not specified with a url.
>
> Also, have the message for a fetch say "foreign" if there is no url.
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
> I rebased this on master and had major conflicts with the
> recent 'advice' series, Daniel, please have a look at this to
> see whether it is sane at all.
Yup, this is right. 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, to avoid having to deal with this
sort of conflict every time someone touches this section of code, which
has happened several times recently now. Anyway, you correctly understood
the intended change here.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH 02/19] Allow fetch to modify refs
From: Daniel Barkalow @ 2009-10-30 5:56 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1256839287-19016-3-git-send-email-srabbelier@gmail.com>
On Thu, 29 Oct 2009, Sverre Rabbelier wrote:
> From: Daniel Barkalow <barkalow@iabervon.org>
>
> This allows the transport to use the null sha1 for a ref reported to
> be present in the remote repository to indicate that a ref exists but
> its actual value is presently unknown and will be set if the objects
> are fetched.
>
> Also adds documentation to the API to specify exactly what the methods
> should do and how they should interpret arguments.
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
> Signed off by me because I rebased it on master and fixed the
> conflicts with nico's patch.
>
> builtin-clone.c | 5 +++--
> transport-helper.c | 4 ++--
> transport.c | 13 +++++++------
> transport.h | 41 +++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/builtin-clone.c b/builtin-clone.c
> index 5762a6f..0042bee 100644
> --- a/builtin-clone.c
> +++ 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.
-Daniel
*This .sig left intentionally blank*
^ 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