Re: [PATCH 2/2] mergetools: Make tortoisemerge work with
On Fri, Jan 25, 2013 at 5:17 PM, Sven Strickroth wrote: > TortoiseGitMerge now separates cli parameter key-values by space instead > of colons as TortoiseSVN TortoiseMerge does and supports filesnames > with spaces in it this way now. These patches look correct (I do not have the tool to test) but I think we should fixup this commit message. How about something like... mergetools: Teach tortoisemerge about TortoiseGitMerge TortoiseGitMerge improved its syntax to allow for file paths with spaces. Detect when it is installed and prefer it over TortoiseMerge. -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/7] mergetool--lib: don't call "exit" in setup_tool
On Fri, Jan 25, 2013 at 4:24 PM, Junio C Hamano wrote: > Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break > t7610 rather badly. I just sent a replacement for the vim/symlink issue stuff. I tried to keep the patch small. John, can you rebase this patch on top of it? > --- >8 -- >8 -- >8 -- >8 -- >8 -- >8 --- > ... > ok 1 - setup > > expecting success: > git checkout -b test1 branch1 && > git submodule update -N && > test_must_fail git merge master >/dev/null 2>&1 && > ( yes "" | git mergetool both >/dev/null 2>&1 ) && > ( yes "" | git mergetool file1 file1 ) && > ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && > ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && > ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && > ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && > ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && > test "$(cat file1)" = "master updated" && > test "$(cat file2)" = "master new" && > test "$(cat subdir/file3)" = "master new sub" && > test "$(cat submod/bar)" = "branch1 submodule" && > git commit -m "branch1 resolved with mergetool" > > M submod > Switched to a new branch 'test1' > Submodule path 'submod': checked out > '39c7f044ed2e6a9cebd5266529badd181c8762b5' > not ok - 2 custom mergetool > # > # git checkout -b test1 branch1 && > # git submodule update -N && > # test_must_fail git merge master >/dev/null 2>&1 && > # ( yes "" | git mergetool both >/dev/null 2>&1 ) && > # ( yes "" | git mergetool file1 file1 ) && > # ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && > # ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && > # ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && > # ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && > # ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && > # test "$(cat file1)" = "master updated" && > # test "$(cat file2)" = "master new" && > # test "$(cat subdir/file3)" = "master new sub" && > # test "$(cat submod/bar)" = "branch1 submodule" && > # git commit -m "branch1 resolved with mergetool" > # > --- 8< -- 8< -- 8< -- 8< -- 8< -- 8< --- > > Due to ">dev/null 2>&1", all of the error clues are hidden, and I > didn't dig further to see which one was failing (this is why tests > shouldn't do these in general). -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: turn on test-lint-shell-syntax by default
On 15.01.13 21:38, Junio C Hamano wrote:
> Torsten Bögershausen writes:
>
>> What do we think about something like this for fishing for which:
>>
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -644,6 +644,10 @@ yes () {
>> :
>> done
>> }
>> +which () {
>> + echo >&2 "which is not portable (please use type)"
>> + exit 1
>> +}
>>
>>
>> This will happen in runtime, which might be good enough ?
>
> if (
> which frotz &&
> test $(frobonitz --version" -le 2.0
> )
> then
> test_set_prereq FROTZ_FROBONITZ
> else
> echo >&2 "suitable Frotz/Frobonitz combo not available;"
> echo >&2 "some tests may be skipped"
> fi
>
> I somehow think this is a lost cause.
I found different ways to detect if frotz is installed in the test suite:
a) use "type"(Should be the fastest ?)
b) call the command directly, check the exit code
c) "! test_have_prereq" (easy to understand, propably most expensive ?)
Do we really need "which" to detect if frotz is installed?
/Torsten
=
if ! type cvs >/dev/null 2>&1
then
skip_all='skipping cvsimport tests, cvs not found'
test_done
fi
===
if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then
# we are in full-on bash mode
true
elif type bash >/dev/null 2>&1; then
# execute in full-on bash mode
unset POSIXLY_CORRECT
exec bash "$0" "$@"
else
echo '1..0 #SKIP skipping bash completion tests; bash not available'
exit 0
fi
===
svn >/dev/null 2>&1
if test $? -ne 1
then
skip_all='skipping git svn tests, svn not found'
test_done
fi
===
( p4 -h && p4d -h ) >/dev/null 2>&1 || {
skip_all='skipping git p4 tests; no p4 or p4d'
test_done
}
===
if ! test_have_prereq PERL; then
skip_all='skipping gitweb tests, perl not available'
test_done
fi
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mergetools: Simplify how we handle "vim" and "defaults"
Remove the exceptions for "vim" and "defaults" in the mergetool library
so that every filename in mergetools/ matches 1:1 with the name of a
valid built-in tool.
Make common functions available in $MERGE_TOOLS_DIR/include/.
Signed-off-by: David Aguilar
---
This should make things ok on platforms where we don't have symlinks.
Makefile | 2 +-
git-mergetool--lib.sh| 41 +++-
mergetools/gvimdiff | 1 +
mergetools/gvimdiff2 | 1 +
mergetools/{defaults => include/defaults.sh} | 0
mergetools/{vim => include/vim.sh} | 0
mergetools/vimdiff | 1 +
mergetools/vimdiff2 | 1 +
8 files changed, 21 insertions(+), 26 deletions(-)
create mode 100644 mergetools/gvimdiff
create mode 100644 mergetools/gvimdiff2
rename mergetools/{defaults => include/defaults.sh} (100%)
rename mergetools/{vim => include/vim.sh} (100%)
create mode 100644 mergetools/vimdiff
create mode 100644 mergetools/vimdiff2
diff --git a/Makefile b/Makefile
index f69979e..3bc6eb5 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,7 +2724,7 @@ install: all
$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
- $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+ cp -R mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
ifndef NO_GETTEXT
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
(cd po/build/locale && $(TAR) cf - .) | \
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index aa38bd1..c866ed8 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,7 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
+MERGE_TOOLS_DIR="$(git --exec-path)/mergetools"
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -44,25 +46,14 @@ valid_tool () {
}
setup_tool () {
- case "$1" in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool="$1"
- ;;
- esac
- mergetools="$(git --exec-path)/mergetools"
+ tool="$1"
- # Load the default definitions
- . "$mergetools/defaults"
- if ! test -f "$mergetools/$tool"
+ if ! test -f "$MERGE_TOOLS_DIR/$tool"
then
return 1
fi
-
- # Load the redefined functions
- . "$mergetools/$tool"
+ . "$MERGE_TOOLS_DIR/include/defaults.sh"
+ . "$MERGE_TOOLS_DIR/$tool"
if merge_mode && ! can_merge
then
@@ -99,7 +90,7 @@ run_merge_tool () {
base_present="$2"
status=0
- # Bring tool-specific functions into scope
+ # Bring tool specific functions into scope
setup_tool "$1"
if merge_mode
@@ -177,18 +168,17 @@ list_merge_tool_candidates () {
show_tool_help () {
unavailable= available= LF='
'
-
- scriptlets="$(git --exec-path)"/mergetools
- for i in "$scriptlets"/*
+ for i in "$MERGE_TOOLS_DIR"/*
do
- . "$scriptlets"/defaults
- . "$i"
-
- tool="$(basename "$i")"
- if test "$tool" = "defaults"
+ if test -d "$i"
then
continue
- elif merge_mode && ! can_merge
+ fi
+
+ . "$MERGE_TOOLS_DIR"/include/defaults.sh
+ . "$i"
+
+ if merge_mode && ! can_merge
then
continue
elif diff_mode && ! can_diff
@@ -196,6 +186,7 @@ show_tool_help () {
continue
fi
+ tool=$(basename "$i")
merge_tool_path=$(translate_merge_tool_path "$tool")
if type "$merge_tool_path" >/dev/null 2>&1
then
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
diff --git a/mergetools/defaults b/mergetools/include/defaults.sh
similarity index 100%
rename from mergetools/defaults
rename to mergetools/include/defaults.sh
diff --git a/mergetools/vim b/mergetools/include/vim.sh
similarity index 100%
rename from mergetools/vim
rename to mergetools/include/vim.sh
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/vimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
new file mode 100644
index 00
Re: [PATCH v2 1/3] branch: reject -D/-d without branch name
On Sat, Jan 26, 2013 at 2:04 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> --- >> builtin/branch.c | 6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) > > Forgot to sign-off? Yes > Is this a real problem? Yes. My thoughts yesterday when I happened to type "git branch -D": "Wait, I just entered a delete command without a branch name. It did not report anything, which in UNIX world usually means successful operation. _What_ did it delete??" I knew this code so I just went check. If I did not know, I might as well list all branches I had and see if something was missing. A short message would have saved me the trouble. > I do not see it particularly wrong to succeed after deleting 0 or > more given branch names. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mergetools: Make tortoisemerge work with
tortoisegitmerge and filesnames with space
The tortoisemerge mergetool does not work with filenames which have
a space in it. Fixing this required changes in git and also in
TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.
TortoiseGitMerge now separates cli parameter key-values by space instead
of colons as TortoiseSVN TortoiseMerge does and supports filesnames
with spaces in it this way now.
Signed-off-by: Sven Strickroth
Reported-by: Sebastian Schuberth
---
mergetools/tortoisemerge | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index 8476afa..2f98829 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -6,9 +6,17 @@ merge_cmd () {
if $base_present
then
touch "$BACKUP"
- "$merge_tool_path" \
- -base:"$BASE" -mine:"$LOCAL" \
- -theirs:"$REMOTE" -merged:"$MERGED"
+ basename="$(basename "$merge_tool_path" .exe)"
+ if test "$basename" = "tortoisegitmerge"
+ then
+ "$merge_tool_path" \
+ -base "$BASE" -mine "$LOCAL" \
+ -theirs "$REMOTE" -merged "$MERGED"
+ else
+ "$merge_tool_path" \
+ -base:"$BASE" -mine:"$LOCAL" \
+ -theirs:"$REMOTE" -merged:"$MERGED"
+ fi
check_unchanged
else
echo "$merge_tool_path cannot be used without a base" 1>&2
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mergetools: Added support for TortoiseGitMerge
The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
(starting with 1.8.0) in order to make clear that this one has special
support for git and prevent confusion with the TortoiseSVN TortoiseMerge
version.
Signed-off-by: Sven Strickroth
---
mergetools/tortoisemerge | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..8476afa 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -11,7 +11,16 @@ merge_cmd () {
-theirs:"$REMOTE" -merged:"$MERGED"
check_unchanged
else
- echo "TortoiseMerge cannot be used without a base" 1>&2
+ echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
fi
}
+
+translate_merge_tool_path() {
+ if type tortoisegitmerge >/dev/null 2>/dev/null
+ then
+ echo tortoisegitmerge
+ else
+ echo tortoisemerge
+ fi
+}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Enhance tortoisemerge to work with
The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
(starting with 1.8.0) in order to make clear that this one has special
support for git and prevent confusion with the TortoiseSVN TortoiseMerge
version.
Signed-off-by: Sven Strickroth
---
mergetools/tortoisemerge | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..8476afa 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -11,7 +11,16 @@ merge_cmd () {
-theirs:"$REMOTE" -merged:"$MERGED"
check_unchanged
else
- echo "TortoiseMerge cannot be used without a base" 1>&2
+ echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
fi
}
+
+translate_merge_tool_path() {
+ if type tortoisegitmerge >/dev/null 2>/dev/null
+ then
+ echo tortoisegitmerge
+ else
+ echo tortoisemerge
+ fi
+}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
Throughout git, it is assumed that the WIN32 preprocessor symbol is
defined on native Windows setups (mingw and msvc) and not on Cygwin.
On Cygwin, most of the time git can pretend this is just another Unix
machine, and Windows-specific magic is generally counterproductive.
Unfortunately Cygwin *does* define the WIN32 symbol in some headers.
Best to rely on a new git-specific symbol NATIVE_WINDOWS instead,
defined as follows:
#if defined(WIN32) && !defined(__CYGWIN__)
# define NATIVE_WINDOWS
#endif
After this change, it should be possible to drop the
CYGWIN_V15_WIN32API setting without any negative effect.
Signed-off-by: Jonathan Nieder
---
Eric Blake wrote:
> Which is why other projects, like gnulib, have
>
> # if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>
> all over the place.
So, how about this?
Completely untested.
abspath.c | 2 +-
compat/terminal.c | 4 ++--
compat/win32.h| 2 +-
diff-no-index.c | 2 +-
git-compat-util.h | 3 ++-
help.c| 2 +-
run-command.c | 10 +-
test-chmtime.c| 2 +-
thread-utils.c| 2 +-
9 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -3,7 +3,7 @@
#include "sigchain.h"
#include "strbuf.h"
-#if defined(HAVE_DEV_TTY) || defined(WIN32)
+#if defined(HAVE_DEV_TTY) || defined(WINDOWS_NATIVE)
static void restore_term(void);
@@ -53,7 +53,7 @@ error:
return -1;
}
-#elif defined(WIN32)
+#elif defined(WINDOWS_NATIVE)
#define INPUT_PATH "CONIN$"
#define OUTPUT_PATH "CONOUT$"
diff --git a/compat/win32.h b/compat/win32.h
index 8ce91048..31dd30f7 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -2,7 +2,7 @@
#define WIN32_H
/* common Win32 functions for MinGW and Cygwin */
-#ifndef WIN32 /* Not defined by Cygwin */
+#ifndef WINDOWS_NATIVE /* Not defined for Cygwin */
#include
#endif
diff --git a/diff-no-index.c b/diff-no-index.c
index 74da6593..a0bc9c50 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -45,7 +45,7 @@ static int get_mode(const char *path, int *mode)
if (!path || !strcmp(path, "/dev/null"))
*mode = 0;
-#ifdef _WIN32
+#ifdef WINDOWS_NATIVE
else if (!strcasecmp(path, "nul"))
*mode = 0;
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b745..ebbdff53 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,10 +85,11 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
+#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
#include
#include
+#define WINDOWS_NATIVE
#endif
#include
diff --git a/help.c b/help.c
index 2a42ec6d..cc1e63f7 100644
--- a/help.c
+++ b/help.c
@@ -106,7 +106,7 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
+#if defined(WINDOWS_NATIVE) || defined(__CYGWIN__)
#if defined(__CYGWIN__)
if ((st.st_mode & S_IXUSR) == 0)
#endif
diff --git a/run-command.c b/run-command.c
index 04712191..04ac6181 100644
--- a/run-command.c
+++ b/run-command.c
@@ -72,7 +72,7 @@ static inline void close_pair(int fd[2])
close(fd[1]);
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static inline void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
@@ -159,7 +159,7 @@ static const char **prepare_shell_cmd(const char **argv)
die("BUG: shell command is empty");
if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
nargv[nargc++] = SHELL_PATH;
#else
nargv[nargc++] = "sh";
@@ -182,7 +182,7 @@ static const char **prepare_shell_cmd(const char **argv)
return nargv;
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int execv_shell_cmd(const char **argv)
{
const char **nargv = prepare_shell_cmd(argv);
@@ -193,7 +193,7 @@ static int execv_shell_cmd(const char **argv)
}
#endif
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int child_err = 2;
static int child_notifier = -1;
@@ -330,7 +330,7 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
fflush(NULL);
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
{
int notify_pipe[2];
if (pipe(notify_pipe))
diff --git a/test-chmtime.c b/test
Re: [PATCH] mergetools: Enhance tortoisemerge to work with
Am 25.01.2013 19:28 schrieb Junio C Hamano:> Sven Strickroth writes: > >> TortoiseGitMerge and filenames with spaces > > ??? ECANNOTPARSE. > > ... ah, wait. Is this a broken-off tail of your subject line? Yes. >> +touch "$BACKUP" >> +basename="$(basename "$merge_tool_path" .exe)" >> +if test "$basename" = "tortoisegitmerge" >> +then >> +"$merge_tool_path" \ >> +-base "$BASE" -mine "$LOCAL" \ >> +-theirs "$REMOTE" -merged "$MERGED" >> +else >> +"$merge_tool_path" \ >> +-base:"$BASE" -mine:"$LOCAL" \ >> +-theirs:"$REMOTE" -merged:"$MERGED" > > Hmph. > > How was the support for "names with spaces" added in this new code? > I do not spot what is different between this "else" clause and the > original body of the merge_cmd (which only supported tortoisemerge). > > They seem to be doing exactly the same thing. Erhm, no. As already stated and also mentioned in the commit log: TortoiseMerge has cli parameter key-values separated by colons, TortoiseGitMerge has key-values separated by spaces. -- Best regards, Sven Strickroth PGP key id F5A9D4C4 @ any key-server -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] git-web--browser: avoid errors in terminal when running Firefox on Windows
Firefox on Windows by default is placed in "C:\Program Files\Mozilla Firefox"
folder, i.e. its path contains spaces. Before running this browser
"git-web--browse"
tests version of Firefox to decide whether to use "-new-tab" option or not.
Quote browser path to avoid error during this test.
Signed-off-by: Alexey Shumkin
Reviewed-by: Jeff King
---
git-web--browse.sh | 2 +-
t/t9901-git-web--browse.sh | 53 +-
2 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/git-web--browse.sh b/git-web--browse.sh
index 1e82726..f96e5bd 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -149,7 +149,7 @@ fi
case "$browser" in
firefox|iceweasel|seamonkey|iceape)
# Check version because firefox < 2.0 does not support "-new-tab".
- vers=$(expr "$($browser_path -version)" : '.* \([0-9][0-9]*\)\..*')
+ vers=$(expr "$("$browser_path" -version)" : '.* \([0-9][0-9]*\)\..*')
NEWTAB='-new-tab'
test "$vers" -lt 2 && NEWTAB=''
"$browser_path" $NEWTAB "$@" &
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index b0dabf7..c1ee813 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -8,8 +8,23 @@ This test checks that git web--browse can handle various valid
URLs.'
. ./test-lib.sh
test_web_browse () {
- # browser=$1 url=$2
+ # browser=$1 url=$2 sleep_timeout=$3
+ sleep_timeout="$3"
+ rm -f fake_browser_ran &&
git web--browse --browser="$1" "$2" >actual &&
+ # if $3 is set
+ # as far as Firefox is run in background (it is run with &)
+ # we trying to avoid race condition
+ # by waiting for "$sleep_timeout" seconds of timeout for
'fake_browser_ran' file appearance
+ if test -n "$sleep_timeout"
+ then
+ for timeout in $(test_seq $sleep_timeout)
+ do
+ test -f fake_browser_ran && break
+ sleep 1
+ done
+ test $timeout -ne $sleep_timeout
+ fi &&
tr -d '\015' text &&
test_cmp expect text
}
@@ -46,6 +61,42 @@ test_expect_success \
'
test_expect_success \
+ 'Paths are properly quoted for Firefox. Version older then v2.0' '
+ echo "fake: http://example.com/foo"; >expect &&
+ write_script "fake browser" <<-\EOF &&
+
+ if test "$1" = "-version"; then
+ echo "Fake Firefox browser version 1.2.3"
+ else
+ # Firefox (in contrast to w3m) is run in background (with &)
+ # so redirect output to "actual"
+ echo "fake: ""$@" >actual
+ fi
+ : >fake_browser_ran
+ EOF
+ git config browser.firefox.path "$(pwd)/fake browser" &&
+ test_web_browse firefox http://example.com/foo 5
+'
+
+test_expect_success \
+ 'Paths are properly quoted for Firefox. Version v2.0 and above' '
+ echo "fake: -new-tab http://example.com/foo"; >expect &&
+ write_script "fake browser" <<-\EOF &&
+
+ if test "$1" = "-version"; then
+ echo "Fake Firefox browser version 2.0.0"
+ else
+ # Firefox (in contrast to w3m) is run in background (with &)
+ # so redirect output to "actual"
+ echo "fake: ""$@" >actual
+ fi
+ : >fake_browser_ran
+ EOF
+ git config browser.firefox.path "$(pwd)/fake browser" &&
+ test_web_browse firefox http://example.com/foo 5
+'
+
+test_expect_success \
'browser command allows arbitrary shell code' '
echo "arg: http://example.com/foo"; >expect &&
git config browser.custom.cmd "
--
1.8.1.1.10.g71fa0b7
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] t9901-git-web--browse.sh: Use "write_script" helper
Use "write_script" helper as suggested by Junio C Hamano. Also, replace `pwd` with $(pwd) call convention. Suggested-by: Junio C Hamano Signed-off-by: Alexey Shumkin --- t/t9901-git-web--browse.sh | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh index b0a6bad..b0dabf7 100755 --- a/t/t9901-git-web--browse.sh +++ b/t/t9901-git-web--browse.sh @@ -38,12 +38,10 @@ test_expect_success \ test_expect_success \ 'browser paths are properly quoted' ' echo fake: http://example.com/foo >expect && - cat >"fake browser" <<-\EOF && - #!/bin/sh + write_script "fake browser" <<-\EOF && echo fake: "$@" EOF - chmod +x "fake browser" && - git config browser.w3m.path "`pwd`/fake browser" && + git config browser.w3m.path "$(pwd)/fake browser" && test_web_browse w3m http://example.com/foo ' -- 1.8.1.1.10.g71fa0b7 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] git-web--browser: avoid errors in terminal when running
Reroll patch after all suggestions Alexey Shumkin (2): t9901-git-web--browse.sh: Use "write_script" helper git-web--browser: avoid errors in terminal when running Firefox on Windows git-web--browse.sh | 2 +- t/t9901-git-web--browse.sh | 59 ++ 2 files changed, 55 insertions(+), 6 deletions(-) -- 1.8.1.1.10.g71fa0b7 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Version 1.8.1 does not compile on Cygwin 1.7.14
On 01/25/2013 05:11 PM, Junio C Hamano wrote: > Mark Levedahl writes: > >> Cygwin and Windows should be treated as completely separate platforms: >> if __CYGWIN__ is defined, do one thing, if not, go ahead and check >> WIN32, but the WIN32 macro should never be tested once we know the >> platform is CYGWIN - these really are different platforms (if you are >> unsure of this, consider that Cygwin includes a cross-compiler to >> target native Win32 as the Cygwin maintainers recognized the platforms >> are different). > > Not disagreeing with your conclusion (they should be treated as > different), why does it define WIN32 in the first place? > > Perhaps we would want > > #ifdef __CYGWIN__ > #undef WIN32 > #endif Wouldn't work. Cygwin gcc does NOT define WIN32; rather, the inclusion of a Windows system header (generally discouraged, but sometimes a necessary evil) might cause WIN32 to be defined for all subsequent headers. Which is why other projects, like gnulib, have # if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ all over the place. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 8/7] mergetool--lib: don't call "exit" in setup_tool
Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break t7610 rather badly. --- >8 -- >8 -- >8 -- >8 -- >8 -- >8 --- ... ok 1 - setup expecting success: git checkout -b test1 branch1 && git submodule update -N && test_must_fail git merge master >/dev/null 2>&1 && ( yes "" | git mergetool both >/dev/null 2>&1 ) && ( yes "" | git mergetool file1 file1 ) && ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && test "$(cat submod/bar)" = "branch1 submodule" && git commit -m "branch1 resolved with mergetool" M submod Switched to a new branch 'test1' Submodule path 'submod': checked out '39c7f044ed2e6a9cebd5266529badd181c8762b5' not ok - 2 custom mergetool # # git checkout -b test1 branch1 && # git submodule update -N && # test_must_fail git merge master >/dev/null 2>&1 && # ( yes "" | git mergetool both >/dev/null 2>&1 ) && # ( yes "" | git mergetool file1 file1 ) && # ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && # ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && # ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && # ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && # ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && # test "$(cat file1)" = "master updated" && # test "$(cat file2)" = "master new" && # test "$(cat subdir/file3)" = "master new sub" && # test "$(cat submod/bar)" = "branch1 submodule" && # git commit -m "branch1 resolved with mergetool" # --- 8< -- 8< -- 8< -- 8< -- 8< -- 8< --- Due to ">dev/null 2>&1", all of the error clues are hidden, and I didn't dig further to see which one was failing (this is why tests shouldn't do these in general). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jan 2013, #09; Fri, 25)
What's cooking in git.git (Jan 2013, #09; Fri, 25) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. As usual, this cycle is expected to last for 8 to 10 weeks, with a preview -rc0 sometime in the middle of next month. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * as/check-ignore (2013-01-16) 13 commits (merged to 'next' on 2013-01-18 at ef45aff) + clean.c, ls-files.c: respect encapsulation of exclude_list_groups (merged to 'next' on 2013-01-14 at 9df2afc) + t0008: avoid brace expansion + add git-check-ignore sub-command + setup.c: document get_pathspec() + add.c: extract new die_if_path_beyond_symlink() for reuse + add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse + pathspec.c: rename newly public functions for clarity + add.c: move pathspec matchers into new pathspec.c for reuse + add.c: remove unused argument from validate_pathspec() + dir.c: improve docs for match_pathspec() and match_pathspec_depth() + dir.c: provide clear_directory() for reclaiming dir_struct memory + dir.c: keep track of where patterns came from + dir.c: use a single struct exclude_list per source of excludes Add a new command "git check-ignore" for debugging .gitignore files. The variable names may want to get cleaned up but that can be done in-tree. * as/pre-push-hook (2013-01-18) 3 commits (merged to 'next' on 2013-01-18 at 37fc4e8) + Add sample pre-push hook script + push: Add support for pre-push hooks + hooks: Add function to check if a hook exists Add an extra hook so that "git push" that is run without making sure what is being pushed is sane can be checked and rejected (as opposed to the user deciding not pushing). * ch/add-auto-submitted-in-sample-post-receive-email (2013-01-17) 1 commit (merged to 'next' on 2013-01-18 at e3205db) + Add Auto-Submitted header to post-receive-email Mark e-mails coming from automated processes should be marked as such; update a sample hook to do so. * cr/push-force-tag-update (2013-01-16) 1 commit (merged to 'next' on 2013-01-18 at c9091d5) + push: fix "refs/tags/ hierarchy cannot be updated without --force" (this branch is used by jc/push-reject-reasons.) Regression fix, not to say "already exists" when we traditionally said "non fast-forward'. * jc/doc-maintainer (2013-01-03) 2 commits (merged to 'next' on 2013-01-11 at f35d582) + howto/maintain: mark titles for asciidoc + Documentation: update "howto maintain git" Describe tools for automation that were invented since this document was originally written. * jk/suppress-clang-warning (2013-01-16) 1 commit (merged to 'next' on 2013-01-18 at 7c0bda7) + fix clang -Wunused-value warnings for error functions * mh/imap-send-shrinkage (2013-01-15) 14 commits (merged to 'next' on 2013-01-18 at 1b7c5ba) + imap-send.c: simplify logic in lf_to_crlf() + imap-send.c: fold struct store into struct imap_store + imap-send.c: remove unused field imap_store::uidvalidity + imap-send.c: use struct imap_store instead of struct store + imap-send.c: remove unused field imap_store::trashnc + imap-send.c: remove namespace fields from struct imap + imap-send.c: remove struct imap argument to parse_imap_list_l() + imap-send.c: inline parse_imap_list() in parse_list() + imap-send.c: remove some unused fields from struct store + imap-send.c: remove struct message + imap-send.c: remove struct store_conf + iamp-send.c: remove unused struct imap_store_conf + imap-send.c: remove struct msg_data + imap-send.c: remove msg_data::flags, which was always zero Remove a lot of unused code from "git imap-send". * mo/cvs-server-updates (2012-12-09) 18 commits (merged to 'next' on 2013-01-08 at 75e2d11) + t9402: Use TABs for indentation + t9402: Rename check.cvsCount and check.list + t9402: Simplify git ls-tree + t9402: Add missing &&; Code style + t9402: No space after IO-redirection + t9402: Dont use test_must_fail cvs + t9402: improve check_end_tree() and check_end_full_tree() + t9402: sed -i is not portable + cvsserver Documentation: new cvs ... -r support + cvsserver: add t9402 to test branch and tag refs + cvsserver: support -r and sticky tags for most operations + cvsserver: Add version awareness to argsfromdir + cvsserver: generalize getmeta() to recognize commit refs + cvsserver: implement req_Sticky and related utilities + cvsserver: add misc commit lookup, file meta data, and file listing functions + cvsserver: define a tag name character escape mechanism + cvsserver: cleanup extra slashes in filename arguments + cvsserver: factor out git-log parsing logic Various git-cvsserver updates. * nd/re
Re: Version 1.8.1 does not compile on Cygwin 1.7.14
Mark Levedahl writes: > Cygwin and Windows should be treated as completely separate platforms: > if __CYGWIN__ is defined, do one thing, if not, go ahead and check > WIN32, but the WIN32 macro should never be tested once we know the > platform is CYGWIN - these really are different platforms (if you are > unsure of this, consider that Cygwin includes a cross-compiler to > target native Win32 as the Cygwin maintainers recognized the platforms > are different). Not disagreeing with your conclusion (they should be treated as different), why does it define WIN32 in the first place? Perhaps we would want #ifdef __CYGWIN__ #undef WIN32 #endif very early in some include file before nothing else is included? Just being curious. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Version 1.8.1 does not compile on Cygwin 1.7.14
On 01/22/2013 01:31 PM, Ramsay Jones wrote:
include order. ;-) As I have mentioned here before, the claim that
"WIN32 is not defined on cygwin" is simply nonsense - it depends on
if/when certain header files are included. For example, *as soon as*
you include (and, I suspect, many other win32 headers)
then "defined(WIN32)" is true. Note that commit 380a4d92 ("Update
cygwin.c for new mingw-64 win32 api headers", 11-11-2012) swaps the
include order for the win32.h and git-compat-util.h header files. [I
don't know the details, Mark didn't elaborate, but it is clearly an
include order problem on cygwin 1.7.x :-D ] This causes compilation
errors on cygwin 1.5.x, exactly because win32.h includes ,
which defines WIN32, which then leads to git-compat-util.h including
.
#if defined(WIN32) && defined(__CYGWIN__)
# undef WIN32
#endif
Cygwin and Windows should be treated as completely separate platforms:
if __CYGWIN__ is defined, do one thing, if not, go ahead and check
WIN32, but the WIN32 macro should never be tested once we know the
platform is CYGWIN - these really are different platforms (if you are
unsure of this, consider that Cygwin includes a cross-compiler to target
native Win32 as the Cygwin maintainers recognized the platforms are
different).
Mark
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows
2013/1/26 Jeff King :
> On Fri, Jan 25, 2013 at 06:44:13PM +0400, Alexey Shumkin wrote:
>
>> test_web_browse () {
>> - # browser=$1 url=$2
>> + # browser=$1 url=$2 sleep_timeout=$3
>> + sleep_timeout="$3"
>> git web--browse --browser="$1" "$2" >actual &&
>> + # if $3 is set
>> + # as far as Firefox is run in background (it is run with &)
>> + # we trying to avoid race condition
>> + # by waiting for "$sleep_timeout" seconds of timeout for
>> 'fake_browser_ran' file appearance
>> + (test -z "$sleep_timeout" || (
>> + for timeout in $(seq 1 $sleep_timeout); do
>> + test -f fake_browser_ran && break
>> + sleep 1
>> + done
>> + test $timeout -ne $sleep_timeout
>> + )
>> + ) &&
>> tr -d '\015' text &&
>
> Gross, but I don't really see another way to handle the asynchronous
> nature of spawning background browsers.
>
> Two things, though:
>
> 1. Should test_web_browse just delete fake_browser_ran for us? Then
> later tests do not have to remember to do so.
Yep, you're right
>
> 2. Seeing fake_browser_ran appeared, we know that the script has
> started. But there is still a race condition in which it may not
> have written anything to "actual" yet.
Definitely right
>
> In this implementation:
>
>> + cat >"fake browser" <<-\EOF &&
>> + #!/bin/sh
>> +
>> + : > fake_browser_ran
>> + if test "$1" = "-version"; then
>> + echo Fake Firefox browser version 1.2.3
>> + else
>> + # Firefox (in contrast to w3m) is run in background (with
>> &)
>> + # so redirect output to "actual"
>> + echo fake: "$@" > actual
>> + fi
>> + EOF
>
> There is a period where fake_browser_ran exists, but nothing is in
> actual. You can solve it by setting fake_browser_ran at the end rather
> than the beginning.
>
> Or you can drop fake_browser_ran entirely, and just atomically move
> actual into place, like:
>
> echo "fake: $*" >actual.tmp
> mv actual.tmp actual
>
> and then tes-t_web_browse can just spin waiting for "actual" to appear.
Not exactly, because, as I see, "actual" file is a result of redirection of
> git web--browse --browser="$1" "$2" >actual &&
command
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] test-lint-duplicates: check numbering in contrib/remote-helpers
Running make inside contrib/remote-helpers failes in "test-lint-duplicates"
This was because the regexp to check for duplicate numbers strips everything
after the first "-" in the filename, including the prefix.
As a result, 2 pathnames like
"/contrib/remote-helpers/test-bzr.sh" and
"/contrib/remote-helpers/test-hg-bidi.sh"
are both converted into
"/contrib/remote" and reported as duplicate.
Improve the regexp:
Remove everything after "t-" (where X stand for a digit)
Rename the tests in contrib/remote-helpers into
t5810-test-bzr.sh,
t5820-test-hg-bidi.sh,
t5821-test-hg-hg-git.sh,
t5830-test-hg.sh
Feed the numbers used in contrib/remote-helpers into t/Makefile
and check that the numbering is unique across both directories
Signed-off-by: Torsten Bögershausen
---
contrib/remote-helpers/Makefile| 3 +-
contrib/remote-helpers/t5810-test-bzr.sh | 143 +++
contrib/remote-helpers/t5820-test-hg-bidi.sh | 243 +++
contrib/remote-helpers/t5821-test-hg-hg-git.sh | 534 +
contrib/remote-helpers/t5830-test-hg.sh| 121 ++
contrib/remote-helpers/test-bzr.sh | 143 ---
contrib/remote-helpers/test-hg-bidi.sh | 243 ---
contrib/remote-helpers/test-hg-hg-git.sh | 534 -
contrib/remote-helpers/test-hg.sh | 121 --
t/Makefile | 2 +-
10 files changed, 1044 insertions(+), 1043 deletions(-)
create mode 100755 contrib/remote-helpers/t5810-test-bzr.sh
create mode 100755 contrib/remote-helpers/t5820-test-hg-bidi.sh
create mode 100755 contrib/remote-helpers/t5821-test-hg-hg-git.sh
create mode 100755 contrib/remote-helpers/t5830-test-hg.sh
delete mode 100755 contrib/remote-helpers/test-bzr.sh
delete mode 100755 contrib/remote-helpers/test-hg-bidi.sh
delete mode 100755 contrib/remote-helpers/test-hg-hg-git.sh
delete mode 100755 contrib/remote-helpers/test-hg.sh
diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile
index 9a76575..012ad50 100644
--- a/contrib/remote-helpers/Makefile
+++ b/contrib/remote-helpers/Makefile
@@ -1,6 +1,7 @@
-TESTS := $(wildcard test*.sh)
+TESTS := $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
export T := $(addprefix $(CURDIR)/,$(TESTS))
+export TDUP := $(sort $(wildcard ../../t/t[0-9][0-9][0-9][0-9]-*.sh))
export MAKE := $(MAKE) -e
export PATH := $(CURDIR):$(PATH)
diff --git a/contrib/remote-helpers/t5810-test-bzr.sh
b/contrib/remote-helpers/t5810-test-bzr.sh
new file mode 100755
index 000..70aa8a0
--- /dev/null
+++ b/contrib/remote-helpers/t5810-test-bzr.sh
@@ -0,0 +1,143 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Felipe Contreras
+#
+
+test_description='Test remote-bzr'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON; then
+ skip_all='skipping remote-bzr tests; python not available'
+ test_done
+fi
+
+if ! "$PYTHON_PATH" -c 'import bzrlib'; then
+ skip_all='skipping remote-bzr tests; bzr not available'
+ test_done
+fi
+
+cmd='
+import bzrlib
+bzrlib.initialize()
+import bzrlib.plugin
+bzrlib.plugin.load_plugins()
+import bzrlib.plugins.fastimport
+'
+
+if ! "$PYTHON_PATH" -c "$cmd"; then
+ echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
+ skip_all='skipping remote-bzr tests; bzr-fastimport not available'
+ test_done
+fi
+
+check () {
+ (cd $1 &&
+ git log --format='%s' -1 &&
+ git symbolic-ref HEAD) > actual &&
+ (echo $2 &&
+ echo "refs/heads/$3") > expected &&
+ test_cmp expected actual
+}
+
+bzr whoami "A U Thor "
+
+test_expect_success 'cloning' '
+ (bzr init bzrrepo &&
+ cd bzrrepo &&
+ echo one > content &&
+ bzr add content &&
+ bzr commit -m one
+ ) &&
+
+ git clone "bzr::$PWD/bzrrepo" gitrepo &&
+ check gitrepo one master
+'
+
+test_expect_success 'pulling' '
+ (cd bzrrepo &&
+ echo two > content &&
+ bzr commit -m two
+ ) &&
+
+ (cd gitrepo && git pull) &&
+
+ check gitrepo two master
+'
+
+test_expect_success 'pushing' '
+ (cd gitrepo &&
+ echo three > content &&
+ git commit -a -m three &&
+ git push
+ ) &&
+
+ echo three > expected &&
+ cat bzrrepo/content > actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'roundtrip' '
+ (cd gitrepo &&
+ git pull &&
+ git log --format="%s" -1 origin/master > actual) &&
+ echo three > expected &&
+ test_cmp expected actual &&
+
+ (cd gitrepo && git push && git pull) &&
+
+ (cd bzrrepo &&
+ echo four > content &&
+ bzr commit -m four
+ ) &&
+
+ (cd gitrepo && git pull && git push) &&
+
+ check gitrepo four master &&
+
+ (cd gitrepo &&
+ echo five > content &&
+ git commit -a -m five &&
+ git push && git pull
+ ) &&
+
+ (cd bzrrepo && bzr revert) &&
+
+ echo five > expected &&
+ cat bzrrepo/content > actual &&
+ test_cmp expected actual
+'
+
+cat > expected < executable
+ chmod +x executable &&
+ bzr add executable
+ bzr commit -m exec &&
+ ln -s conte
[RFC] test-lint-duplicates: check numbering in contrib/remote-helpers
Running make inside contrib/remote-helpers failes in "test-lint-duplicates"
This was because the regexp to check for duplicate numbers strips everything
after the first "-" in the filename, including the prefix.
As a result, 2 pathnames like
"/contrib/remote-helpers/test-bzr.sh" and
"/contrib/remote-helpers/test-hg-bidi.sh"
are both converted into
"/contrib/remote" and reported as duplicate.
Improve the regexp:
Remove everything after "t-" (where X stand for a digit)
Rename the tests in contrib/remote-helpers into
t5810-test-bzr.sh,
t5820-test-hg-bidi.sh,
t5821-test-hg-hg-git.sh,
t5830-test-hg.sh
Feed the numbers used in contrib/remote-helpers into t/Makefile
and check that the numbering is unique across both directories
Signed-off-by: Torsten Bögershausen
---
contrib/remote-helpers/Makefile| 3 +-
contrib/remote-helpers/t5810-test-bzr.sh | 143 +++
contrib/remote-helpers/t5820-test-hg-bidi.sh | 243 +++
contrib/remote-helpers/t5821-test-hg-hg-git.sh | 534 +
contrib/remote-helpers/t5830-test-hg.sh| 121 ++
contrib/remote-helpers/test-bzr.sh | 143 ---
contrib/remote-helpers/test-hg-bidi.sh | 243 ---
contrib/remote-helpers/test-hg-hg-git.sh | 534 -
contrib/remote-helpers/test-hg.sh | 121 --
t/Makefile | 2 +-
10 files changed, 1044 insertions(+), 1043 deletions(-)
create mode 100755 contrib/remote-helpers/t5810-test-bzr.sh
create mode 100755 contrib/remote-helpers/t5820-test-hg-bidi.sh
create mode 100755 contrib/remote-helpers/t5821-test-hg-hg-git.sh
create mode 100755 contrib/remote-helpers/t5830-test-hg.sh
delete mode 100755 contrib/remote-helpers/test-bzr.sh
delete mode 100755 contrib/remote-helpers/test-hg-bidi.sh
delete mode 100755 contrib/remote-helpers/test-hg-hg-git.sh
delete mode 100755 contrib/remote-helpers/test-hg.sh
diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile
index 9a76575..012ad50 100644
--- a/contrib/remote-helpers/Makefile
+++ b/contrib/remote-helpers/Makefile
@@ -1,6 +1,7 @@
-TESTS := $(wildcard test*.sh)
+TESTS := $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
export T := $(addprefix $(CURDIR)/,$(TESTS))
+export TDUP := $(sort $(wildcard ../../t/t[0-9][0-9][0-9][0-9]-*.sh))
export MAKE := $(MAKE) -e
export PATH := $(CURDIR):$(PATH)
diff --git a/contrib/remote-helpers/t5810-test-bzr.sh
b/contrib/remote-helpers/t5810-test-bzr.sh
new file mode 100755
index 000..70aa8a0
--- /dev/null
+++ b/contrib/remote-helpers/t5810-test-bzr.sh
@@ -0,0 +1,143 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Felipe Contreras
+#
+
+test_description='Test remote-bzr'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON; then
+ skip_all='skipping remote-bzr tests; python not available'
+ test_done
+fi
+
+if ! "$PYTHON_PATH" -c 'import bzrlib'; then
+ skip_all='skipping remote-bzr tests; bzr not available'
+ test_done
+fi
+
+cmd='
+import bzrlib
+bzrlib.initialize()
+import bzrlib.plugin
+bzrlib.plugin.load_plugins()
+import bzrlib.plugins.fastimport
+'
+
+if ! "$PYTHON_PATH" -c "$cmd"; then
+ echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
+ skip_all='skipping remote-bzr tests; bzr-fastimport not available'
+ test_done
+fi
+
+check () {
+ (cd $1 &&
+ git log --format='%s' -1 &&
+ git symbolic-ref HEAD) > actual &&
+ (echo $2 &&
+ echo "refs/heads/$3") > expected &&
+ test_cmp expected actual
+}
+
+bzr whoami "A U Thor "
+
+test_expect_success 'cloning' '
+ (bzr init bzrrepo &&
+ cd bzrrepo &&
+ echo one > content &&
+ bzr add content &&
+ bzr commit -m one
+ ) &&
+
+ git clone "bzr::$PWD/bzrrepo" gitrepo &&
+ check gitrepo one master
+'
+
+test_expect_success 'pulling' '
+ (cd bzrrepo &&
+ echo two > content &&
+ bzr commit -m two
+ ) &&
+
+ (cd gitrepo && git pull) &&
+
+ check gitrepo two master
+'
+
+test_expect_success 'pushing' '
+ (cd gitrepo &&
+ echo three > content &&
+ git commit -a -m three &&
+ git push
+ ) &&
+
+ echo three > expected &&
+ cat bzrrepo/content > actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'roundtrip' '
+ (cd gitrepo &&
+ git pull &&
+ git log --format="%s" -1 origin/master > actual) &&
+ echo three > expected &&
+ test_cmp expected actual &&
+
+ (cd gitrepo && git push && git pull) &&
+
+ (cd bzrrepo &&
+ echo four > content &&
+ bzr commit -m four
+ ) &&
+
+ (cd gitrepo && git pull && git push) &&
+
+ check gitrepo four master &&
+
+ (cd gitrepo &&
+ echo five > content &&
+ git commit -a -m five &&
+ git push && git pull
+ ) &&
+
+ (cd bzrrepo && bzr revert) &&
+
+ echo five > expected &&
+ cat bzrrepo/content > actual &&
+ test_cmp expected actual
+'
+
+cat > expected < executable
+ chmod +x executable &&
+ bzr add executable
+ bzr commit -m exec &&
+ ln -s conte
Re: [PATCH] send-email: Honor multi-part email messages
On Fri, Jan 25, 2013 at 06:47:00PM +0100, Krzysztof Mazur wrote:
> On Fri, Jan 25, 2013 at 07:28:54PM +0400, Alexey Shumkin wrote:
> > "git format-patch --attach/--inline" generates multi-part messages.
> > Every part of such messages can contain non-ASCII characters with its own
> > "Content-Type" and "Content-Transfer-Encoding" headers.
> > But git-send-mail script interprets a patch-file as one-part message
> > and does not recognize multi-part messages.
> > So already quoted printable email subject may be encoded as quoted printable
> > again. Due to this bug email subject looks corrupted in email clients.
>
> I don't think that the problem with the Subject is multi-part message
> specific. The real problem with the Subject is probably that
> is_rfc2047_quoted() does not detect that the Subject is already quoted.
I have not even looked at this problem at all, but seeing this function
name:
> > sub body_or_subject_has_nonascii {
Makes me think something is very wrong. The subject line should not have
anything to do whatsoever with a content-type or
content-transfer-encoding header. It should either be rfc2047 encoded or
not, and the encoding used does not have to correspond to what is used
elsewhere in the message. rfc2047 is very clear that other MIME headers
are not necessary to interpret encoded words in headers.
So this loop:
foreach my $f (@files) {
next unless (body_or_subject_has_nonascii($f)
&& !file_declares_8bit_cte($f));
$broken_encoding{$f} = 1;
}
does not seem right at all. Only the body depends on the 8bit CTE.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails
On Fri, Jan 25, 2013 at 10:46:48AM -0800, Junio C Hamano wrote:
> Will queue this one, to be merged to 'maint' and 'master'.
>
> -- >8 --
> From: Jonathan Nieder
> Date: Thu, 24 Jan 2013 15:21:46 -0800
> Subject: [PATCH] ident: do not drop username when reading from /etc/mailname
Thanks, looks fine to me (and thanks to Jonathan). One nit:
> - if (strbuf_getline(buf, mailname, '\n') == EOF) {
> + if (strbuf_getline(&mailnamebuf, mailname, '\n') == EOF) {
> if (ferror(mailname))
> warning("cannot read /etc/mailname: %s",
> strerror(errno));
> + strbuf_release(&mailnamebuf);
> fclose(mailname);
> return -1;
> }
This strbuf_release is unnecessary, as an EOF return by definition means
we did not read anything. I don't mind it as a defensive measure,
though, in case the strbuf implementation changes to pre-allocate.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
On Fri, Jan 25, 2013 at 10:34:56AM -0800, Junio C Hamano wrote: > >> Not so quick, though. The lower level "read from help -a" is only > >> run once and its output kept in a two-level cache hierarchy; we need > >> to reset both. > > > > Ugh, I didn't even think about that. > > > > I wonder if it would be simpler if the completion tests actually ran a > > new bash for each test. That would be slower, but it somehow seems > > cleaner. > > I agree 100% with that. Let's leave this fix as-is, at least as a > tentative fix while "git check-ignore" graduates into the upcoming > release, and let somebody who is interested work on an update to > this test script to do so as an independent topic. Sounds good to me. Thanks. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows
On Fri, Jan 25, 2013 at 06:44:13PM +0400, Alexey Shumkin wrote:
> test_web_browse () {
> - # browser=$1 url=$2
> + # browser=$1 url=$2 sleep_timeout=$3
> + sleep_timeout="$3"
> git web--browse --browser="$1" "$2" >actual &&
> + # if $3 is set
> + # as far as Firefox is run in background (it is run with &)
> + # we trying to avoid race condition
> + # by waiting for "$sleep_timeout" seconds of timeout for
> 'fake_browser_ran' file appearance
> + (test -z "$sleep_timeout" || (
> + for timeout in $(seq 1 $sleep_timeout); do
> + test -f fake_browser_ran && break
> + sleep 1
> + done
> + test $timeout -ne $sleep_timeout
> + )
> + ) &&
> tr -d '\015' text &&
Gross, but I don't really see another way to handle the asynchronous
nature of spawning background browsers.
Two things, though:
1. Should test_web_browse just delete fake_browser_ran for us? Then
later tests do not have to remember to do so.
2. Seeing fake_browser_ran appeared, we know that the script has
started. But there is still a race condition in which it may not
have written anything to "actual" yet.
In this implementation:
> + cat >"fake browser" <<-\EOF &&
> + #!/bin/sh
> +
> + : > fake_browser_ran
> + if test "$1" = "-version"; then
> + echo Fake Firefox browser version 1.2.3
> + else
> + # Firefox (in contrast to w3m) is run in background (with &)
> + # so redirect output to "actual"
> + echo fake: "$@" > actual
> + fi
> + EOF
There is a period where fake_browser_ran exists, but nothing is in
actual. You can solve it by setting fake_browser_ran at the end rather
than the beginning.
Or you can drop fake_browser_ran entirely, and just atomically move
actual into place, like:
echo "fake: $*" >actual.tmp
mv actual.tmp actual
and then test_web_browse can just spin waiting for "actual" to appear.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/7] mergetool--lib: don't call "exit" in setup_tool
This will make it easier to use setup_tool in places where we expect
that the selected tool will not support the current mode.
Signed-off-by: John Keeping
---
git-mergetool--lib.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4c1e129..c6bd8ba 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -67,11 +67,11 @@ setup_tool () {
if merge_mode && ! can_merge
then
echo "error: '$tool' can not be used to resolve merges" >&2
- exit 1
+ return 1
elif diff_mode && ! can_diff
then
echo "error: '$tool' can only be used to resolve merges" >&2
- exit 1
+ return 1
fi
return 0
}
@@ -100,7 +100,7 @@ run_merge_tool () {
status=0
# Bring tool-specific functions into scope
- setup_tool "$1"
+ setup_tool "$1" || return
if merge_mode
then
--
1.8.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/7] mergetool--lib: fix path lookup in guess_merge_tool
guess_merge_tool calls translate_merge_tool_path in order to get the
correct name of the tool to check whether it can be found on the user's
system. But this function is designed to be overridden by tool
scriptlets so it does nothing if the relevant scriptlet has not been
sourced.
Fix this by calling setup_tool before doing anything.
Signed-off-by: John Keeping
---
git-mergetool--lib.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index c6bd8ba..46860c5 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -219,6 +219,7 @@ guess_merge_tool () {
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
+ setup_tool "$i" 2>&1 || continue
merge_tool_path="$(translate_merge_tool_path "$i")"
if type "$merge_tool_path" >/dev/null 2>&1
then
--
1.8.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
On Fri, Jan 25, 2013 at 01:47:59PM -0800, Junio C Hamano wrote: > John Keeping writes: > > With the patch above, the block of code at the top becomes: > > > > test "$tool" = defaults && continue > > > > setup_tool "$tool" 2>/dev/null || continue > > merge_tool_path=$(translate_merge_tool_path "$tool") > > > > which IMHO is pretty readable. > > Of course it is. The current callers of setup_tool may need some > adjustments, but that should be fairly trivial, I hope. There are only two and one of them already seems like it doesn't want the command to cause the script to exit. David, can you incorporate the following two patches when you re-roll? Your original 7/7 with the change above will want to build on 8/7. John -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
John Keeping writes: >> > It doesn't - the "|| continue" is to catch errors from setup_tool. >> >> Ugh. > > Is that targeted at my suggestion at the top of this email or calling > exit in setup_tool? At the fact that you had to go a convoluted route because you cannot just run setup_tool in subshell and do translate_merge_tool_path after that, because the latter needs to look at the shell variable the former sets. > With the patch above, the block of code at the top becomes: > > test "$tool" = defaults && continue > > setup_tool "$tool" 2>/dev/null || continue > merge_tool_path=$(translate_merge_tool_path "$tool") > > which IMHO is pretty readable. Of course it is. The current callers of setup_tool may need some adjustments, but that should be fairly trivial, I hope. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
On Fri, Jan 25, 2013 at 12:56:37PM -0800, Junio C Hamano wrote:
> John Keeping writes:
>
> > On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote:
> >> John Keeping writes:
> >>
> >> > Actually, can we just change all of the above part of the loop to:
> >> >
> >> > test "$tool" = defaults && continue
> >> >
> >> > merge_tool_path=$(
> >> > setup_tool "$tool" >/dev/null 2>&1 &&
> >> > translate_merge_tool_path "$tool"
> >> > ) || continue
> >>
> >> Meaning "setup_tool ought to know which mode we are in and should
> >> fail if we are in merge mode and it does not support merging"? That
> >> line of reasoning makes tons of sense to me, compared to this script
> >> implementing that logic for these scriptlets.
> >
> > Yes, that's part of what setup_tool does. It actually calls "exit" if
> > the "mode? && can_mode" test fails, which is why we need to call it in
> > the subshell.
> >
> > I think this would get even better if we add a preparatory patch like
> > this, so we can just call setup_tool and then set merge_tool_path:
> >
> > -- >8 --
> >
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> > index 888ae3e..4644cbf 100644
> > --- a/git-mergetool--lib.sh
> > +++ b/git-mergetool--lib.sh
> > @@ -67,11 +67,11 @@ setup_tool () {
> > if merge_mode && ! can_merge
> > then
> > echo "error: '$tool' can not be used to resolve merges" >&2
> > - exit 1
> > + return 1
> > elif diff_mode && ! can_diff
> > then
> > echo "error: '$tool' can only be used to resolve merges" >&2
> > - exit 1
> > + return 1
> > fi
> > return 0
> > }
> > @@ -100,7 +100,7 @@ run_merge_tool () {
> > status=0
> >
> > # Bring tool-specific functions into scope
> > - setup_tool "$1"
> > + setup_tool "$1" || return
> >
> > if merge_mode
> > then
> >
> > -- 8< --
> >
> >> How/when does translate_merge_tool_path fail?
> >
> > It doesn't - the "|| continue" is to catch errors from setup_tool.
>
> Ugh.
Is that targeted at my suggestion at the top of this email or calling
exit in setup_tool?
With the patch above, the block of code at the top becomes:
test "$tool" = defaults && continue
setup_tool "$tool" 2>/dev/null || continue
merge_tool_path=$(translate_merge_tool_path "$tool")
which IMHO is pretty readable.
John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] git-p4 support for older python
Both patches look simple enough. Pete, what do you think? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
John Keeping writes:
> On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote:
>> John Keeping writes:
>>
>> > Actually, can we just change all of the above part of the loop to:
>> >
>> >test "$tool" = defaults && continue
>> >
>> >merge_tool_path=$(
>> >setup_tool "$tool" >/dev/null 2>&1 &&
>> >translate_merge_tool_path "$tool"
>> >) || continue
>>
>> Meaning "setup_tool ought to know which mode we are in and should
>> fail if we are in merge mode and it does not support merging"? That
>> line of reasoning makes tons of sense to me, compared to this script
>> implementing that logic for these scriptlets.
>
> Yes, that's part of what setup_tool does. It actually calls "exit" if
> the "mode? && can_mode" test fails, which is why we need to call it in
> the subshell.
>
> I think this would get even better if we add a preparatory patch like
> this, so we can just call setup_tool and then set merge_tool_path:
>
> -- >8 --
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 888ae3e..4644cbf 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -67,11 +67,11 @@ setup_tool () {
> if merge_mode && ! can_merge
> then
> echo "error: '$tool' can not be used to resolve merges" >&2
> - exit 1
> + return 1
> elif diff_mode && ! can_diff
> then
> echo "error: '$tool' can only be used to resolve merges" >&2
> - exit 1
> + return 1
> fi
> return 0
> }
> @@ -100,7 +100,7 @@ run_merge_tool () {
> status=0
>
> # Bring tool-specific functions into scope
> - setup_tool "$1"
> + setup_tool "$1" || return
>
> if merge_mode
> then
>
> -- 8< --
>
>> How/when does translate_merge_tool_path fail?
>
> It doesn't - the "|| continue" is to catch errors from setup_tool.
Ugh.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote:
> John Keeping writes:
>
> > Actually, can we just change all of the above part of the loop to:
> >
> > test "$tool" = defaults && continue
> >
> > merge_tool_path=$(
> > setup_tool "$tool" >/dev/null 2>&1 &&
> > translate_merge_tool_path "$tool"
> > ) || continue
>
> Meaning "setup_tool ought to know which mode we are in and should
> fail if we are in merge mode and it does not support merging"? That
> line of reasoning makes tons of sense to me, compared to this script
> implementing that logic for these scriptlets.
Yes, that's part of what setup_tool does. It actually calls "exit" if
the "mode? && can_mode" test fails, which is why we need to call it in
the subshell.
I think this would get even better if we add a preparatory patch like
this, so we can just call setup_tool and then set merge_tool_path:
-- >8 --
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 888ae3e..4644cbf 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -67,11 +67,11 @@ setup_tool () {
if merge_mode && ! can_merge
then
echo "error: '$tool' can not be used to resolve merges" >&2
- exit 1
+ return 1
elif diff_mode && ! can_diff
then
echo "error: '$tool' can only be used to resolve merges" >&2
- exit 1
+ return 1
fi
return 0
}
@@ -100,7 +100,7 @@ run_merge_tool () {
status=0
# Bring tool-specific functions into scope
- setup_tool "$1"
+ setup_tool "$1" || return
if merge_mode
then
-- 8< --
> How/when does translate_merge_tool_path fail?
It doesn't - the "|| continue" is to catch errors from setup_tool.
John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] git-p4 support for older python
Offered for consideration. These two patches allow git-p4 to run on python 2.4 which is shipped on RHEL 5.X. The changes are minor and unintrusive in my opinion, but please feel free to reject one or both of them for any reason (in which case the version check at the top of git-p4.py should be updated). Brandon Casey (2): git-p4.py: support Python 2.5 git-p4.py: support Python 2.4 INSTALL| 2 +- git-p4.py | 30 ++ t/t9802-git-p4-filetype.sh | 11 ++- 3 files changed, 33 insertions(+), 10 deletions(-) -- 1.8.1.1.297.gad3d74e --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] git-p4.py: support Python 2.4
Python 2.4 lacks the following features:
subprocess.check_call
struct.pack_into
Take a cue from 460d1026 and provide an implementation of the
CalledProcessError exception. Then replace the calls to
subproccess.check_call with calls to subprocess.call that check the return
status and raise a CalledProcessError exception if necessary.
The struct.pack_into in t/9802 can be converted into a single struct.pack
call which is available in Python 2.4.
Signed-off-by: Brandon Casey
---
INSTALL| 2 +-
git-p4.py | 27 ---
t/t9802-git-p4-filetype.sh | 11 ++-
3 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/INSTALL b/INSTALL
index fc723b3..b96e16d 100644
--- a/INSTALL
+++ b/INSTALL
@@ -131,7 +131,7 @@ Issues of note:
use English. Under autoconf the configure script will do this
automatically if it can't find libintl on the system.
- - Python version 2.5 or later is needed to use the git-p4
+ - Python version 2.4 or later is needed to use the git-p4
interface to Perforce.
- Some platform specific issues are dealt with Makefile rules,
diff --git a/git-p4.py b/git-p4.py
index 4f95d7a..faec09d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -18,6 +18,21 @@ import optparse, os, marshal, subprocess, shelve
import tempfile, getopt, os.path, time, platform
import re, shutil
+try:
+from subprocess import CalledProcessError
+except ImportError:
+# from python2.7:subprocess.py
+# Exception classes used by this module.
+class CalledProcessError(Exception):
+"""This exception is raised when a process run by check_call() returns
+a non-zero exit status. The exit status will be stored in the
+returncode attribute."""
+def __init__(self, returncode, cmd):
+self.returncode = returncode
+self.cmd = cmd
+def __str__(self):
+return "Command '%s' returned non-zero exit status %d" %
(self.cmd, self.returncode)
+
verbose = False
# Only labels/tags matching this will be imported/exported
@@ -158,13 +173,17 @@ def system(cmd):
expand = isinstance(cmd,basestring)
if verbose:
sys.stderr.write("executing %s\n" % str(cmd))
-subprocess.check_call(cmd, shell=expand)
+retcode = subprocess.call(cmd, shell=expand)
+if retcode:
+raise CalledProcessError(retcode, cmd)
def p4_system(cmd):
"""Specifically invoke p4 as the system command. """
real_cmd = p4_build_cmd(cmd)
expand = isinstance(real_cmd, basestring)
-subprocess.check_call(real_cmd, shell=expand)
+retcode = subprocess.call(real_cmd, shell=expand)
+if retcode:
+raise CalledProcessError(retcode, real_cmd)
def p4_integrate(src, dest):
p4_system(["integrate", "-Dt", wildcard_encode(src),
wildcard_encode(dest)])
@@ -3174,7 +3193,9 @@ class P4Clone(P4Sync):
init_cmd = [ "git", "init" ]
if self.cloneBare:
init_cmd.append("--bare")
-subprocess.check_call(init_cmd)
+retcode = subprocess.call(init_cmd)
+if retcode:
+raise CalledProcessError(retcode, init_cmd)
if not P4Sync.run(self, depotPaths):
return False
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 21924df..be299dc 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -105,12 +105,13 @@ build_gendouble() {
cat >gendouble.py <<-\EOF
import sys
import struct
- import array
- s = array.array("c", '\0' * 26)
- struct.pack_into(">L", s, 0, 0x00051607) # AppleDouble
- struct.pack_into(">L", s, 4, 0x0002) # version 2
- s.tofile(sys.stdout)
+ s = struct.pack(">LL18s",
+ 0x00051607, # AppleDouble
+ 0x0002, # version 2
+ "" # pad to 26 bytes
+ )
+ sys.stdout.write(s);
EOF
}
--
1.8.1.1.297.gad3d74e
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git-p4.py: support Python 2.5
Python 2.5 and older do not accept None as the first argument to
translate() and complain with:
TypeError: expected a character buffer object
Satisfy this older python by calling maketrans() to generate an empty
translation table and supplying that to translate().
This allows git-p4 to be used with Python 2.5.
Signed-off-by: Brandon Casey
---
INSTALL | 2 +-
git-p4.py | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/INSTALL b/INSTALL
index 28f34bd..fc723b3 100644
--- a/INSTALL
+++ b/INSTALL
@@ -131,7 +131,7 @@ Issues of note:
use English. Under autoconf the configure script will do this
automatically if it can't find libintl on the system.
- - Python version 2.6 or later is needed to use the git-p4
+ - Python version 2.5 or later is needed to use the git-p4
interface to Perforce.
- Some platform specific issues are dealt with Makefile rules,
diff --git a/git-p4.py b/git-p4.py
index 2da5649..4f95d7a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -768,7 +768,8 @@ def wildcard_encode(path):
return path
def wildcard_present(path):
-return path.translate(None, "*#@%") != path
+from string import maketrans
+return path.translate(maketrans("",""), "*#@%") != path
class Command:
def __init__(self):
--
1.8.1.1.297.gad3d74e
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3
On Wed, Jan 23, 2013 at 12:36 PM, Junio C Hamano wrote: > Sverre Rabbelier writes: > >> On Wed, Jan 23, 2013 at 11:47 AM, John Keeping wrote: When did we last revisit what minimal python version we are ok with requiring? >>> >>> I was wondering if people would weigh in discussing that in response to >>> [1] but no one has commented on that part of it. As another datapoint, >>> Brandon Casey was suggesting patching git-p4.py to support Python 2.4 >>> [2]. >>> >>> [1] http://article.gmane.org/gmane.comp.version-control.git/213920 >>> [2] http://article.gmane.org/gmane.comp.version-control.git/214048 >> >> I for one would be happy to kill off support for anything older than >> 2.6 (which had it's latest release on October 1st, 2008). >> >> Junio, how have we decided in the past which version of x to support? > > I do not think there was any conclusion. $gmane/212215 claiming 2.4 > support matters for RHEL 5.x users was the last on the topic as far > as I can tell, so it boils down to another question: do users on > RHEL 5.x matter? > > I can read from $gmane/212215 that users of the said platform can > safely keep using Python 2.4 under their vendor support contract > until 2017. But let's focus on what do these users expect of their > system and software they run on it a bit. > > When they want to run a piece software that is not shipped with > RHEL, either by writing their own or by importing from elsewhere, > that needs 2.6 features, what are their options? > > (a) The platform vendor optionally supplies 2.6 with or without > support; > > (b) The users can and do install 2.6 as /usr/local/bin/python2.6, > which may even be community-supported, but the vendor does not > support it; or > > (c) The vendor terminates the support contract for users who choose > to go (b). > > I think we can safely discard (c); if that is the case, the users on > the said platform will not choose to update Git either, so it does > not matter where the future versions of Git sets the lower bound of > Python version at. > > If we are not talking about the situation (c), then the users can > choose to use 2.6, and more importantly, Python being a popular > software, I would imagine that there are reputable sources of > prepackaged RPMs for them to do so without going too much hassle of > configuring, compiling and installing. > > Now how does the decision we make today for releases of Git that > haven't yet happened will affect these users? As these versions of > newer Git were not shipped with RHEL 5.x, and also I am assuming > that Git is a more niche product than Python is, I would imagine > that it is very unlikely that the vendor gives it the users as an > optional package. The users will have to do the same thing to be > able to use such versions of Git as whatever they do in order to use > Python 2.6. > > Given that, what the vendor originally shipped and officially > supports does not affect the choices we would make today for newer > versions of Git. The users in a shop where additional third-party > software in /usr/local/bin is strictly forbidden, they are stuck > with the version of Git that the vendor shipped anyway, because they > won't be able to install an updated Git in /usr/local/bin, either. > > That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if > you are really paranoid, /usr/local/only-for-git/bin/python2.6 where > nobody's $PATH points at) is impossible. > > So personally I do not think dropping 2.4 is a huge problem for > future versions of Git, but I'd like to hear from those working in > IT support for large and slow-moving organizations (aka RHEL 5 > customers). I'm not really in the demographic that you asked to hear from, but I'll give my 2 cents anyway. :) Firstly, I defer to those with more knowledge and experience with python to decide which version should be the minimum version supported. Python 2.6 seems to be the consensus and that's fine with me. With respect to older platforms like RHEL 5.X that don't ship with Python 2.6 or later, I suspect most people who work in an organization with a dedicated IT staff can request that a more recent version of python be installed. So, I don't think a python 2.6 requirement (if there was one) would be a blocker for them, and I don't think it would be a major pain for the sysadmin to install. My only opinion is that if we can avoid breaking older platforms fairly easily, we should do so. If there is someone out there building git packages (e.g. EPEL) for RHEL 5.X or anything else, I imagine that one less dependency makes installing and supporting the package that much easier. So, my comments shouldn't be taken to suggest that git should support any particular version of python. That decision should be made by those who are willing to support whatever version they feel strongly about. -Brandon -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
John Keeping writes: > Actually, can we just change all of the above part of the loop to: > > test "$tool" = defaults && continue > > merge_tool_path=$( > setup_tool "$tool" >/dev/null 2>&1 && > translate_merge_tool_path "$tool" > ) || continue Meaning "setup_tool ought to know which mode we are in and should fail if we are in merge mode and it does not support merging"? That line of reasoning makes tons of sense to me, compared to this script implementing that logic for these scriptlets. How/when does translate_merge_tool_path fail? > >> >if type "$merge_tool_path" >/dev/null 2>&1 >> >then >> > - available="$available$i$LF" >> > + available="$available$tool$LF" >> >else >> > - unavailable="$unavailable$i$LF" >> > + unavailable="$unavailable$tool$LF" >> >fi >> >done >> -- >> To unsubscribe from this list: send the line "unsubscribe git" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
On Fri, Jan 25, 2013 at 07:54:46PM +, John Keeping wrote:
> On Fri, Jan 25, 2013 at 01:43:54AM -0800, David Aguilar wrote:
> > Check the can_diff and can_merge functions before deciding whether to
> > add the tool to the available/unavailable lists. This makes --tool-help
> > context-
> > sensitive so that "git mergetool --tool-help" displays merge tools only
> > and "git difftool --tool-help" displays diff tools only.
>
> This log message is misleading - the existing code in
> list_merge_tool_candidates already filters the tools like this, so the
> change is more:
>
> mergetool--lib: don't use a hardcoded list for "--tool-help"
>
> Instead of using a list of tools in list_merge_tool_candidates, list
> the available scriptlets and query each of those to know whether it
> applies to diff mode and/or merge mode.
>
> guess_merge_tool still relies on list_merge_tool_candidates so we
> can't remove that function now.
>
>
> The patch seems to do the right thing, although I have a couple of minor
> nits...
>
> > Signed-off-by: David Aguilar
> > ---
> > git-mergetool--lib.sh | 26 +-
> > 1 file changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> > index db8218a..c547c59 100644
> > --- a/git-mergetool--lib.sh
> > +++ b/git-mergetool--lib.sh
> > @@ -168,17 +168,33 @@ list_merge_tool_candidates () {
> > }
> >
> > show_tool_help () {
> > - list_merge_tool_candidates
> > unavailable= available= LF='
> > '
> > - for i in $tools
> > +
> > + scriptlets="$(git --exec-path)"/mergetools
> > + for i in "$scriptlets"/*
> > do
> > - merge_tool_path=$(translate_merge_tool_path "$i")
> > + . "$scriptlets"/defaults
> > + . "$i"
> > +
> > + tool="$(basename "$i")"
>
> Quotes are unnecessary here.
>
> > + if test "$tool" = "defaults"
> > + then
> > + continue
> > + elif merge_mode && ! can_merge
> > + then
> > + continue
> > + elif diff_mode && ! can_diff
> > + then
> > + continue
> > + fi
>
> Would this be better as:
>
> test "$tool" = "defaults" && continue
>
> can_merge || ! merge_mode || continue
> can_diff || ! diff_mode || continue
>
> or is that a bit too concise?
>
> I'd prefer to see two separate if statements either way since the "test
> $tool = defaults" case is different from the "does it apply to the
> current mode?" case. The "$tool = defaults" case could even move to the
> top of the loop.
>
> > + merge_tool_path=$(translate_merge_tool_path "$tool")
Actually, can we just change all of the above part of the loop to:
test "$tool" = defaults && continue
merge_tool_path=$(
setup_tool "$tool" >/dev/null 2>&1 &&
translate_merge_tool_path "$tool"
) || continue
> > if type "$merge_tool_path" >/dev/null 2>&1
> > then
> > - available="$available$i$LF"
> > + available="$available$tool$LF"
> > else
> > - unavailable="$unavailable$i$LF"
> > + unavailable="$unavailable$tool$LF"
> > fi
> > done
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
John Keeping writes: >> +tool="$(basename "$i")" > > Quotes are unnecessary here. Yeah, the outer quotes aren't needed; the inner ones are. >> +if test "$tool" = "defaults" >> +then >> +continue >> +elif merge_mode && ! can_merge >> +then >> +continue >> +elif diff_mode && ! can_diff >> +then >> +continue >> +fi > > Would this be better as: > > test "$tool" = "defaults" && continue > > can_merge || ! merge_mode || continue > can_diff || ! diff_mode || continue > > or is that a bit too concise? It is beyond "too concise"; it is unreadable, and more importantly, the latter two lines are illogical (why do you even ask if it can be used for merging, before asking merge_mode to see if the answer to that question matters to you?) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
On Fri, Jan 25, 2013 at 01:43:54AM -0800, David Aguilar wrote:
> Check the can_diff and can_merge functions before deciding whether to
> add the tool to the available/unavailable lists. This makes --tool-help
> context-
> sensitive so that "git mergetool --tool-help" displays merge tools only
> and "git difftool --tool-help" displays diff tools only.
This log message is misleading - the existing code in
list_merge_tool_candidates already filters the tools like this, so the
change is more:
mergetool--lib: don't use a hardcoded list for "--tool-help"
Instead of using a list of tools in list_merge_tool_candidates, list
the available scriptlets and query each of those to know whether it
applies to diff mode and/or merge mode.
guess_merge_tool still relies on list_merge_tool_candidates so we
can't remove that function now.
The patch seems to do the right thing, although I have a couple of minor
nits...
> Signed-off-by: David Aguilar
> ---
> git-mergetool--lib.sh | 26 +-
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index db8218a..c547c59 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -168,17 +168,33 @@ list_merge_tool_candidates () {
> }
>
> show_tool_help () {
> - list_merge_tool_candidates
> unavailable= available= LF='
> '
> - for i in $tools
> +
> + scriptlets="$(git --exec-path)"/mergetools
> + for i in "$scriptlets"/*
> do
> - merge_tool_path=$(translate_merge_tool_path "$i")
> + . "$scriptlets"/defaults
> + . "$i"
> +
> + tool="$(basename "$i")"
Quotes are unnecessary here.
> + if test "$tool" = "defaults"
> + then
> + continue
> + elif merge_mode && ! can_merge
> + then
> + continue
> + elif diff_mode && ! can_diff
> + then
> + continue
> + fi
Would this be better as:
test "$tool" = "defaults" && continue
can_merge || ! merge_mode || continue
can_diff || ! diff_mode || continue
or is that a bit too concise?
I'd prefer to see two separate if statements either way since the "test
$tool = defaults" case is different from the "does it apply to the
current mode?" case. The "$tool = defaults" case could even move to the
top of the loop.
> + merge_tool_path=$(translate_merge_tool_path "$tool")
> if type "$merge_tool_path" >/dev/null 2>&1
> then
> - available="$available$i$LF"
> + available="$available$tool$LF"
> else
> - unavailable="$unavailable$i$LF"
> + unavailable="$unavailable$tool$LF"
> fi
> done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows
Alexey Shumkin writes:
> Firefox on Windows by default is placed in "C:\Program Files\Mozilla Firefox"
> folder, i.e. its path contains spaces. Before running this browser
> "git-web--browse"
> tests version of Firefox to decide whether to use "-new-tab" option or not.
>
> Quote browser path to avoid error during this test.
>
> Signed-off-by: Alexey Shumkin
> Reviewed-by: Jeff King
Thanks, both.
> ---
> git-web--browse.sh | 2 +-
> t/t9901-git-web--browse.sh | 57
> +-
> 2 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/git-web--browse.sh b/git-web--browse.sh
> index 1e82726..f96e5bd 100755
> --- a/git-web--browse.sh
> +++ b/git-web--browse.sh
> @@ -149,7 +149,7 @@ fi
> case "$browser" in
> firefox|iceweasel|seamonkey|iceape)
> # Check version because firefox < 2.0 does not support "-new-tab".
> - vers=$(expr "$($browser_path -version)" : '.* \([0-9][0-9]*\)\..*')
> + vers=$(expr "$("$browser_path" -version)" : '.* \([0-9][0-9]*\)\..*')
> NEWTAB='-new-tab'
> test "$vers" -lt 2 && NEWTAB=''
> "$browser_path" $NEWTAB "$@" &
> diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
> index b0a6bad..30d5294 100755
> --- a/t/t9901-git-web--browse.sh
> +++ b/t/t9901-git-web--browse.sh
> @@ -8,8 +8,21 @@ This test checks that git web--browse can handle various
> valid URLs.'
> . ./test-lib.sh
>
> test_web_browse () {
> - # browser=$1 url=$2
> + # browser=$1 url=$2 sleep_timeout=$3
> + sleep_timeout="$3"
> git web--browse --browser="$1" "$2" >actual &&
> + # if $3 is set
> + # as far as Firefox is run in background (it is run with &)
> + # we trying to avoid race condition
> + # by waiting for "$sleep_timeout" seconds of timeout for
> 'fake_browser_ran' file appearance
> + (test -z "$sleep_timeout" || (
> + for timeout in $(seq 1 $sleep_timeout); do
> + test -f fake_browser_ran && break
> + sleep 1
> + done
> + test $timeout -ne $sleep_timeout
> + )
> + ) &&
Style:
- do/then/else begin a new line (a good rule of thumb is remember
this rule is to write control structures without using
semicolon).
- do not use "seq"; it is not available in some places.
I do not think of a reason why you want ( nested (subshell) ), but
if you don't need them, perhaps I'd write the above this way:
if test -n $sleep_timeout
then
for timeout in $(test_seq $sleep_timeout)
do
test -f fake_browser_ran && break
sleep 1
done
test $timeout -ne $sleep_timeout
fi &&
> @@ -48,6 +61,48 @@ test_expect_success \
> '
>
> test_expect_success \
> + 'Firefox below v2.0 paths are properly quoted' '
-ECANNOTPARSE.
"Paths to firefox older than v2.0 are properly quoted" you mean,
perhaps? I dunno.
> + echo fake: http://example.com/foo >expect &&
> + rm -f fake_browser_ran &&
> + cat >"fake browser" <<-\EOF &&
> + #!/bin/sh
Consider using "write_script" helper so that you get the path to the
shell the user specified via $SHELL_PATH.
> +
> + : > fake_browser_ran
Style: no SP between redirection operator and filename, i.e.
: >fake_browser_ran
> + if test "$1" = "-version"; then
Style (see above).
> + echo Fake Firefox browser version 1.2.3
> + else
> + # Firefox (in contrast to w3m) is run in background (with &)
> + # so redirect output to "actual"
> + echo fake: "$@" > actual
Style (see above).
> + fi
> + EOF
> + chmod +x "fake browser" &&
> + git config browser.firefox.path "`pwd`/fake browser" &&
We tend to prefer $(pwd) over `pwd`.
> + test_web_browse firefox http://example.com/foo 5
> +'
> +
> +test_expect_success \
> + 'Firefox not lower v2.0 paths are properly quoted' '
s/not lower v2.0/v2.0 and above/, but again -ECANNOTPARSE.
> + echo fake: -new-tab http://example.com/foo >expect &&
I'd feel safer if you quoted the arguments to "echo", i.e.
echo "fake: -new-tab http://example.com/foo"; >expect &&
The same style comments as above apply to the remainder of patch.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] add: warn when -u or -A is used without filepattern
Matthieu Moy writes:
> Most git commands that can be used with our without a filepattern are
> tree-wide by default, the filepattern being used to restrict their scope.
> A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'.
>
> The inconsistancy of 'git add -u' and 'git add -A' are particularly
s/consistan/consisten/;
> diff --git a/builtin/add.c b/builtin/add.c
> index e664100..8252d19 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -363,6 +363,33 @@ static int add_files(struct dir_struct *dir, int flags)
> return exit_status;
> }
>
> +static void warn_pathless_add(const char *option_name) {
> + /*
> + * To be consistant with "git add -p" and most Git
Likewise.
> + warning(_("The behavior of 'git add %s' with no path argument from a
> subdirectory of the\n"
> ...
> + option_name,
> + option_name,
> + option_name);
> +}
> +
> int cmd_add(int argc, const char **argv, const char *prefix)
> {
> int exit_status = 0;
> @@ -392,8 +420,14 @@ int cmd_add(int argc, const char **argv, const char
> *prefix)
> die(_("-A and -u are mutually incompatible"));
> if (!show_only && ignore_missing)
> die(_("Option --ignore-missing can only be used together with
> --dry-run"));
> - if ((addremove || take_worktree_changes) && !argc) {
> + if (addremove)
> + option_with_implicit_dot = "--all";
> + if (take_worktree_changes)
> + option_with_implicit_dot = "--update";
I wonder if we want to say in the message
The behaviour of 'git add --all (or -A)'...
otherwise people who typed "git add -A" and got this message with
just "--all" may go "Huh?" for a brief moment. I however do not
think replacing these strings to
option_with_implicit_dot = "--all (-A)";
is a solution, given they are goven to _("l10n template %s").
Other than that the patch looks reasonable.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
David Aguilar writes: > On Fri, Jan 25, 2013 at 2:38 AM, John Keeping wrote: >> On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote: >>> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty >>> list "vim" as being an unavailable tool. This is because they attempt >>> to find a tool named according to the mergetool scriptlet instead of the >>> Git- >>> recognized tool name. >> >> Actually, after my patches both git-difftool and git-mergetool get this >> right since list_merge_tool_candidates lists vimdiff and gvimdiff. >> >>> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim" >>> scriptlet. This required git-mergetool--lib to special-case it when >>> setting up the tool. >>> >>> Remove the exception for "vim" and allow the scriptlets to be found >>> naturally by using symlinks to a single "vimdiff" scriptlet. >> >> I wonder if it would be better to make these single-line scripts instead >> of symlinks: >> >> . "$MERGE_TOOLS_DIR"/vimdiff >> >> where we make git-mergetool--lib.sh export: >> >> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools > > That sounds like the way to go. Yup, I'll expect a reroll of this one and possibly the next one (I haven't read yet). 1-5/7 looked all very sensible. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] branch: reject -D/-d without branch name
Nguyễn Thái Ngọc Duy writes:
> ---
> builtin/branch.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
Forgot to sign-off?
Is this a real problem?
I do not see it particularly wrong to succeed after deleting 0 or
more given branch names.
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 873f624..50fcacc 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char
> *prefix)
> colopts = 0;
> }
>
> - if (delete)
> + if (delete) {
> + if (!argc)
> + die(_("branch name required"));
> return delete_branches(argc, argv, delete > 1, kinds, quiet);
> - else if (list) {
> + } else if (list) {
> int ret = print_ref_list(kinds, detached, verbose, abbrev,
>with_commit, argv);
> print_columns(&output, colopts, NULL);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] read-cache: refuse to create index referring to external objects
Duy Nguyen writes:
> Even
> when cache-tree is not involved, I do not want the index to point to
> an non-existing SHA-1 ("git diff --cached" may fail next time, for
> example).
I think we have tests that explicitly add SHA-1 that names an object
that does not exist to the index and check failures; you may have to
think what to do with them.
>> This is a tangent, but in addition, you may want to add an even
>> narrower variant that checks the same but ignoring _all_ alternate
>> object databases, "external" or not:
>>
>> int has_sha1_file_local(const unsigned char *sha1);
>>
>> That may be useful if we want to later make the alternate safer to
>> use; instead of letting the codepath to create an object ask
>> has_sha1_file() to see if it already exists and refrain from writing
>> a new copy, we switch to ask has_sha1_file_locally() and even if an
>> alternate object database we borrow from has it, we keep our own
>> copy in our repository.
This is not a tangent, but if you want to go this "forbid making our
repository depend on objects we do not have but we know about after
we peek submodule odb" route [*1*], write_sha1_file() needs to be
told about has_sha1_file_proper(). We may "git add" a new blob in
the superproject, the blob may not yet exist in *our* repository,
but may happen to already exist in the submodue odb. In such a
case, write_sha1_file() has to write that blob in our repository,
without the existing has_sha1_file() check bypassing it. Otherwise
our attempt to create a tree that contains that blob will fail,
saying that the blob only seems to exist to us via submodule odb but
not in our repository.
[Footnote]
*1* which I do not necessarily agree with---I am in favor of getting
rid of add_submodule_odb() to fix these issues at the root cause of
them.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails
Jeff King writes:
> (with a proper commit message, of course).
Will queue this one, to be merged to 'maint' and 'master'.
-- >8 --
From: Jonathan Nieder
Date: Thu, 24 Jan 2013 15:21:46 -0800
Subject: [PATCH] ident: do not drop username when reading from /etc/mailname
An earlier conversion from fgets() to strbuf_getline() in the
codepath to read from /etc/mailname to learn the default host-part
of the ident e-mail address forgot that strbuf_getline() stores the
line at the beginning of the buffer just like fgets().
The "username@" the caller has prepared in the strbuf, expecting the
function to append the host-part to it, was lost because of this.
Reported-by: Mihai Rusu
Signed-off-by: Jonathan Nieder
Acked-by: Jeff King
Signed-off-by: Junio C Hamano
---
ident.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ident.c b/ident.c
index 484e0a9..fb4cc72 100644
--- a/ident.c
+++ b/ident.c
@@ -41,6 +41,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf
*name)
static int add_mailname_host(struct strbuf *buf)
{
FILE *mailname;
+ struct strbuf mailnamebuf = STRBUF_INIT;
mailname = fopen("/etc/mailname", "r");
if (!mailname) {
@@ -49,14 +50,17 @@ static int add_mailname_host(struct strbuf *buf)
strerror(errno));
return -1;
}
- if (strbuf_getline(buf, mailname, '\n') == EOF) {
+ if (strbuf_getline(&mailnamebuf, mailname, '\n') == EOF) {
if (ferror(mailname))
warning("cannot read /etc/mailname: %s",
strerror(errno));
+ strbuf_release(&mailnamebuf);
fclose(mailname);
return -1;
}
/* success! */
+ strbuf_addbuf(buf, &mailnamebuf);
+ strbuf_release(&mailnamebuf);
fclose(mailname);
return 0;
}
--
1.8.1.1.525.gdace530
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9902: protect test from stray build artifacts
Jeff King writes: > On Thu, Jan 24, 2013 at 08:19:30PM -0800, Junio C Hamano wrote: > >> > Ahh, ok, we show one element per line and just make sure "bundle" >> > is there, and we do not care what other buns appear in the output. >> > >> Not so quick, though. The lower level "read from help -a" is only >> run once and its output kept in a two-level cache hierarchy; we need >> to reset both. > > Ugh, I didn't even think about that. > > I wonder if it would be simpler if the completion tests actually ran a > new bash for each test. That would be slower, but it somehow seems > cleaner. I agree 100% with that. Let's leave this fix as-is, at least as a tentative fix while "git check-ignore" graduates into the upcoming release, and let somebody who is interested work on an update to this test script to do so as an independent topic. > >> It starts to look a bit too intimately tied to the implementation of >> what is being tested for my taste, though. >> [...] >> +test_expect_success 'help -a read correctly by command list generator' ' >> +__git_all_commands= && >> +__git_porcelain_commands= && >> +GIT_TESTING_COMMAND_COMPLETION= && >> +run_completion "git bun" && >> +grep "^bundle $" out >> +' > > Agreed. I could take or leave it at this point. It's nice to check that > changes to "help -a" will not break it, but ultimately it feels a bit > too contrived to catch anything useful. > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Enhance tortoisemerge to work with
Sven Strickroth writes:
> TortoiseGitMerge and filenames with spaces
??? ECANNOTPARSE.
... ah, wait. Is this a broken-off tail of your subject line?
It may be a sign that you are doing too many unrelated things in a
single patch when your subject does not fit on a single line.
Perhaps this is better done as a two-patch series?
* mergetools: fix tortoisemerge support for pathnames with SP
* mergetools: support tortoisegitmerge
> mergetools/tortoisemerge | 51
>
> 1 file changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
> index ed7db49..8ee99a5 100644
> --- a/mergetools/tortoisemerge
> +++ b/mergetools/tortoisemerge
> @@ -1,17 +1,34 @@
> -can_diff () {
> - return 1
> -}
> -
> -merge_cmd () {
> - if $base_present
> - then
> - touch "$BACKUP"
> - "$merge_tool_path" \
> - -base:"$BASE" -mine:"$LOCAL" \
> - -theirs:"$REMOTE" -merged:"$MERGED"
> - check_unchanged
> - else
> - echo "TortoiseMerge cannot be used without a base" 1>&2
> - return 1
> - fi
> -}
> +can_diff () {
> + return 1
> +}
> +
> +merge_cmd () {
> + if $base_present
> + then
> + touch "$BACKUP"
> + basename="$(basename "$merge_tool_path" .exe)"
> + if test "$basename" = "tortoisegitmerge"
> + then
> + "$merge_tool_path" \
> + -base "$BASE" -mine "$LOCAL" \
> + -theirs "$REMOTE" -merged "$MERGED"
> + else
> + "$merge_tool_path" \
> + -base:"$BASE" -mine:"$LOCAL" \
> + -theirs:"$REMOTE" -merged:"$MERGED"
Hmph.
How was the support for "names with spaces" added in this new code?
I do not spot what is different between this "else" clause and the
original body of the merge_cmd (which only supported tortoisemerge).
They seem to be doing exactly the same thing.
> + fi
> + check_unchanged
> + else
> + echo "$merge_tool_path cannot be used without a base" 1>&2
> + return 1
> + fi
> +}
> +
> +translate_merge_tool_path() {
> + if type tortoisegitmerge >/dev/null 2>/dev/null
> + then
> + echo tortoisegitmerge
> + else
> + echo tortoisemerge
> + fi
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resent] send-email: Honor multi-part email messages
Alexey Shumkin writes: > This function is used to determine "broken" (non-ASCII) headers (to be encode > them) > The problem is if "Subject" is not broken, but message body contains > non-ASCII chars, > subject is marked as broken and encoded again. I think that is not a "problem" but is a mere symptom. The remainder of the codeflow of send-email, AFAICS (it's not my code), is not prepared to deal with multipart messages at all. In order to handle multi-part properly, you may still have to fix broken Subject: of the whole thing, and you may also want to fix broken headers inside one part while keeping correctly formatted part intact. Your patch just stops an early error checking that is meant for a non multi-part message that happens to trigger on a multi-part message in your test case from triggering (i.e. masking a symptom) and let the remaining lines of the multi-part message to codepath that does not do anything special to handle multi-part messages correctly, letting it do whatever it happens to do to a message assuming it is not a multi-part message, no? In other words, making send-email capable of handling a multi-part might be a worthy thing to do, but I do not think your patch is a good first step for doing so. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git merge error question: The following untracked working tree files would be overwritten by merge
Carsten Fuchs writes: > Hi all, > > in my repo, I'm doing this: > >> $ git status >> # On branch master >> # Your branch is behind 'origin/master' by 2 commits, and can be >> fast-forwarded. >> # >> # Untracked files: >> # (use "git add ..." to include in what will be committed) >> # >> # obsolete/ >> nothing added to commit but untracked files present (use "git add" to track) >> >> $ git merge origin/master --ff-only >> Updating f419d57..2da6052 >> error: The following untracked working tree files would be overwritten by >> merge: >> obsolete/e107/Readme.txt >> obsolete/e107/article.php >> obsolete/e107/backend.php >> [...] > ... > Compare with what Subversion did in an analogous case: When I ran "svn > update" and the update brought new files for which there already was > an untracked copy in the working directory, Subversion: > - started to consider the file as tracked, > - but left the file in the working-copy alone. > > As a result, a subsequent "svn status" might > a) no longer show the file at all, if the foreign copy in the > working directory happened to be the same as the one brought by the > "svn update", or > b) flag the file as modified, if different from the one that "svn > update" would have created in its place. Interesting. So before running "status", the merge is recorded (in this particular case you are doing ff-only so there is nothing new to record, but if the rest of the tree merges cleanly, the new tree that contains "obsolete" from the other branch you just merged will be the contents you record in the merge commit), and working tree is immediately dirty? It makes sense, even though I haven't tried to think things through to see if there are tricky corner cases. > So my real question is, why does Git not do something analogous? The answer to that question is "because nobody thought that doing so would help users very much and bothered to implement it"; it is not "people thought about doing so and found reasons why that would not help users". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: Honor multi-part email messages
On Fri, Jan 25, 2013 at 07:28:54PM +0400, Alexey Shumkin wrote:
> "git format-patch --attach/--inline" generates multi-part messages.
> Every part of such messages can contain non-ASCII characters with its own
> "Content-Type" and "Content-Transfer-Encoding" headers.
> But git-send-mail script interprets a patch-file as one-part message
> and does not recognize multi-part messages.
> So already quoted printable email subject may be encoded as quoted printable
> again. Due to this bug email subject looks corrupted in email clients.
I don't think that the problem with the Subject is multi-part message
specific. The real problem with the Subject is probably that
is_rfc2047_quoted() does not detect that the Subject is already quoted.
Of course we still need that explicit multi-part message support to
avoid "Which 8bit encoding should I declare [UTF-8]? " message.
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 94c7f76..d49befe 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1499,12 +1499,17 @@ sub file_has_nonascii {
>
> sub body_or_subject_has_nonascii {
> my $fn = shift;
> + my $multipart = 0;
> open(my $fh, '<', $fn)
> or die "unable to open $fn: $!\n";
> while (my $line = <$fh>) {
> last if $line =~ /^$/;
> + if ($line =~ /^Content-Type:\s*multipart\/mixed.*$/) {
> + $multipart = 1;
> + }
> return 1 if $line =~ /^Subject.*[^[:ascii:]]/;
> }
> + return 0 if $multipart;
> while (my $line = <$fh>) {
> return 1 if $line =~ /[^[:ascii:]]/;
> }
After this change the function name is no longer appropriate.
Maybe we should join body_or_subject_has_nonascii()
and file_declares_8bit_cte() because in case of multi-part messages
"next unless (body_or_subject_has_nonascii($f)
&& !file_declares_8bit_cte($f));"
is not valid anymore. We could also check for broken_encoding
in single pass.
Thanks,
Krzysiek
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: segmentation fault (nullpointer) with git log --submodule -p
Jonathon Mah writes:
> Just to note, the proposals so far don't prevent a "smart-ass"
> function from freeing the buffer when it's called underneath the
> use/release scope, as in:
>
> with_commit_buffer(commit); {
> fn1_needing_buffer(commit);
> walk_rev_tree_or_something();
> fn2_needing_buffer(commit);
> } done_with_commit_buffer(commit);
I think the goal of everybody discussing these ideas is to make sure
that all code follows the simple ownership policy proposed at the
beginning of this subthread: commit->buffer belongs to the revision
traversal machinery, and other users could borrow it when available.
Even though your sample code will break, from that point of view, I
do not think it is something worth worrying about. If the function
"walk_rev_tree_or_something()" discards commit->buffer, it by
definition must be a part of the revision traversal machinery, and
any code that calls it inside with_commit_buffer() or uses the field
after such a call without revalidating commit->buffer, is already in
violation. With or without such a macro, we would need to be
careful about enforcing the ownership rule, and I think a code
structure like the above example is easier to spot problems in
during the review than the current code.
Because retaining commit->buffer is done for the benefit of the
next/future users of the data, and not for the users that _are_
using them right now, I do not think the usual refcounting that
discards when nobody references the data is a good match to the
problem we are discussing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resent] send-email: Honor multi-part email messages
Let's try to involve Krzysztof Mazur who have met the similar problem recently (according to this thread http://thread.gmane.org/gmane.comp.version-control.git/208297/focus=208310) I recitate myself: >Well, as I understand "current" algorithm: >1. It assumes that file is one-part email message >2. Function searches non-ASCII characters in Subject header >3. If none then it looks non-ASCII characters at message body > >my changes are to skip looking at message body of a multi-part >message as it has parts with their own Content-Type headers > >The said above in details: >1. To set flag when we meet Content-Type: multipart/mixed header >2. After we processed all headers and did not found non-ASCII characters >in a Subject we take a look at this flag and exit with 0 >if it is a multi-part message >>I think your patch is wrong. What happens when we see a Subject: >>line with a non-ascii on it that causes an early return of the loop >>before your new code has a chance to see Content-Type: header? This function is used to determine "broken" (non-ASCII) headers (to be encode them) The problem is if "Subject" is not broken, but message body contains non-ASCII chars, subject is marked as broken and encoded again. P.S. To involved: the beginning of thread is here http://thread.gmane.org/gmane.comp.version-control.git/181743 Alexey Shumkin (1): send-email: Honor multi-part email messages git-send-email.perl | 5 t/t9001-send-email.sh | 66 +++ 2 files changed, 71 insertions(+) -- 1.8.1.1.10.g9255f3f -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] send-email: Honor multi-part email messages
"git format-patch --attach/--inline" generates multi-part messages.
Every part of such messages can contain non-ASCII characters with its own
"Content-Type" and "Content-Transfer-Encoding" headers.
But git-send-mail script interprets a patch-file as one-part message
and does not recognize multi-part messages.
So already quoted printable email subject may be encoded as quoted printable
again. Due to this bug email subject looks corrupted in email clients.
Signed-off-by: Alexey Shumkin
---
git-send-email.perl | 5
t/t9001-send-email.sh | 66 +++
2 files changed, 71 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 94c7f76..d49befe 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1499,12 +1499,17 @@ sub file_has_nonascii {
sub body_or_subject_has_nonascii {
my $fn = shift;
+ my $multipart = 0;
open(my $fh, '<', $fn)
or die "unable to open $fn: $!\n";
while (my $line = <$fh>) {
last if $line =~ /^$/;
+ if ($line =~ /^Content-Type:\s*multipart\/mixed.*$/) {
+ $multipart = 1;
+ }
return 1 if $line =~ /^Subject.*[^[:ascii:]]/;
}
+ return 0 if $multipart;
while (my $line = <$fh>) {
return 1 if $line =~ /[^[:ascii:]]/;
}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 97d6f4c..c7ed370 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1323,4 +1323,70 @@ test_expect_success $PREREQ
'sendemail.aliasfile=~/.mailrc' '
grep "^!someone@example\.org!$" commandline1
'
+test_expect_success $PREREQ 'setup multi-part message' '
+cat >multi-part-email-using-8bit <
+From: aut...@example.com
+Date: Sat, 12 Jun 2010 15:53:58 +0200
+Subject: [PATCH]
=?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=20?=
+ =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB?=
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="123"
+
+This is a multi-part message in MIME format.
+--1.7.6.3.4.gf71f
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+This is a message created with "git format-patch --attach=123"
+---
+ master |1 +
+ файл |1 +
+ 2 files changed, 2 insertions(+), 0 deletions(-)
+ create mode 100644 master
+ create mode 100644 файл
+
+
+--123
+Content-Type: text/x-patch; name="0001-.patch"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename="0001-.patch"
+
+diff --git a/master b/master
+new file mode 100644
+index 000..1f7391f
+--- /dev/null
b/master
+@@ -0,0 +1 @@
++master
+diff --git a/файл b/файл
+new file mode 100644
+index 000..44e5cfe
+--- /dev/null
b/файл
+@@ -0,0 +1 @@
++содержимое файла
+
+--123--
+EOF
+'
+
+test_expect_success $PREREQ 'setup expect' '
+cat >expected actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.1.1.10.g9255f3f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
> Alexey Shumkin writes: > > >> Why do we want "whatever_7" variables and use "cut -c1-7" to > >> produce them? Is "7" something we care deeply about? > >> > >> I think what we care a lot more than "7" that happens to be the > >> current default value is to make sure that, if we ever update the > >> default abbreviation length to a larger value, the abbreviation > >> shown with --format=%h is consistent with the abbreviation that is > >> given by rev-parse --short. > >> > >> head1_short=$(git rev-parse --short $head1) > >> > >> perhaps? > >> ... > >> Likewise. > >> > >> > +tree2_7=$(echo $tree2 | cut -c1-7) > >> > >> Likewise. > > but is there "git something" to return abbreviated tree hash except > > "pretty formats" that is implicitly tested here? > > Does "git rev-parse --short $tree2" count? Oops! Yep! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
Alexey Shumkin writes: >> Why do we want "whatever_7" variables and use "cut -c1-7" to produce >> them? Is "7" something we care deeply about? >> >> I think what we care a lot more than "7" that happens to be the >> current default value is to make sure that, if we ever update the >> default abbreviation length to a larger value, the abbreviation >> shown with --format=%h is consistent with the abbreviation that is >> given by rev-parse --short. >> >> head1_short=$(git rev-parse --short $head1) >> >> perhaps? >> ... >> Likewise. >> >> > + tree2_7=$(echo $tree2 | cut -c1-7) >> >> Likewise. > but is there "git something" to return abbreviated tree hash except > "pretty formats" that is implicitly tested here? Does "git rev-parse --short $tree2" count? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows
Firefox on Windows by default is placed in "C:\Program Files\Mozilla Firefox"
folder, i.e. its path contains spaces. Before running this browser
"git-web--browse"
tests version of Firefox to decide whether to use "-new-tab" option or not.
Quote browser path to avoid error during this test.
Signed-off-by: Alexey Shumkin
Reviewed-by: Jeff King
---
git-web--browse.sh | 2 +-
t/t9901-git-web--browse.sh | 57 +-
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/git-web--browse.sh b/git-web--browse.sh
index 1e82726..f96e5bd 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -149,7 +149,7 @@ fi
case "$browser" in
firefox|iceweasel|seamonkey|iceape)
# Check version because firefox < 2.0 does not support "-new-tab".
- vers=$(expr "$($browser_path -version)" : '.* \([0-9][0-9]*\)\..*')
+ vers=$(expr "$("$browser_path" -version)" : '.* \([0-9][0-9]*\)\..*')
NEWTAB='-new-tab'
test "$vers" -lt 2 && NEWTAB=''
"$browser_path" $NEWTAB "$@" &
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index b0a6bad..30d5294 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -8,8 +8,21 @@ This test checks that git web--browse can handle various valid
URLs.'
. ./test-lib.sh
test_web_browse () {
- # browser=$1 url=$2
+ # browser=$1 url=$2 sleep_timeout=$3
+ sleep_timeout="$3"
git web--browse --browser="$1" "$2" >actual &&
+ # if $3 is set
+ # as far as Firefox is run in background (it is run with &)
+ # we trying to avoid race condition
+ # by waiting for "$sleep_timeout" seconds of timeout for
'fake_browser_ran' file appearance
+ (test -z "$sleep_timeout" || (
+ for timeout in $(seq 1 $sleep_timeout); do
+ test -f fake_browser_ran && break
+ sleep 1
+ done
+ test $timeout -ne $sleep_timeout
+ )
+ ) &&
tr -d '\015' text &&
test_cmp expect text
}
@@ -48,6 +61,48 @@ test_expect_success \
'
test_expect_success \
+ 'Firefox below v2.0 paths are properly quoted' '
+ echo fake: http://example.com/foo >expect &&
+ rm -f fake_browser_ran &&
+ cat >"fake browser" <<-\EOF &&
+ #!/bin/sh
+
+ : > fake_browser_ran
+ if test "$1" = "-version"; then
+ echo Fake Firefox browser version 1.2.3
+ else
+ # Firefox (in contrast to w3m) is run in background (with &)
+ # so redirect output to "actual"
+ echo fake: "$@" > actual
+ fi
+ EOF
+ chmod +x "fake browser" &&
+ git config browser.firefox.path "`pwd`/fake browser" &&
+ test_web_browse firefox http://example.com/foo 5
+'
+
+test_expect_success \
+ 'Firefox not lower v2.0 paths are properly quoted' '
+ echo fake: -new-tab http://example.com/foo >expect &&
+ rm -f fake_browser_ran &&
+ cat >"fake browser" <<-\EOF &&
+ #!/bin/sh
+
+ : > fake_browser_ran
+ if test "$1" = "-version"; then
+ echo Fake Firefox browser version 2.0.0
+ else
+ # Firefox (in contrast to w3m) is run in background (with &)
+ # so redirect output to "actual"
+ echo fake: "$@" > actual
+ fi
+ EOF
+ chmod +x "fake browser" &&
+ git config browser.firefox.path "`pwd`/fake browser" &&
+ test_web_browse firefox http://example.com/foo 5
+'
+
+test_expect_success \
'browser command allows arbitrary shell code' '
echo "arg: http://example.com/foo"; >expect &&
git config browser.custom.cmd "
--
1.8.1.1.10.g9255f3f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-core vs git package on ubuntu
Cc-ing the Git list again. Mario Michael Krell writes: > I am sorry for being so unspecific. I had problems finding your > mentioned website and quickly went through the "getting started" > tutorial and found the mentioned command at: > > http://git-scm.com/book/en/Getting-Started-Installing-Git OK, this is the "pro Git" book. I just submitted a pull request: https://github.com/progit/progit/pull/358 -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-core vs git package on ubuntu
On Fri, 25 Jan 2013 14:50:24 +0100 Mario Michael Krell wrote: > In your documentation you say, that git should be installed on Unix > using > > apt-get install git-core Note that Ubuntu is not Unix. > Unfortunately it tells the user, that this package is obsolete and > "git" should be used instead. Is this an error in the package manager > or in the website documentation? Neither of them. Git was packaged for Debian (and hence appeared in Ubuntu) when another package with the name "git" already existed in the archive, and was unrelated to Git. So the Git package maintainer picked the name "git-core". After that, the maintainers of both packages discussed this issue and the maintainer of the original package named "git" agreed to change the name of his package, and then, subsequently, "git-core" has been renamed to "git", and the "git-core" package has been turned into transitional dummy obsolete package. Now, whenever you're trying to install the "git-core" package, the package system tells you it's obsolete and suggests the "correct" package to install. After some time (the next OS release or later), the "git-core" package will be removed completely from the archive. This is the standard way to handle such situations in Debian and its derivatives, so nothing special here. The documentation on the whatever site you were referring to should probably be updated as git-core is obsolete even in the current stable release of Debian [1]. I'm not sure which LTS release of Ubuntu is currently supported, but you might check the state of the git-core package in it yourself, using [2]. 1. http://packages.debian.org/squeeze/git-core 2. http://packages.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-core vs git package on ubuntu
Mario Michael Krell writes: > Dear git developers, > > In your documentation you say, Which documentation? > that git should be installed on Unix using > > apt-get install git-core A quick grep shows that this is not the case in the documentation provided with Git, and it's not what I see on http://git-scm.com/download/linux either. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git-core vs git package on ubuntu
Dear git developers, In your documentation you say, that git should be installed on Unix using apt-get install git-core Unfortunately it tells the user, that this package is obsolete and "git" should be used instead. Is this an error in the package manager or in the website documentation? Greets Mario-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mergetools: Enhance tortoisemerge to work with
TortoiseGitMerge and filenames with spaces
- The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
(starting with 1.8.0) in order to make clear that this one has special
support for git, (uses spaces as cli parameter key-value separators)
and prevent confusion with the TortoiseSVN TortoiseMerge version.
- The tortoisemerge mergetool does not work with filenames which have
a space in it. Fixing this required changes in git and also in
TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.
Signed-off-by: Sven Strickroth
Reported-by: Sebastian Schuberth
---
mergetools/tortoisemerge | 51
1 file changed, 34 insertions(+), 17 deletions(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..8ee99a5 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -1,17 +1,34 @@
-can_diff () {
- return 1
-}
-
-merge_cmd () {
- if $base_present
- then
- touch "$BACKUP"
- "$merge_tool_path" \
- -base:"$BASE" -mine:"$LOCAL" \
- -theirs:"$REMOTE" -merged:"$MERGED"
- check_unchanged
- else
- echo "TortoiseMerge cannot be used without a base" 1>&2
- return 1
- fi
-}
+can_diff () {
+ return 1
+}
+
+merge_cmd () {
+ if $base_present
+ then
+ touch "$BACKUP"
+ basename="$(basename "$merge_tool_path" .exe)"
+ if test "$basename" = "tortoisegitmerge"
+ then
+ "$merge_tool_path" \
+ -base "$BASE" -mine "$LOCAL" \
+ -theirs "$REMOTE" -merged "$MERGED"
+ else
+ "$merge_tool_path" \
+ -base:"$BASE" -mine:"$LOCAL" \
+ -theirs:"$REMOTE" -merged:"$MERGED"
+ fi
+ check_unchanged
+ else
+ echo "$merge_tool_path cannot be used without a base" 1>&2
+ return 1
+ fi
+}
+
+translate_merge_tool_path() {
+ if type tortoisegitmerge >/dev/null 2>/dev/null
+ then
+ echo tortoisegitmerge
+ else
+ echo tortoisemerge
+ fi
+}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] branch: mark more strings for translation
---
builtin/branch.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ca61c5b..597b578 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct
ref_item *item,
int verbose, int abbrev)
{
struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
- const char *sub = " invalid ref ";
+ const char *sub = _(" invalid ref ");
struct commit *commit = item->commit;
if (commit && !parse_commit(commit)) {
@@ -590,7 +590,7 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
if (!filter)
- die("object '%s' does not point to a commit",
+ die(_("object '%s' does not point to a commit"),
sha1_to_hex(merge_filter_ref));
filter->object.flags |= UNINTERESTING;
@@ -854,7 +854,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (!argc) {
if (detached)
- die("Cannot give description to detached HEAD");
+ die(_("Cannot give description to detached
HEAD"));
branch_name = head;
} else if (argc == 1)
branch_name = argv[0];
@@ -866,10 +866,11 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
strbuf_release(&branch_ref);
if (!argc)
- return error("No commit on branch '%s' yet.",
+ return error(_("No commit on branch '%s' yet."),
branch_name);
else
- return error("No such branch '%s'.",
branch_name);
+ return error(_("No branch named '%s'."),
+branch_name);
}
strbuf_release(&branch_ref);
--
1.8.0.rc2.23.g1fb49df
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] branch: give a more helpful message on redundant arguments
While at there, do not stop user from editing a branch description
when the unrelated HEAD is detached.
---
builtin/branch.c | 12 ++--
t/t3200-branch.sh | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 50fcacc..ca61c5b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -852,14 +852,14 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
const char *branch_name;
struct strbuf branch_ref = STRBUF_INIT;
- if (detached)
- die("Cannot give description to detached HEAD");
- if (!argc)
+ if (!argc) {
+ if (detached)
+ die("Cannot give description to detached HEAD");
branch_name = head;
- else if (argc == 1)
+ } else if (argc == 1)
branch_name = argv[0];
else
- usage_with_options(builtin_branch_usage, options);
+ die(_("cannot edit description of more than one
branch"));
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf)) {
@@ -881,7 +881,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
else if (argc == 2)
rename_branch(argv[0], argv[1], rename > 1);
else
- usage_with_options(builtin_branch_usage, options);
+ die(_("too many branches for a rename operation"));
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 80e6be3..f3e0e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -73,8 +73,8 @@ test_expect_success \
test_expect_success \
'git branch -m dumps usage' \
- 'test_expect_code 129 git branch -m 2>err &&
- test_i18ngrep "[Uu]sage: git branch" err'
+ 'test_expect_code 128 git branch -m 2>err &&
+ test_i18ngrep "too many branches for a rename operation" err'
test_expect_success \
'git branch -m m m/m should work' \
--
1.8.0.rc2.23.g1fb49df
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] branch: reject -D/-d without branch name
---
builtin/branch.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..50fcacc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
colopts = 0;
}
- if (delete)
+ if (delete) {
+ if (!argc)
+ die(_("branch name required"));
return delete_branches(argc, argv, delete > 1, kinds, quiet);
- else if (list) {
+ } else if (list) {
int ret = print_ref_list(kinds, detached, verbose, abbrev,
with_commit, argv);
print_columns(&output, colopts, NULL);
--
1.8.0.rc2.23.g1fb49df
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
On Fri, Jan 25, 2013 at 11:34 AM, Sebastian Schuberth wrote: >> I thought Git did something sensible there like create a normal file? > > It does not. Also see my answer over here: > http://stackoverflow.com/questions/11412028/git-not-storing-symlink-as-a-symlink-on-windows/11412341#11412341 This one might be the even more appropriate answer: http://stackoverflow.com/questions/11662868/what-happens-when-i-clone-a-repository-with-symlinks-on-windows/11664406#11664406 -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
> Why do we want "whatever_7" variables and use "cut -c1-7" to produce > them? Is "7" something we care deeply about? > > I think what we care a lot more than "7" that happens to be the > current default value is to make sure that, if we ever update the > default abbreviation length to a larger value, the abbreviation > shown with --format=%h is consistent with the abbreviation that is > given by rev-parse --short. > > head1_short=$(git rev-parse --short $head1) > > perhaps? > > > + echo changed >foo && > > + git commit -a -m "changed foo" && > > + head2=$(git rev-parse --verify HEAD) && > > + head2_7=$(echo $head2 | cut -c1-7) && > > + head2_parent=$(git cat-file -p HEAD | grep parent | cut -f > > 2 -d" ") && > > Do not use "cat-file -p" that is for human consumption in scripts, > unless you are testing how the format for human consumption should > look like (we may later add more pretty-print to them), which is not > the case here. > > Also be careful and pay attention to the end of the header; you do > not want to pick up a random "parent" string in the middle of a log > message. > > head2_parent=$(git cat-file commit HEAD | sed -n -e > "s/^parent //p" -e "/^$/q") > > would be much better. > > > + head2_parent_7=$(echo $head2_parent | cut -c1-7) && > > + tree2=$(git cat-file -p HEAD | grep tree | cut -f 2 -d" ") > > && > > Likewise. > > > + tree2_7=$(echo $tree2 | cut -c1-7) > > Likewise. but is there "git something" to return abbreviated tree hash except "pretty formats" that is implicitly tested here? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] add: warn when -u or -A is used without filepattern
Most git commands that can be used with our without a filepattern are
tree-wide by default, the filepattern being used to restrict their scope.
A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'.
The inconsistancy of 'git add -u' and 'git add -A' are particularly
problematic since other 'git add' subcommands (namely 'git add -p' and
'git add -e') are tree-wide by default.
Flipping the default now is unacceptable, so this patch starts training
users to type explicitely 'git add -u|-A :/' or 'git add -u|-A .', to prepare
for the next steps:
* forbid 'git add -u|-A' without filepattern (like 'git add' without
option)
* much later, maybe, re-allow 'git add -u|-A' without filepattern, with a
tree-wide scope.
A nice side effect of this patch is that it makes the :/ special
filepattern easier to discover for users.
When the command is called from the root of the tree, there is no
ambiguity and no need to change the behavior, hence no need to warn.
Signed-off-by: Matthieu Moy
---
Changes since v1:
* Do not warn from the root of the tree.
* Say explicitely "Git 2.0" to announce the change.
(plus fix a C99 style issue)
Documentation/git-add.txt | 7 ---
builtin/add.c | 36 +++-
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index fd9e36b..5333559 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -107,9 +107,10 @@ apply to the index. See EDITING PATCHES below.
from the index if the corresponding files in the working tree
have been removed.
+
-If no is given, default to "."; in other words,
-update all tracked files in the current directory and its
-subdirectories.
+If no is given, the current version of Git defaults to
+"."; in other words, update all tracked files in the current directory
+and its subdirectories. This default will change in a future version
+of Git, hence the form without should not be used.
-A::
--all::
diff --git a/builtin/add.c b/builtin/add.c
index e664100..8252d19 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -363,6 +363,33 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
+static void warn_pathless_add(const char *option_name) {
+ /*
+* To be consistant with "git add -p" and most Git
+* commands, we should default to being tree-wide, but
+* this is not the original behavior and can't be
+* changed until users trained themselves not to type
+* "git add -u" or "git add -A". For now, we warn and
+* keep the old behavior. Later, this warning can be
+* turned into a die(...), and eventually we may
+* reallow the command with a new behavior.
+*/
+ warning(_("The behavior of 'git add %s' with no path argument from a
subdirectory of the\n"
+ "tree will change in Git 2.0 and shouldn't be used anymore.\n"
+ "To add content for the whole tree, run:\n"
+ "\n"
+ " git add %s :/\n"
+ "\n"
+ "To restrict the command to the current directory, run:\n"
+ "\n"
+ " git add %s .\n"
+ "\n"
+ "With the current Git version, the command is restricted to
the current directory."),
+ option_name,
+ option_name,
+ option_name);
+}
+
int cmd_add(int argc, const char **argv, const char *prefix)
{
int exit_status = 0;
@@ -373,6 +400,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ const char *option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -392,8 +420,14 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
die(_("-A and -u are mutually incompatible"));
if (!show_only && ignore_missing)
die(_("Option --ignore-missing can only be used together with
--dry-run"));
- if ((addremove || take_worktree_changes) && !argc) {
+ if (addremove)
+ option_with_implicit_dot = "--all";
+ if (take_worktree_changes)
+ option_with_implicit_dot = "--update";
+ if (option_with_implicit_dot && !argc) {
static const char *here[2] = { ".", NULL };
+ if (prefix)
+ warn_pathless_add(option_with_implicit_dot);
argc = 1;
argv = here;
}
--
1.8.0.1.527.gd366564.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] git-difftool: use git-mergetool--lib for "--tool-help"
On Fri, Jan 25, 2013 at 01:55:03AM -0800, David Aguilar wrote: > list_merge_tool_candidates() has a bunch of other special cases > for $EDITOR, $DISPLAY, $GNOME-something and such so I think > we should keep using it only for the guess_merge_tool() path. > > I honestly want to remove list_merge_tool_candidates every > time I read it, but I recognize that it does serve a purpose > for users who have not configured anything. Actually, I'm not sure it does. I asked one of my colleagues whether he used git-mergetool the other day and he said no because he couldn't understand the OS X FileMerge tool and was happier to edit things manually in vim. I don't think he'd realised that he could configure a different mergetool. Perhaps we're trying to be too clever by guessing what the user wants and should instead exit with a message saying: You have not configured a merge tool to use. Please select one of the following tools and configure it using: git config merge.tool These tools are availalble on your system: ... These tools are supported but unavailable: ... This may be too much of a regression for current users though. John -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
On Fri, Jan 25, 2013 at 2:38 AM, John Keeping wrote: > On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote: >> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty >> list "vim" as being an unavailable tool. This is because they attempt >> to find a tool named according to the mergetool scriptlet instead of the Git- >> recognized tool name. > > Actually, after my patches both git-difftool and git-mergetool get this > right since list_merge_tool_candidates lists vimdiff and gvimdiff. > >> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim" >> scriptlet. This required git-mergetool--lib to special-case it when >> setting up the tool. >> >> Remove the exception for "vim" and allow the scriptlets to be found >> naturally by using symlinks to a single "vimdiff" scriptlet. > > I wonder if it would be better to make these single-line scripts instead > of symlinks: > > . "$MERGE_TOOLS_DIR"/vimdiff > > where we make git-mergetool--lib.sh export: > > MERGE_TOOLS_DIR=$(git --exec-path)/mergetools That sounds like the way to go. -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote: > "git difftool --tool-help" and "git mergetool --tool-help" incorreclty > list "vim" as being an unavailable tool. This is because they attempt > to find a tool named according to the mergetool scriptlet instead of the Git- > recognized tool name. Actually, after my patches both git-difftool and git-mergetool get this right since list_merge_tool_candidates lists vimdiff and gvimdiff. > vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim" > scriptlet. This required git-mergetool--lib to special-case it when > setting up the tool. > > Remove the exception for "vim" and allow the scriptlets to be found > naturally by using symlinks to a single "vimdiff" scriptlet. I wonder if it would be better to make these single-line scripts instead of symlinks: . "$MERGE_TOOLS_DIR"/vimdiff where we make git-mergetool--lib.sh export: MERGE_TOOLS_DIR=$(git --exec-path)/mergetools John -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git merge error question: The following untracked working tree files would be overwritten by merge
Hi all, in my repo, I'm doing this: > $ git status > # On branch master > # Your branch is behind 'origin/master' by 2 commits, and can be fast-forwarded. > # > # Untracked files: > # (use "git add ..." to include in what will be committed) > # > # obsolete/ > nothing added to commit but untracked files present (use "git add" to track) > > $ git merge origin/master --ff-only > Updating f419d57..2da6052 > error: The following untracked working tree files would be overwritten by merge: > obsolete/e107/Readme.txt > obsolete/e107/article.php > obsolete/e107/backend.php > [...] That is, the local repository has the untracked directory "obsolete", which was added upstream as well, and now I try to reconcile. I seem to understand the problem stated in the error message, and the solution seems to be simple as well: renaming the obsolete/ directory is enough. But why does Git find a problem here at all? Compare with what Subversion did in an analogous case: When I ran "svn update" and the update brought new files for which there already was an untracked copy in the working directory, Subversion: - started to consider the file as tracked, - but left the file in the working-copy alone. As a result, a subsequent "svn status" might a) no longer show the file at all, if the foreign copy in the working directory happened to be the same as the one brought by the "svn update", or b) flag the file as modified, if different from the one that "svn update" would have created in its place. So my real question is, why does Git not do something analogous? (Afaics, update the HEAD, update the Index, but leave the working-copy edition alone?) I searched for this beforehand, and most advice involves either stashing, or with "git reset --hard" the loss of the untracked files. Sorry if this is a stupid question -- I still consider myself a Git learner. Best regards, Carsten -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
On Fri, Jan 25, 2013 at 2:23 AM, Sebastian Schuberth wrote: > On 2013/01/25 10:43 , David Aguilar wrote: > >> Remove the exception for "vim" and allow the scriptlets to be found >> naturally by using symlinks to a single "vimdiff" scriptlet. This > > > I guess that won't work on platforms where Git does not support symlinks, > then, like Windows. But Windows has (g)vimdiff, so loosing these on that > platform would not be so nice. I thought Git did something sensible there like create a normal file? If not then I was thinking we could add a provides_tools() function that each scriptlet could supply. -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
On 2013/01/25 10:43 , David Aguilar wrote: Remove the exception for "vim" and allow the scriptlets to be found naturally by using symlinks to a single "vimdiff" scriptlet. This I guess that won't work on platforms where Git does not support symlinks, then, like Windows. But Windows has (g)vimdiff, so loosing these on that platform would not be so nice. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Enhance tortoisemerge to work with
On Fri, Jan 25, 2013 at 1:06 AM, Sven Strickroth
wrote:
> TortoiseGitMerge and filenames with spaces
>
> - The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
> (starting with 1.8.0) in order to make clear that this one has special
> support for git, (uses spaces as cli parameter key-value separators)
> and prevent confusion with the TortoiseSVN TortoiseMerge version.
> - The tortoisemerge mergetool does not work with filenames which have
> a space in it. Fixing this required changes in git and also in
> TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.
>
> Signed-off-by: Sven Strickroth
> Reported-by: Sebastian Schuberth
> ---
> mergetools/tortoisemerge | 24
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
> index ed7db49..9890737 100644
> --- a/mergetools/tortoisemerge
> +++ b/mergetools/tortoisemerge
> @@ -6,12 +6,28 @@ merge_cmd () {
> if $base_present
> then
> touch "$BACKUP"
> - "$merge_tool_path" \
> - -base:"$BASE" -mine:"$LOCAL" \
> - -theirs:"$REMOTE" -merged:"$MERGED"
> + if test "$merge_tool_path" == "tortoisegitmerge"
I like the approach this is taking. Thank you.
I have one small note:
I think this should use "=" instead of "==" here.
It might also make sense to wrap a basename call around it
so that users can set their own mergetool.tortoisemerge.path
basename="$(basename "$merge_tool_path" .exe)"
if test "$basename" = "tortoisegitmerge"
...
> + then
> + "$merge_tool_path" \
> + -base "$BASE" -mine "$LOCAL" \
> + -theirs "$REMOTE" -merged "$MERGED"
> + else
> + "$merge_tool_path" \
> + -base:"$BASE" -mine:"$LOCAL" \
> + -theirs:"$REMOTE" -merged:"$MERGED"
> + fi
> check_unchanged
> else
> - echo "TortoiseMerge cannot be used without a base" 1>&2
> + echo "$merge_tool_path cannot be used without a base" 1>&2
> return 1
> fi
> }
> +
> +translate_merge_tool_path() {
> + if type tortoisegitmerge >/dev/null 2>/dev/null
> + then
> + echo tortoisegitmerge
> + else
> + echo tortoisemerge
> + fi
> +}
> --
> Best regards,
> Sven Strickroth
> PGP key id F5A9D4C4 @ any key-server
--
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] git-difftool: use git-mergetool--lib for "--tool-help"
On Fri, Jan 25, 2013 at 1:19 AM, John Keeping wrote: > On Thu, Jan 24, 2013 at 09:29:58PM -0800, David Aguilar wrote: >> On Thu, Jan 24, 2013 at 11:55 AM, John Keeping wrote: >> > The "--tool-help" option to git-difftool currently displays incorrect >> > output since it uses the names of the files in >> > "$GIT_EXEC_PATH/mergetools/" rather than the list of command names in >> > git-mergetool--lib. >> > >> > Fix this by simply delegating the "--tool-help" argument to the >> > show_tool_help function in git-mergetool--lib. >> >> Very nice. >> >> One thought I had was that the unified show_tool_help should >> probably check TOOL_MODE=diff and skip over the >> !can_diff entries. >> >> The current output of "git difftool --tool-help" before your >> patches has the problem that it will list tools such as >> "tortoisemerge" as "valid but not available" because it >> does not differentiate between missing and !can_diff. > > list_merge_tool_candidates does this for us, so it should Just Work > since we use that to generate the list of tools that we loop over. Yup, kind of. I looked more closely and found a lot of special-cases around vim which I've eliminated in the series I just sent (which includes your patches as its base). list_merge_tool_candidates() has a bunch of other special cases for $EDITOR, $DISPLAY, $GNOME-something and such so I think we should keep using it only for the guess_merge_tool() path. I honestly want to remove list_merge_tool_candidates every time I read it, but I recognize that it does serve a purpose for users who have not configured anything. In order to be useful for the documentation I think the code could be refactored a tiny bit more beyond what I've sent so far. The gathering of available tools can be split off from the reporting, and then show_tool_help() can use the gatherer. With what I sent, though, there's at least a 1:1 correspondance between the name of the scriptlets and the names accepted by --tool=, which is the first step towards doing that. I have to check out for now but I'll keep poking at this when the weekend rolls around. cheers, -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Add tortoisegitmerge helper
On Thu, Jan 24, 2013 at 11:54:25PM -0800, David Aguilar wrote:
> On Thu, Jan 24, 2013 at 11:21 PM, Junio C Hamano wrote:
> > Is there a way for me to programatically tell what merge.tool and
> > diff.tool could be enabled for a particular source checkout of Git
> > regardless of what platform am I on (that is, even though I won't
> > touch Windows, I want to see 'tortoise' appear in the output of such
> > a procedure)? We could generate a small text file from the Makefile
> > in Documentation and include it when building the manual pages if
> > such a procedure is available.
>
> That's a good idea.
> Here's one way... (typed into gmail, so probably broken)
>
> LF='
> '
> mergetools=
> difftools=
> scriptlets="$(git --exec-path)"/mergetools
>
> for script in "$scriptlets"/*
> do
> tool="$(basename "$script")"
> if test "$tool" = "defaults"
> then
> continue
> fi
> . "$scriptlets"/defaults
> can_diff && difftools="$difftools$tool$LF"
> can_merge && mergetools="$mergetools$tool$LF"
> done
I don't think this will work since the names of the valid tools are not
necessarily the same as the names of the scriptlets - this is the exact
issue that prompted my patches to git-difftool yesterday.
The best option I can see given what's currently available is something
like this:
-- >8 --
sed -n -e '/^list_merge_tool_candidates/,/^}/ {
/tools=/ {
s/.*tools=//
s/"//g
s/\$tools//
s/ /\n/g
p
}
}' git-mergetool--lib.sh |sort |uniq |while read -r tool
do
test -z "$tool" && continue
( . git-mergetool--lib && setup_tool $tool
# Use can_diff and can_merge here.
)
done
-- 8< --
John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] git-mergetool: remove redundant assignment
From: John Keeping
TOOL_MODE is set at the top of git-mergetool.sh so there is no need to
set it again in show_tool_help. Removing this lets us re-use
show_tool_help in git-difftool.
Signed-off-by: John Keeping
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 89a857f..1748315 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -175,7 +175,6 @@ list_merge_tool_candidates () {
}
show_tool_help () {
- TOOL_MODE=merge
list_merge_tool_candidates
unavailable= available= LF='
'
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] git-mergetool: move show_tool_help to mergetool--lib
From: John Keeping
This is the first step in unifying "git difftool --tool-help" and
"git mergetool --tool-help".
Signed-off-by: John Keeping
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 37 +
git-mergetool.sh | 37 -
2 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f013a03..89a857f 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -174,6 +174,43 @@ list_merge_tool_candidates () {
esac
}
+show_tool_help () {
+ TOOL_MODE=merge
+ list_merge_tool_candidates
+ unavailable= available= LF='
+'
+ for i in $tools
+ do
+ merge_tool_path=$(translate_merge_tool_path "$i")
+ if type "$merge_tool_path" >/dev/null 2>&1
+ then
+ available="$available$i$LF"
+ else
+ unavailable="$unavailable$i$LF"
+ fi
+ done
+ if test -n "$available"
+ then
+ echo "'git mergetool --tool=' may be set to one of the
following:"
+ echo "$available" | sort | sed -e 's/^/ /'
+ else
+ echo "No suitable tool for 'git mergetool --tool=' found."
+ fi
+ if test -n "$unavailable"
+ then
+ echo
+ echo 'The following tools are valid, but not currently
available:'
+ echo "$unavailable" | sort | sed -e 's/^/ /'
+ fi
+ if test -n "$unavailable$available"
+ then
+ echo
+ echo "Some of the tools listed above only work in a windowed"
+ echo "environment. If run in a terminal-only session, they will
fail."
+ fi
+ exit 0
+}
+
guess_merge_tool () {
list_merge_tool_candidates
echo >&2 "merge tool candidates: $tools"
diff --git a/git-mergetool.sh b/git-mergetool.sh
index c50e18a..c0ee9aa 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -315,43 +315,6 @@ merge_file () {
return 0
}
-show_tool_help () {
- TOOL_MODE=merge
- list_merge_tool_candidates
- unavailable= available= LF='
-'
- for i in $tools
- do
- merge_tool_path=$(translate_merge_tool_path "$i")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- available="$available$i$LF"
- else
- unavailable="$unavailable$i$LF"
- fi
- done
- if test -n "$available"
- then
- echo "'git mergetool --tool=' may be set to one of the
following:"
- echo "$available" | sort | sed -e 's/^/ /'
- else
- echo "No suitable tool for 'git mergetool --tool=' found."
- fi
- if test -n "$unavailable"
- then
- echo
- echo 'The following tools are valid, but not currently
available:'
- echo "$unavailable" | sort | sed -e 's/^/ /'
- fi
- if test -n "$unavailable$available"
- then
- echo
- echo "Some of the tools listed above only work in a windowed"
- echo "environment. If run in a terminal-only session, they will
fail."
- fi
- exit 0
-}
-
prompt=$(git config --bool mergetool.prompt || echo true)
while test $# != 0
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
"git difftool --tool-help" and "git mergetool --tool-help" incorreclty
list "vim" as being an unavailable tool. This is because they attempt
to find a tool named according to the mergetool scriptlet instead of the Git-
recognized tool name.
vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
scriptlet. This required git-mergetool--lib to special-case it when
setting up the tool.
Remove the exception for "vim" and allow the scriptlets to be found
naturally by using symlinks to a single "vimdiff" scriptlet. This
causes the --tool-help option to correctly list vimdiff, vimdiff2,
gvimdiff, and gvimdiff2 in its output.
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 9 +
mergetools/gvimdiff | 1 +
mergetools/gvimdiff2| 1 +
mergetools/{vim => vimdiff} | 0
mergetools/vimdiff2 | 1 +
5 files changed, 4 insertions(+), 8 deletions(-)
create mode 12 mergetools/gvimdiff
create mode 12 mergetools/gvimdiff2
rename mergetools/{vim => vimdiff} (100%)
create mode 12 mergetools/vimdiff2
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4c1e129..db8218a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -44,14 +44,7 @@ valid_tool () {
}
setup_tool () {
- case "$1" in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool="$1"
- ;;
- esac
+ tool="$1"
mergetools="$(git --exec-path)/mergetools"
# Load the default definitions
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 12
index 000..2a72558
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+vimdiff
\ No newline at end of file
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 12
index 000..2a72558
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+vimdiff
\ No newline at end of file
diff --git a/mergetools/vim b/mergetools/vimdiff
similarity index 100%
rename from mergetools/vim
rename to mergetools/vimdiff
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
new file mode 12
index 000..2a72558
--- /dev/null
+++ b/mergetools/vimdiff2
@@ -0,0 +1 @@
+vimdiff
\ No newline at end of file
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] mergetool--lib: Improve show_tool_help() output
Check the can_diff and can_merge functions before deciding whether to
add the tool to the available/unavailable lists. This makes --tool-help
context-
sensitive so that "git mergetool --tool-help" displays merge tools only
and "git difftool --tool-help" displays diff tools only.
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index db8218a..c547c59 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -168,17 +168,33 @@ list_merge_tool_candidates () {
}
show_tool_help () {
- list_merge_tool_candidates
unavailable= available= LF='
'
- for i in $tools
+
+ scriptlets="$(git --exec-path)"/mergetools
+ for i in "$scriptlets"/*
do
- merge_tool_path=$(translate_merge_tool_path "$i")
+ . "$scriptlets"/defaults
+ . "$i"
+
+ tool="$(basename "$i")"
+ if test "$tool" = "defaults"
+ then
+ continue
+ elif merge_mode && ! can_merge
+ then
+ continue
+ elif diff_mode && ! can_diff
+ then
+ continue
+ fi
+
+ merge_tool_path=$(translate_merge_tool_path "$tool")
if type "$merge_tool_path" >/dev/null 2>&1
then
- available="$available$i$LF"
+ available="$available$tool$LF"
else
- unavailable="$unavailable$i$LF"
+ unavailable="$unavailable$tool$LF"
fi
done
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] git-difftool: use git-mergetool--lib for "--tool-help"
From: John Keeping
The "--tool-help" option to git-difftool currently displays incorrect
output since it uses the names of the files in
"$GIT_EXEC_PATH/mergetools/" rather than the list of command names in
git-mergetool--lib.
Fix this by simply delegating the "--tool-help" argument to the
show_tool_help function in git-mergetool--lib.
Signed-off-by: John Keeping
Signed-off-by: David Aguilar
---
git-difftool.perl | 55 +++
1 file changed, 7 insertions(+), 48 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index edd0493..0a90de4 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -59,57 +59,16 @@ sub find_worktree
return $worktree;
}
-sub filter_tool_scripts
-{
- my ($tools) = @_;
- if (-d $_) {
- if ($_ ne ".") {
- # Ignore files in subdirectories
- $File::Find::prune = 1;
- }
- } else {
- if ((-f $_) && ($_ ne "defaults")) {
- push(@$tools, $_);
- }
- }
-}
-
sub print_tool_help
{
- my ($cmd, @found, @notfound, @tools);
- my $gitpath = Git::exec_path();
-
- find(sub { filter_tool_scripts(\@tools) }, "$gitpath/mergetools");
-
- foreach my $tool (@tools) {
- $cmd = "TOOL_MODE=diff";
- $cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"';
- $cmd .= " && get_merge_tool_path $tool >/dev/null 2>&1";
- $cmd .= " && can_diff >/dev/null 2>&1";
- if (system('sh', '-c', $cmd) == 0) {
- push(@found, $tool);
- } else {
- push(@notfound, $tool);
- }
- }
-
- print << 'EOF';
-'git difftool --tool=' may be set to one of the following:
-EOF
- print "\t$_\n" for (sort(@found));
+ my $cmd = 'TOOL_MODE=diff';
+ $cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"';
+ $cmd .= ' && show_tool_help';
- print << 'EOF';
-
-The following tools are valid, but not currently available:
-EOF
- print "\t$_\n" for (sort(@notfound));
-
- print << 'EOF';
-
-NOTE: Some of the tools listed above only work in a windowed
-environment. If run in a terminal-only session, they will fail.
-EOF
- exit(0);
+ # See the comment at the bottom of file_diff() for the reason behind
+ # using system() followed by exit() instead of exec().
+ my $rc = system('sh', '-c', $cmd);
+ exit($rc | ($rc >> 8));
}
sub exit_cleanup
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] mergetools/vim: Remove redundant diff command
vimdiff and vimdiff2 differ only by their merge command so remove the
logic in the diff command since it's not actually needed.
Signed-off-by: David Aguilar
---
mergetools/vim | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/mergetools/vim b/mergetools/vim
index 619594a..39d0327 100644
--- a/mergetools/vim
+++ b/mergetools/vim
@@ -1,14 +1,6 @@
diff_cmd () {
- case "$1" in
- gvimdiff|vimdiff)
- "$merge_tool_path" -R -f -d \
- -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
- ;;
- gvimdiff2|vimdiff2)
- "$merge_tool_path" -R -f -d \
- -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
- ;;
- esac
+ "$merge_tool_path" -R -f -d \
+ -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
}
merge_cmd () {
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] git-mergetool: don't hardcode 'mergetool' in show_tool_help
From: John Keeping
When using show_tool_help from git-difftool we will want it to print
"git difftool" not "git mergetool" so use "git ${TOOL_MODE}tool".
Signed-off-by: John Keeping
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1748315..4c1e129 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -188,12 +188,14 @@ show_tool_help () {
unavailable="$unavailable$i$LF"
fi
done
+
+ cmd_name=${TOOL_MODE}tool
if test -n "$available"
then
- echo "'git mergetool --tool=' may be set to one of the
following:"
+ echo "'git $cmd_name --tool=' may be set to one of the
following:"
echo "$available" | sort | sed -e 's/^/ /'
else
- echo "No suitable tool for 'git mergetool --tool=' found."
+ echo "No suitable tool for 'git $cmd_name --tool=' found."
fi
if test -n "$unavailable"
then
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] mergetool-lib improvements for --tool-help
I ran with John's idea and simplified a few more things so
that the --tool-help output shows the correct results.
My final commit is dependent on John's commits but the other two
could be picked up independently since they are general improvements.
This does add a few symlinks to the repo for the sake of simplicity;
I'm not sure if this presents a problem for folks on other platforms.
The replacement of vim with vimdiff and symlinks simplifies things
so that we can later auto-generate the list-of-all-tools for use
by Documentation/. This was not previously possible due to the
special-case for the vim tools which this series eliminates.
David Aguilar (3):
mergetools/vim: Remove redundant diff command
mergetools: Fix difftool/mergetool --tool-help listing for vim
mergetool--lib: Improve show_tool_help() output
John Keeping (4):
git-mergetool: move show_tool_help to mergetool--lib
git-mergetool: remove redundant assignment
git-mergetool: don't hardcode 'mergetool' in show_tool_help
git-difftool: use git-mergetool--lib for "--tool-help"
git-difftool.perl | 55 +--
git-mergetool--lib.sh | 63 +++--
git-mergetool.sh| 37 --
mergetools/gvimdiff | 1 +
mergetools/gvimdiff2| 1 +
mergetools/{vim => vimdiff} | 12 ++---
mergetools/vimdiff2 | 1 +
7 files changed, 67 insertions(+), 103 deletions(-)
create mode 12 mergetools/gvimdiff
create mode 12 mergetools/gvimdiff2
rename mergetools/{vim => vimdiff} (68%)
create mode 12 mergetools/vimdiff2
--
1.8.1.1.367.g22b1720.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] git-difftool: use git-mergetool--lib for "--tool-help"
On Thu, Jan 24, 2013 at 09:29:58PM -0800, David Aguilar wrote: > On Thu, Jan 24, 2013 at 11:55 AM, John Keeping wrote: > > The "--tool-help" option to git-difftool currently displays incorrect > > output since it uses the names of the files in > > "$GIT_EXEC_PATH/mergetools/" rather than the list of command names in > > git-mergetool--lib. > > > > Fix this by simply delegating the "--tool-help" argument to the > > show_tool_help function in git-mergetool--lib. > > Very nice. > > One thought I had was that the unified show_tool_help should > probably check TOOL_MODE=diff and skip over the > !can_diff entries. > > The current output of "git difftool --tool-help" before your > patches has the problem that it will list tools such as > "tortoisemerge" as "valid but not available" because it > does not differentiate between missing and !can_diff. list_merge_tool_candidates does this for us, so it should Just Work since we use that to generate the list of tools that we loop over. John -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
> Alexey Shumkin writes: > > > The expected SHA-1 digests are always available in variables. Use > > them instead of hardcoding. > > > > Signed-off-by: Alexey Shumkin > > --- > > t/t6006-rev-list-format.sh | 130 > > + 1 file changed, 72 > > insertions(+), 58 deletions(-) > > > > diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh > > index f94f0c4..c248509 100755 > > --- a/t/t6006-rev-list-format.sh > > +++ b/t/t6006-rev-list-format.sh > > @@ -6,8 +6,19 @@ test_description='git rev-list --pretty=format > > test' > > test_tick > > test_expect_success 'setup' ' > > -touch foo && git add foo && git commit -m "added foo" && > > - echo changed >foo && git commit -a -m "changed foo" > > + touch foo && > > This is inherited from the original, but these days we try to avoid > touch, unless it is about setting a new file timestamp. The > canonical (in the script we write) way to create an empty file is: > > : >foo > > with or without ": ", it does not matter that much. Ok! > > > + git add foo && > > + git commit -m "added foo" && > > + head1=$(git rev-parse --verify HEAD) && > > + head1_7=$(echo $head1 | cut -c1-7) && > > Why do we want "whatever_7" variables and use "cut -c1-7" to produce > them? Is "7" something we care deeply about? I did not spend too much time to think of names of variables at the moment I was writing this code ) > > I think what we care a lot more than "7" that happens to be the > current default value is to make sure that, if we ever update the > default abbreviation length to a larger value, the abbreviation > shown with --format=%h is consistent with the abbreviation that is > given by rev-parse --short. > > head1_short=$(git rev-parse --short $head1) > > perhaps? It's an inherited code from 1.5 years ago Git ;) taken from some other tests I'll change it as you propose ) > > > + echo changed >foo && > > + git commit -a -m "changed foo" && > > + head2=$(git rev-parse --verify HEAD) && > > + head2_7=$(echo $head2 | cut -c1-7) && > > + head2_parent=$(git cat-file -p HEAD | grep parent | cut -f > > 2 -d" ") && > > Do not use "cat-file -p" that is for human consumption in scripts, > unless you are testing how the format for human consumption should > look like (we may later add more pretty-print to them), which is not > the case here. > > Also be careful and pay attention to the end of the header; you do > not want to pick up a random "parent" string in the middle of a log > message. > > head2_parent=$(git cat-file commit HEAD | sed -n -e > "s/^parent //p" -e "/^$/q") > > would be much better. yep! you're definitely right > > > + head2_parent_7=$(echo $head2_parent | cut -c1-7) && > > + tree2=$(git cat-file -p HEAD | grep tree | cut -f 2 -d" ") > > && > > Likewise. > > > + tree2_7=$(echo $tree2 | cut -c1-7) > > Likewise. > > > @@ -131,39 +142,42 @@ This commit message is much longer than the > > others, and it will be encoded in iso8859-1. We should therefore > > include an iso8859 character: ¡bueno! > > EOF > > + > > test_expect_success 'setup complex body' ' > > -git config i18n.commitencoding iso8859-1 && > > - echo change2 >foo && git commit -a -F commit-msg > > + git config i18n.commitencoding iso8859-1 && > > + echo change2 >foo && git commit -a -F commit-msg && > > + head3=$(git rev-parse --verify HEAD) && > > + head3_7=$(echo $head3 | cut -c1-7) > > ' > > Likewise. > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jan 2013, #08; Tue, 22)
On Thu, Jan 24, 2013 at 10:55:57PM -0600, Chris Rorvick wrote: > On Wed, Jan 23, 2013 at 3:12 PM, John Keeping wrote: > > The existing script (git-cvsimport.perl) won't ever work with cvsps-3 > > since features it relies on have been removed. > > Not reporting the ancestry branch seems to be the big one. Are there > others? For some reason I thought the non-fast-export output mode had already been removed, but now that I check it looks like it's still there just with a warning that it may be removed in the future. John -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] t7102 (reset): refactoring: don't hardcode SHA-1 in expected outputs
> Alexey Shumkin writes:
>
> > The expected SHA-1 digests are always available in variables. Use
> > them instead of hardcoding.
> >
> > Signed-off-by: Alexey Shumkin
> > ---
>
> Looks good (" refactoring:" in the title may not want to be there,
> though).
oops, it's remained from "working version" after rebasing
>
> Thanks.
>
> > t/t7102-reset.sh | 41 +
> > 1 file changed, 21 insertions(+), 20 deletions(-)
> >
> > diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> > index b096dc8..cf492f4 100755
> > --- a/t/t7102-reset.sh
> > +++ b/t/t7102-reset.sh
> > @@ -28,7 +28,8 @@ test_expect_success 'creating initial files and
> > commits' '
> > echo "1st line 2nd file" >secondfile &&
> > echo "2nd line 2nd file" >>secondfile &&
> > - git commit -a -m "modify 2nd file"
> > + git commit -a -m "modify 2nd file" &&
> > + head5=$(git rev-parse --verify HEAD)
> > '
> > # git log --pretty=oneline # to see those SHA1 involved
> >
> > @@ -56,7 +57,7 @@ test_expect_success 'giving a non existing
> > revision should fail' ' test_must_fail git reset --mixed aa &&
> > test_must_fail git reset --soft aa &&
> > test_must_fail git reset --hard aa &&
> > - check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > test_expect_success 'reset --soft with unmerged index should fail'
> > ' @@ -74,7 +75,7 @@ test_expect_success \
> > test_must_fail git reset --hard -- first &&
> > test_must_fail git reset --soft HEAD^ -- first &&
> > test_must_fail git reset --hard HEAD^ -- first &&
> > - check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > test_expect_success 'giving unrecognized options should fail' '
> > @@ -86,7 +87,7 @@ test_expect_success 'giving unrecognized options
> > should fail' ' test_must_fail git reset --soft -o &&
> > test_must_fail git reset --hard --other &&
> > test_must_fail git reset --hard -o &&
> > - check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > test_expect_success \
> > @@ -110,7 +111,7 @@ test_expect_success \
> >
> > git checkout master &&
> > git branch -D branch1 branch2 &&
> > - check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > test_expect_success \
> > @@ -133,27 +134,27 @@ test_expect_success \
> >
> > git checkout master &&
> > git branch -D branch3 branch4 &&
> > - check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > test_expect_success \
> > 'resetting to HEAD with no changes should succeed and do
> > nothing' ' git reset --hard &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset --hard HEAD &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset --soft &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset --soft HEAD &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset --mixed &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset --mixed HEAD &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > + check_changes $head5 &&
> > git reset HEAD &&
> > - check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > >.diff_expect
> > @@ -176,7 +177,7 @@ test_expect_success '--soft reset only should
> > show changes in diff --cached' ' git reset --soft HEAD^ &&
> > check_changes d1a4bc3abce4829628ae2dcb0d60ef3d1a78b1c4 &&
> > test "$(git rev-parse ORIG_HEAD)" = \
> > - 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + $head5
> > '
> >
> > >.diff_expect
> > @@ -193,7 +194,7 @@ test_expect_success \
> > git commit -a -C ORIG_HEAD &&
> > check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d &&
> > test "$(git rev-parse ORIG_HEAD)" = \
> > - 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + $head5
> > '
> >
> > >.diff_expect
> > @@ -303,7 +304,7 @@ test_expect_success 'redoing the last two
> > commits should succeed' ' echo "1st line 2nd file" >secondfile &&
> > echo "2nd line 2nd file" >>secondfile &&
> > git commit -a -m "modify 2nd file" &&
> > - check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > + check_changes $head5
> > '
> >
> > >.diff_expect
> >
Re: [PATCH v4 3/4] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
> Alexey Shumkin writes:
>
> > The following two commands are expected to give the same output to
> > a terminal:
> >
> > $ git log --oneline --no-color
> > $ git log --pretty=format:'%h %s'
> >
> > However, the former pays attention to i18n.logOutputEncoding
> > configuration, while the latter does not when it format "%s".
> > Log messages written in an encoding i18n.commitEncoding which
> > differs from terminal encoding are shown corrupted with the latter
> > even when i18n.logOutputEncoding and terminal encoding are the same.
> >
> > The same corruption is true for
> > $ git diff --submodule=log
> > and
> > $ git rev-list --pretty=format:%s HEAD
> > and
> > $ git reset --hard
> >
> > Signed-off-by: Alexey Shumkin
> > ---
> > t/t4041-diff-submodule-option.sh | 33 ---
> > t/t4205-log-pretty-formats.sh| 43 +++
> > t/t6006-rev-list-format.sh | 90
> > ++--
> > t/t7102-reset.sh | 32 +++--- 4 files
> > changed, 138 insertions(+), 60 deletions(-)
> >
> > diff --git a/t/t4041-diff-submodule-option.sh
> > b/t/t4041-diff-submodule-option.sh index 32d4a60..e7d6363 100755
> > --- a/t/t4041-diff-submodule-option.sh
> > +++ b/t/t4041-diff-submodule-option.sh
> > @@ -1,6 +1,7 @@
> > #!/bin/sh
> > #
> > # Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin
> > +# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding
> > tests) #
> >
> > test_description='Support for verbose submodule differences in git
> > diff @@ -10,6 +11,7 @@ This test tries to verify the sanity of the
> > --submodule option of git diff.
> > . ./test-lib.sh
> >
> > +added=$(printf
> > "\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275")
>
> Please have an in-code comment before this line to explain what this
> variable is about, e.g.
>
> # String "added" in Russian, encoded in UTF-8, used in
> # sample commit log messages in add_file() function below.
> added=$(printf "...")
Ok!
>
> > add_file () {
> > (
> > cd "$1" &&
> > @@ -19,7 +21,8 @@ add_file () {
> > echo "$name" >"$name" &&
> > git add "$name" &&
> > test_tick &&
> > - git commit -m "Add $name" || exit
> > + msg_added_cp1251=$(echo "Add $name ($added
> > $name)" | iconv -f utf-8 -t cp1251) &&
> > + git -c 'i18n.commitEncoding=cp1251' commit
> > -m "$msg_added_cp1251" done >/dev/null &&
> > git rev-parse --short --verify HEAD
> > )
>
> Does this patch make the all tests in this script fail for people
> without cp1251 locale installed? We already have tests that depend
> on 8859-1 and some other locales, and we'd be better off limiting
> such dependency to the minimum.
Hmmm, I'll try to feign something to avoid using cp1251
>
> Same comment applies to the changes to other test scripts.
>
> Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mergetools: Enhance tortoisemerge to work with
TortoiseGitMerge and filenames with spaces
- The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
(starting with 1.8.0) in order to make clear that this one has special
support for git, (uses spaces as cli parameter key-value separators)
and prevent confusion with the TortoiseSVN TortoiseMerge version.
- The tortoisemerge mergetool does not work with filenames which have
a space in it. Fixing this required changes in git and also in
TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.
Signed-off-by: Sven Strickroth
Reported-by: Sebastian Schuberth
---
mergetools/tortoisemerge | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..9890737 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -6,12 +6,28 @@ merge_cmd () {
if $base_present
then
touch "$BACKUP"
- "$merge_tool_path" \
- -base:"$BASE" -mine:"$LOCAL" \
- -theirs:"$REMOTE" -merged:"$MERGED"
+ if test "$merge_tool_path" == "tortoisegitmerge"
+ then
+ "$merge_tool_path" \
+ -base "$BASE" -mine "$LOCAL" \
+ -theirs "$REMOTE" -merged "$MERGED"
+ else
+ "$merge_tool_path" \
+ -base:"$BASE" -mine:"$LOCAL" \
+ -theirs:"$REMOTE" -merged:"$MERGED"
+ fi
check_unchanged
else
- echo "TortoiseMerge cannot be used without a base" 1>&2
+ echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
fi
}
+
+translate_merge_tool_path() {
+ if type tortoisegitmerge >/dev/null 2>/dev/null
+ then
+ echo tortoisegitmerge
+ else
+ echo tortoisemerge
+ fi
+}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
> Alexey Shumkin writes: > > > diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh > > index c248509..4db43a4 100755 > > --- a/t/t6006-rev-list-format.sh > > +++ b/t/t6006-rev-list-format.sh > > ... > > @@ -112,12 +133,12 @@ commit $head2 > > commit $head1 > > EOF > > > > -test_format raw-body %B <<'EOF' > > -commit 131a310eb913d107dd3c09a65d1651175898735d > > -changed foo > > +test_format failure raw-body %B < > +commit $head2 > > +$changed > > > > -commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873 > > -added foo > > +commit $head1 > > +$added > > > > EOF > > It may have been easier to follow if you did this "Don't hardcode" > as a separate preparatory patch, like your first two patches. Yep, I missed that in my bunch of rebasing ;) > > > @@ -135,16 +156,16 @@ commit $head1 > > foo > > EOF > > > > -cat >commit-msg <<'EOF' > > +iconv -f utf-8 -t cp1251 > commit-msg < > Test printing of complex bodies > > > > This commit message is much longer than the others, > > -and it will be encoded in iso8859-1. We should therefore > > -include an iso8859 character: ¡bueno! > > +and it will be encoded in cp1251. We should therefore > > +include an cp1251 character: так вот! > > EOF > > > > test_expect_success 'setup complex body' ' > > - git config i18n.commitencoding iso8859-1 && > > + git config i18n.commitencoding cp1251 && > > What is going on here? > > Is this an example that shows that i18n.commitencoding works > correctly with iso8859-1 but not with cp1251? It show only that I speak and write Russian not Spanish )) I'll revert back these changes. > > > diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh > > index cf492f4..699c824 100755 > > --- a/t/t7102-reset.sh > > +++ b/t/t7102-reset.sh > > ... > > @@ -192,7 +214,7 @@ test_expect_success \ > > 'changing files and redo the last commit should succeed' ' > > echo "3rd line 2nd file" >>secondfile && > > git commit -a -C ORIG_HEAD && > > - check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d && > > + check_changes f06f78b8dd468c722952b77569dd0db212442c25 && > > test "$(git rev-parse ORIG_HEAD)" = \ > > $head5 > > ' > > This and remaining hunks to this script shows that it would be > helped by the same love you gave to other scripts with your first > two patches before you add the "non-unicode" tests, no? Sorry, I haven't got you :-[ (it seems, my English is not good enough) Do you mean "avoid hardcoded SHA-1"? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: reject -D/-d without branch name
On Fri, Jan 25, 2013 at 3:45 PM, Matthieu Moy
wrote:
> Nguyễn Thái Ngọc Duy writes:
>
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 873f624..1d3e842 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char
>> *prefix)
>> colopts = 0;
>> }
>>
>> - if (delete)
>> + if (delete && argc)
>> return delete_branches(argc, argv, delete > 1, kinds, quiet);
>> else if (list) {
>> int ret = print_ref_list(kinds, detached, verbose, abbrev,
>
> Shouldn't this error out with a clean error message ("branch name
> expected" or so)?
Yeah. But on the other hand, this command seems to prefer to print
help usage when incorrect number of arguments is given (checkout
blocks "if (edit_description)" and "if (rename)" in cmd_branch).
Should those be fixed too?
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: reject -D/-d without branch name
Nguyễn Thái Ngọc Duy writes:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 873f624..1d3e842 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char
> *prefix)
> colopts = 0;
> }
>
> - if (delete)
> + if (delete && argc)
> return delete_branches(argc, argv, delete > 1, kinds, quiet);
> else if (list) {
> int ret = print_ref_list(kinds, detached, verbose, abbrev,
Shouldn't this error out with a clean error message ("branch name
expected" or so)?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mergetool--lib: fix startup options for gvimdiff tool
Maybe, some time ;)
Actually, I'm not TCL-programmer. With one of these patches I just have
solved one my problem (to run tortoisemerge with git-gui) when I
was showing to my collegue how to work with Git, and on the side I
fixed another two bugs. So, I decided to sumbit these patches, to avoid
applying them every time after each Git update as I did last 1.5 years
with other patches which still are not submitted, because I'm too lazy
to follow Git development workflow in my free time )
> On Wed, Jan 23, 2013 at 11:16 PM, Alexey Shumkin
> wrote:
> > Options are taken from /mergetools/vim
> >
> > Signed-off-by: Alexey Shumkin
> > ---
> > git-gui/lib/mergetool.tcl | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
>
> A better long-term solution might be to teach git gui to use "git
> difftool".
>
> Would it be better to teach git-gui (and gitk) about
> mergetool/difftool? That would allow us to possibly eliminate this
> duplication.
>
> We did start towards that path when difftool learned the --extcmd
> option (for use by gitk) but I have not followed through.
>
> What do you think about trying that approach?
>
>
> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> > index 3c8e73b..4fc1cab 100644
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -211,7 +211,13 @@ proc merge_resolve_tool2 {} {
> > }
> > }
> > gvimdiff {
> > - set cmdline [list "$merge_tool_path" -f "$LOCAL"
> > "$MERGED" "$REMOTE"]
> > + if {$base_stage ne {}} {
> > + set cmdline [list "$merge_tool_path" -f -d
> > -c "wincmd J" \
> > + "$MERGED" "$LOCAL" "$BASE"
> > "$REMOTE"]
> > + } else {
> > + set cmdline [list "$merge_tool_path" -f -d
> > -c "wincmd l" \
> > + "$LOCAL" "$MERGED" "$REMOTE"]
> > + }
> > }
> > kdiff3 {
> > if {$base_stage ne {}} {
> > --
> > 1.8.1.1.10.g9255f3f
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe git" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] branch: reject -D/-d without branch name
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/branch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..1d3e842 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
colopts = 0;
}
- if (delete)
+ if (delete && argc)
return delete_branches(argc, argv, delete > 1, kinds, quiet);
else if (list) {
int ret = print_ref_list(kinds, detached, verbose, abbrev,
--
1.8.1.1.380.g782aa97
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
