* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: John Keeping @ 2013-01-27 22:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <7v622iuzea.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 12:47:09PM -0800, Junio C Hamano wrote:
> I remember that I earlier asked somewhere if we want to say "Python
> 3.x that is older than 3.y is unsupported"
>
> http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926
>
> but I was told that we will support all versions in 3.x series, IIRC.
>
> Does this patch contradict with that? If so I think we would need
> to revisit the update to CodingGuidelines in that thread.
Yes. I'll send an update to that over the next couple of days.
I think 3.1 and later is fine, when I said "Python 3.0 is unsupported"
in the commit message below, I meant "unsupported by the Python
developers". Support ended at least 3 months ago:
http://hg.python.org/peps/rev/6d2e9d41dfaa
John
^ permalink raw reply
* Re: [PATCH 4/4] doc: Generate a list of valid merge tools
From: Junio C Hamano @ 2013-01-27 22:36 UTC (permalink / raw)
To: David Aguilar; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-5-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> +mergetools_txt = mergetools-diff.txt mergetools-merge.txt
> +
> +$(mergetools_txt): mergetools-list.made
> +
> +mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
> + $(QUIET_GEN)$(RM) $@ && \
> + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
> + . ../git-mergetool--lib.sh && \
> + filter_tools can_diff "* "' > mergetools-diff.txt && \
> + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
> + . ../git-mergetool--lib.sh && \
> + filter_tools can_merge "* "' > mergetools-merge.txt && \
> + date > $@
Nicely done.
By omitting is_available check and only checking can_*, we would get
the same result on all platforms and I'd see tortoise appear in the
output even I do not do Windows.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
From: Junio C Hamano @ 2013-01-27 22:32 UTC (permalink / raw)
To: David Aguilar; +Cc: Johannes Sixt, git, John Keeping
In-Reply-To: <CAJDDKr61wS1ruPYDW4+exWBWveyS0Dp7Gmu5gWRh_99-frc_7Q@mail.gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Definitely. I learned this the hard way when the tests broke on me while
> working it ;-) My patch rewrites things to always use var=$(command)
> expressions with separate test "$var" evaluating them.
OK; that wasn't clear from the log message.
^ permalink raw reply
* Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
From: Junio C Hamano @ 2013-01-27 22:31 UTC (permalink / raw)
To: David Aguilar; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-4-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Refactor show_tool_help() so that the tool-finding logic is broken out
> into separate functions.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> git-mergetool--lib.sh | 60 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index cf52423..894b849 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -2,6 +2,33 @@
> # git-mergetool--lib is a library for common merge tool functions
> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>
> +mode_ok () {
> + diff_mode && can_diff ||
> + merge_mode && can_merge
> +}
I think you got the operator precedence mixed up.
What happens when diff_mode=true, can_diff=true, merge_mode=true and
can_merge=false?
if true && true || true && false
then
echo OK
else
echo NO
fi
if (true && true) || (true && false)
then
echo OK
else
echo NO
fi
> +is_available () {
> + merge_tool_path=$(translate_merge_tool_path "$1") &&
> + type "$merge_tool_path" >/dev/null 2>&1
> +}
> +
> +filter_tools () {
> + filter="$1"
> + prefix="$2"
> + (
> + cd "$MERGE_TOOLS_DIR" &&
> + for i in *
> + do
> + echo "$i"
> + done
> + ) | sort | while read tool
> + do
Please start a new line before keywords that define the syntactic
structure, like "while", i.e.
... piped | commands |
while read tool
do
...
^ permalink raw reply
* Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
From: Johannes Sixt @ 2013-01-27 22:26 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <1359321886-80523-4-git-send-email-davvid@gmail.com>
Am 27.01.2013 22:24, schrieb David Aguilar:
> Refactor show_tool_help() so that the tool-finding logic is broken out
> into separate functions.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> git-mergetool--lib.sh | 60 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index cf52423..894b849 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -2,6 +2,33 @@
> # git-mergetool--lib is a library for common merge tool functions
> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>
> +mode_ok () {
> + diff_mode && can_diff ||
> + merge_mode && can_merge
> +}
&& and || have the same precedence: if diff_mode and can_diff both are
"true", then the result of the function is that of can_merge. I don't
think that is what is intended.
> +filter_tools () {
> + filter="$1"
> + prefix="$2"
> + (
> + cd "$MERGE_TOOLS_DIR" &&
> + for i in *
> + do
> + echo "$i"
> + done
cd "$MERGE_TOOLS_DIR" &&
printf "%s\n" *
But what's wrong with "ls -1"? It would save the explicit sort.
> + ) | sort | while read tool
> + do
> + setup_tool "$tool" 2>/dev/null &&
> + (eval "$filter" "$tool") &&
> + printf "$prefix$tool\n"
> + done
> +}
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
From: Junio C Hamano @ 2013-01-27 22:21 UTC (permalink / raw)
To: David Aguilar; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-2-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Use $(command "$arg") instead of "$(command "$arg")" as the latter is
> harder to read.
Did you miss my comment that this is about RHS of an assignment?
^ permalink raw reply
* Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
From: David Aguilar @ 2013-01-27 22:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <5105A56E.4010002@kdbg.org>
On Sun, Jan 27, 2013 at 2:08 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 27.01.2013 22:24, schrieb David Aguilar:
>> Use $(command "$arg") instead of "$(command "$arg")" as the latter is
>> harder to read.
>
> If at all, you should restrict yourself to simplify only variable
> assignments. Because this case:
>
>> - if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
>> + if test -z $(get_merge_tool_cmd "$merge_tool") &&
>
> cannot work as intended: If the output of $() is empty, then without the
> outer quotes this becomes
>
> test -z
>
> without an operand for -z, which is a syntax error (of the test command).
Definitely. I learned this the hard way when the tests broke on me while
working it ;-) My patch rewrites things to always use var=$(command)
expressions with separate test "$var" evaluating them.
Thanks for the tip,
--
David
^ permalink raw reply
* Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
From: Johannes Sixt @ 2013-01-27 22:08 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <1359321886-80523-2-git-send-email-davvid@gmail.com>
Am 27.01.2013 22:24, schrieb David Aguilar:
> Use $(command "$arg") instead of "$(command "$arg")" as the latter is
> harder to read.
If at all, you should restrict yourself to simplify only variable
assignments. Because this case:
> - if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
> + if test -z $(get_merge_tool_cmd "$merge_tool") &&
cannot work as intended: If the output of $() is empty, then without the
outer quotes this becomes
test -z
without an operand for -z, which is a syntax error (of the test command).
-- Hannes
^ permalink raw reply
* [PATCH 3/4] mergetool--lib: Add functions for finding available tools
From: David Aguilar @ 2013-01-27 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-1-git-send-email-davvid@gmail.com>
Refactor show_tool_help() so that the tool-finding logic is broken out
into separate functions.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool--lib.sh | 60 +++++++++++++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index cf52423..894b849 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,33 @@
# git-mergetool--lib is a library for common merge tool functions
MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+mode_ok () {
+ diff_mode && can_diff ||
+ merge_mode && can_merge
+}
+
+is_available () {
+ merge_tool_path=$(translate_merge_tool_path "$1") &&
+ type "$merge_tool_path" >/dev/null 2>&1
+}
+
+filter_tools () {
+ filter="$1"
+ prefix="$2"
+ (
+ cd "$MERGE_TOOLS_DIR" &&
+ for i in *
+ do
+ echo "$i"
+ done
+ ) | sort | while read tool
+ do
+ setup_tool "$tool" 2>/dev/null &&
+ (eval "$filter" "$tool") &&
+ printf "$prefix$tool\n"
+ done
+}
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -199,27 +226,13 @@ list_merge_tool_candidates () {
}
show_tool_help () {
- unavailable= available= LF='
-'
- for i in "$MERGE_TOOLS_DIR"/*
- do
- tool=$(basename "$i")
- setup_tool "$tool" 2>/dev/null || continue
-
- merge_tool_path=$(translate_merge_tool_path "$tool")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- available="$available$tool$LF"
- else
- unavailable="$unavailable$tool$LF"
- fi
- done
-
cmd_name=${TOOL_MODE}tool
+ available=$(filter_tools 'mode_ok && is_available' '\t\t')
+ unavailable=$(filter_tools 'mode_ok && ! is_available' '\t\t')
if test -n "$available"
then
echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
- echo "$available" | sort | sed -e 's/^/ /'
+ printf "$available"
else
echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
fi
@@ -227,7 +240,7 @@ show_tool_help () {
then
echo
echo 'The following tools are valid, but not currently available:'
- echo "$unavailable" | sort | sed -e 's/^/ /'
+ printf "$unavailable"
fi
if test -n "$unavailable$available"
then
@@ -250,17 +263,12 @@ $tools
printf "$msg" >&2
# Loop over each candidate and stop when a valid merge tool is found.
- for i in $tools
+ for tool in $tools
do
- merge_tool_path=$(translate_merge_tool_path "$i")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- echo "$i"
- return 0
- fi
+ is_available "$tool" && echo "$tool" && return 0
done
- echo >&2 "No known merge resolution program available."
+ echo >&2 "No known ${TOOL_MODE} tool is available."
return 1
}
--
1.8.0.13.gf25ae33
^ permalink raw reply related
* [PATCH 4/4] doc: Generate a list of valid merge tools
From: David Aguilar @ 2013-01-27 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-1-git-send-email-davvid@gmail.com>
Use the new filter_tools() function to build lists of all
the built-in tools supported by difftool and mergetool.
This frees us from needing to update the documentation
whenever a new tool is added.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Documentation/.gitignore | 1 +
Documentation/Makefile | 16 +++++++++++++++-
Documentation/diff-config.txt | 13 +++++++------
Documentation/merge-config.txt | 12 ++++++------
git-mergetool--lib.sh | 1 +
5 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index d62aebd..2c8b2d6 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -9,4 +9,5 @@ gitman.info
howto-index.txt
doc.dep
cmds-*.txt
+mergetools-*.txt
manpage-base-url.xsl
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 267dfe1..f595d26 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -226,13 +226,27 @@ cmd-list.made: cmd-list.perl ../command-list.txt $(MAN1_TXT)
$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(QUIET_STDERR) && \
date >$@
+mergetools_txt = mergetools-diff.txt mergetools-merge.txt
+
+$(mergetools_txt): mergetools-list.made
+
+mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
+ $(QUIET_GEN)$(RM) $@ && \
+ $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+ . ../git-mergetool--lib.sh && \
+ filter_tools can_diff "* "' > mergetools-diff.txt && \
+ $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+ . ../git-mergetool--lib.sh && \
+ filter_tools can_merge "* "' > mergetools-merge.txt && \
+ date > $@
+
clean:
$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
$(RM) *.pdf
$(RM) howto-index.txt howto/*.html doc.dep
$(RM) technical/api-*.html technical/api-index.txt
- $(RM) $(cmds_txt) *.made
+ $(RM) $(cmds_txt) $(mergetools_txt) *.made
$(RM) manpage-base-url.xsl
$(MAN_HTML): %.html : %.txt
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 67a90a8..7c968d1 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -132,9 +132,10 @@ diff.<driver>.cachetextconv::
conversion outputs. See linkgit:gitattributes[5] for details.
diff.tool::
- The diff tool to be used by linkgit:git-difftool[1]. This
- option overrides `merge.tool`, and has the same valid built-in
- values as `merge.tool` minus "tortoisemerge" and plus
- "kompare". Any other value is treated as a custom diff tool,
- and there must be a corresponding `difftool.<tool>.cmd`
- option.
+ Controls which diff tool is used by linkgit:git-difftool[1].
+ This variable overrides the value configured in `merge.tool`.
+ The list below shows the valid built-in values.
+ Any other value is treated as a custom diff tool and requires
+ that a corresponding difftool.<tool>.cmd variable is defined.
+
+include::mergetools-diff.txt[]
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 861bd6f..5f40e71 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -52,12 +52,12 @@ merge.stat::
at the end of the merge. True by default.
merge.tool::
- Controls which merge resolution program is used by
- linkgit:git-mergetool[1]. Valid built-in values are: "araxis",
- "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld",
- "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff"
- and "xxdiff". Any other value is treated is custom merge tool
- and there must be a corresponding mergetool.<tool>.cmd option.
+ Controls which merge tool is used by linkgit:git-mergetool[1].
+ The list below shows the valid built-in values.
+ Any other value is treated as a custom merge tool and requires
+ that a corresponding mergetool.<tool>.cmd variable is defined.
+
+include::mergetools-merge.txt[]
merge.verbosity::
Controls the amount of output shown by the recursive merge
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 894b849..59e1650 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,6 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
+test -z "$MERGE_TOOLS_DIR" &&
MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
mode_ok () {
--
1.8.0.13.gf25ae33
^ permalink raw reply related
* [PATCH 2/4] mergetool--lib: Improve the help text in guess_merge_tool()
From: David Aguilar @ 2013-01-27 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-1-git-send-email-davvid@gmail.com>
This code path is only activated when the user does not have a valid
configured tool. Add a message to guide new users towards configuring a
default tool.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool--lib.sh | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a5aae9..cf52423 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -240,7 +240,14 @@ show_tool_help () {
guess_merge_tool () {
list_merge_tool_candidates
- echo >&2 "merge tool candidates: $tools"
+ msg="\
+
+This message is displayed because '$TOOL_MODE.tool' is not configured.
+See 'git ${TOOL_MODE}tool --tool-help' or 'git help config' for more details.
+'git ${TOOL_MODE}tool' will now attempt to use one of the following tools:
+$tools
+"
+ printf "$msg" >&2
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
--
1.8.0.13.gf25ae33
^ permalink raw reply related
* [PATCH 1/4] mergetool--lib: Simplify command expressions
From: David Aguilar @ 2013-01-27 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-1-git-send-email-davvid@gmail.com>
Use $(command "$arg") instead of "$(command "$arg")" as the latter is
harder to read. Make the expression in get_merge_tool_cmd() even
simpler by avoiding "echo" completely.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool--lib.sh | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1d0fb12..9a5aae9 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -32,17 +32,10 @@ check_unchanged () {
fi
}
-valid_tool_config () {
- if test -n "$(get_merge_tool_cmd "$1")"
- then
- return 0
- else
- return 1
- fi
-}
-
valid_tool () {
- setup_tool "$1" || valid_tool_config "$1"
+ setup_tool "$1" && return 0
+ cmd=$(get_merge_tool_cmd "$1")
+ test -n "$cmd"
}
setup_tool () {
@@ -96,14 +89,13 @@ setup_tool () {
}
get_merge_tool_cmd () {
- # Prints the custom command for a merge tool
merge_tool="$1"
if diff_mode
then
- echo "$(git config difftool.$merge_tool.cmd ||
- git config mergetool.$merge_tool.cmd)"
+ git config "difftool.$merge_tool.cmd" ||
+ git config "mergetool.$merge_tool.cmd"
else
- echo "$(git config mergetool.$merge_tool.cmd)"
+ git config "mergetool.$merge_tool.cmd"
fi
}
@@ -114,7 +106,7 @@ run_merge_tool () {
GIT_PREFIX=${GIT_PREFIX:-.}
export GIT_PREFIX
- merge_tool_path="$(get_merge_tool_path "$1")" || exit
+ merge_tool_path=$(get_merge_tool_path "$1") || exit
base_present="$2"
status=0
@@ -145,7 +137,7 @@ run_merge_tool () {
# Run a either a configured or built-in diff tool
run_diff_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
( eval $merge_tool_cmd )
@@ -158,11 +150,11 @@ run_diff_cmd () {
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
- trust_exit_code="$(git config --bool \
- mergetool."$1".trustExitCode || echo false)"
+ trust_exit_code=$(git config --bool \
+ "mergetool.$1.trustExitCode" || echo false)
if test "$trust_exit_code" = "false"
then
touch "$BACKUP"
@@ -253,7 +245,7 @@ guess_merge_tool () {
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
- merge_tool_path="$(translate_merge_tool_path "$i")"
+ merge_tool_path=$(translate_merge_tool_path "$i")
if type "$merge_tool_path" >/dev/null 2>&1
then
echo "$i"
@@ -300,9 +292,9 @@ get_merge_tool_path () {
fi
if test -z "$merge_tool_path"
then
- merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
+ merge_tool_path=$(translate_merge_tool_path "$merge_tool")
fi
- if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+ if test -z $(get_merge_tool_cmd "$merge_tool") &&
! type "$merge_tool_path" >/dev/null 2>&1
then
echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
@@ -314,11 +306,11 @@ get_merge_tool_path () {
get_merge_tool () {
# Check if a merge tool has been configured
- merge_tool="$(get_configured_merge_tool)"
+ merge_tool=$(get_configured_merge_tool)
# Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
- merge_tool="$(guess_merge_tool)" || exit
+ merge_tool=$(guess_merge_tool) || exit
fi
echo "$merge_tool"
}
--
1.8.0.13.gf25ae33
^ permalink raw reply related
* [PATCH 0/4] Documentation: Auto-generate merge tool lists
From: David Aguilar @ 2013-01-27 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
Refactor the mergetool-lib so that we can reuse it in
Documentation/Makefile. The end result is that the
diff.tool and merge.tool documentation now includes
an auto-generated list of all available tools.
This applies on top of jk/mergetool in pu.
David Aguilar (4):
mergetool--lib: Simplify command expressions
mergetool--lib: Improve the help text in guess_merge_tool()
mergetool--lib: Add functions for finding available tools
doc: Generate a list of valid merge tools
Documentation/.gitignore | 1 +
Documentation/Makefile | 16 +++++-
Documentation/diff-config.txt | 13 ++---
Documentation/merge-config.txt | 12 ++---
git-mergetool--lib.sh | 108 ++++++++++++++++++++++-------------------
5 files changed, 87 insertions(+), 63 deletions(-)
--
1.8.0.13.gf25ae33
^ permalink raw reply
* Re: mergetool: include custom tools in '--tool-help'
From: David Aguilar @ 2013-01-27 21:10 UTC (permalink / raw)
To: John Keeping; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <7vmwvuv0ya.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 12:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> John Keeping <john@keeping.me.uk> writes:
>
>> I think I'd want to do this with a suffix if at all, so the output would
>> be like this:
>>
>> 'git mergetool --tool=<tool>' may be set to one of the following:
>>
>> araxis
>> gvimdiff
>> gvimdiff2
>> mytool (user-defined)
>> vimdiff
>> vimdiff2
>
> That is fine by me, but the real users of mergetool please feel free
> to raise objections.
This seems pretty useful.
I did a bit of refactoring last night that I'd like to post here,
the end result being something that's plugged into Documentation/.
I think what I did may also help add this functionality, and
could be useful to build upon.
I'll send my patches shortly so you can take a look.
Basically, I added a simple way to loop over the tools
and filter them. This is reused in show_tool_help() and
Documentation/Makefile.
The refactoring changes how show_tool_help() works,
so I'd like you to take a look before we add a new feature
since it might make it easier to do.
--
David
^ permalink raw reply
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: Junio C Hamano @ 2013-01-27 20:47 UTC (permalink / raw)
To: John Keeping; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <7va9ruuzsf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> John Keeping <john@keeping.me.uk> writes:
>
>> So I think the answer is "habit, but I probably shouldn't have put it
>> in in this case".
>
> OK, then I'll queue with a local amend to drop the leading
> underscore.
So this is what I will be queuing (I'd appreciate the second set of
eyes, though), with the leading-underscore removal and log message
typofixes.
I remember that I earlier asked somewhere if we want to say "Python
3.x that is older than 3.y is unsupported"
http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926
but I was told that we will support all versions in 3.x series, IIRC.
Does this patch contradict with that? If so I think we would need
to revisit the update to CodingGuidelines in that thread.
I am perfectly fine with discarding early 3.x as "0.x releases of
Python3", but I would want to see our document say so if that is
what we do.
-- >8 --
From: John Keeping <john@keeping.me.uk>
Date: Sun, 27 Jan 2013 14:50:56 +0000
Subject: [PATCH] git-remote-testpy: fix path hashing on Python 3
When this change was originally made (0846b0c - git-remote-testpy:
hash bytes explicitly , I didn't realise that the "hex" encoding we
chose is a "bytes to bytes" encoding so it just fails with an error
on Python 3 in the same way as the original code.
It is not possible to provide a single code path that works on
Python 2 and Python 3 since Python 2.x will attempt to decode the
string before encoding it, which fails for strings that are not
valid in the default encoding. Python 3.1 introduced the
"surrogateescape" error handler which handles this correctly and
permits a bytes -> unicode -> bytes round-trip to be lossless.
At this point Python 3.0 is unsupported so we don't go out of our
way to try to support it.
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-remote-testpy.py | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..6098bdd 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
sys.exit(1)
+
+def encode_filepath(path):
+ """Encodes a Unicode file path to a byte string.
+
+ On Python 2 this is a no-op; on Python 3 we encode the string as
+ suggested by [1] which allows an exact round-trip from the command line
+ to the filesystem.
+
+ [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
+
+ """
+ if sys.hexversion < 0x03000000:
+ return path
+ return path.encode('utf-8', 'surrogateescape')
+
+
def get_repo(alias, url):
"""Returns a git repository object initialized for usage.
"""
@@ -45,7 +61,7 @@ def get_repo(alias, url):
repo.get_head()
hasher = _digest()
- hasher.update(repo.path.encode('hex'))
+ hasher.update(encode_filepath(repo.path))
repo.hash = hasher.hexdigest()
repo.get_base_path = lambda base: os.path.join(
--
1.8.1.1.550.g40037fd
^ permalink raw reply related
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: Junio C Hamano @ 2013-01-27 20:38 UTC (permalink / raw)
To: John Keeping; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <20130127202106.GU7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> On Sun, Jan 27, 2013 at 12:11:20PM -0800, Junio C Hamano wrote:
>> John Keeping <john@keeping.me.uk> writes:
>>
>> >> Thanks; will queue and wait for an Ack from Michael.
>> >>
>> >> Does the helper function need to be named with leading underscore,
>> >> though?
>> >
>> > ... Since this is a script
>> > not a library module I don't feel strongly about it in this case.
>>
>> That is exactly why I asked.
>
> So I think the answer is "habit, but I probably shouldn't have put it
> in in this case".
OK, then I'll queue with a local amend to drop the leading
underscore.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] add: warn when -u or -A is used without filepattern
From: Junio C Hamano @ 2013-01-27 20:33 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, Jonathan Nieder, Robin Rosenberg, Piotr Krukowiecki,
Eric James Michael Ritz, Tomas Carnecky
In-Reply-To: <vpqtxq28v3s.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Plus, option_with_implicit_dot is used in cut-and-paste ready commands
> below.
I do not think we should aim for easy cut-and-paste, especially when
the real purpose of the change is to train people's fingers; the
message should discouraging cut-and-paste in a case like this, if
anything.
But we could obviously do this, if you really want to cut-and-paste.
builtin/add.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 7552f7f..ba72a57 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -363,7 +363,7 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
-static void warn_pathless_add(const char *option_name) {
+static void warn_pathless_add(const char *option_name, const char *short_name) {
/*
* To be consistent with "git add -p" and most Git
* commands, we should default to being tree-wide, but
@@ -374,20 +374,21 @@ static void warn_pathless_add(const char *option_name) {
* 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"
+ warning(_("The behavior of 'git add %s (or %s)' with no path argument from a\n"
+ "subdirectory of the tree will change in Git 2.0 and should not be\n"
+ "used anymore.\n"
"To add content for the whole tree, run:\n"
"\n"
- " git add %s :/\n"
+ " git add %s :/ ;# or git add %s :/\n"
"\n"
"To restrict the command to the current directory, run:\n"
"\n"
- " git add %s .\n"
+ " git add %s . ;# or git add %s .\n"
"\n"
"With the current Git version, the command is restricted to the current directory."),
- option_name,
- option_name,
- option_name);
+ option_name, short_name,
+ option_name, short_name,
+ option_name, short_name);
}
int cmd_add(int argc, const char **argv, const char *prefix)
@@ -401,6 +402,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int require_pathspec;
char *seen = NULL;
const char *option_with_implicit_dot = NULL;
+ const char *short_option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -420,14 +422,19 @@ 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)
+ if (addremove) {
option_with_implicit_dot = "--all";
- if (take_worktree_changes)
+ short_option_with_implicit_dot = "-A";
+ }
+ if (take_worktree_changes) {
option_with_implicit_dot = "--update";
+ short_option_with_implicit_dot = "-u";
+ }
if (option_with_implicit_dot && !argc) {
static const char *here[2] = { ".", NULL };
if (prefix)
- warn_pathless_add(option_with_implicit_dot);
+ warn_pathless_add(option_with_implicit_dot,
+ short_option_with_implicit_dot);
argc = 1;
argv = here;
}
^ permalink raw reply related
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-27 20:25 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jonathan Nieder, git, kraai
In-Reply-To: <7v4ni2y1fm.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> If we did not care about incurring runtime performance cost, we
> could arrange:
> ...
> Then you can wrap commands whose use we want to limit, perhaps like
> this, in the test framework:
> ...
> sed () {
> ...
> done
> if test -z "$must_abort"
> sed "$@"
> fi
> }
Of course, aside from missing "then", this needs to use the
real "sed", so this has to be
if test -z "$must_abort"
then
command sed "$@"
fi
or something like that.
An approach along this line may reduce both the false negatives and
false positives down to an acceptable level, but I doubt the result
would be efficient enough for us to tolerate the runtime penalty.
^ permalink raw reply
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: John Keeping @ 2013-01-27 20:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <7vr4l6v11z.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 12:11:20PM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> >> Thanks; will queue and wait for an Ack from Michael.
> >>
> >> Does the helper function need to be named with leading underscore,
> >> though?
> >
> > ... Since this is a script
> > not a library module I don't feel strongly about it in this case.
>
> That is exactly why I asked.
So I think the answer is "habit, but I probably shouldn't have put it
in in this case".
John
^ permalink raw reply
* Re: mergetool: include custom tools in '--tool-help'
From: Junio C Hamano @ 2013-01-27 20:13 UTC (permalink / raw)
To: John Keeping; +Cc: git, David Aguilar
In-Reply-To: <20130127195618.GS7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> I think I'd want to do this with a suffix if at all, so the output would
> be like this:
>
> 'git mergetool --tool=<tool>' may be set to one of the following:
>
> araxis
> gvimdiff
> gvimdiff2
> mytool (user-defined)
> vimdiff
> vimdiff2
That is fine by me, but the real users of mergetool please feel free
to raise objections.
^ permalink raw reply
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: Junio C Hamano @ 2013-01-27 20:11 UTC (permalink / raw)
To: John Keeping; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <20130127200401.GT7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
>> Thanks; will queue and wait for an Ack from Michael.
>>
>> Does the helper function need to be named with leading underscore,
>> though?
>
> ... Since this is a script
> not a library module I don't feel strongly about it in this case.
That is exactly why I asked.
^ permalink raw reply
* Re: [PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory
From: Junio C Hamano @ 2013-01-27 20:09 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jeff King, git
In-Reply-To: <20130127102753.GB4228@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Jeff King wrote:
>
>> When we look up a sha1 object for reading, we first check
>> packfiles, and then loose objects. If we still haven't found
>> it, we re-scan the list of packfiles in `objects/pack`. This
>> final step ensures that we can co-exist with a simultaneous
>> repack process which creates a new pack and then prunes the
>> old object.
>
> I like the context above and what follows it, but I think you forgot
> to mention what the patch actually does. :)
>
> I guess it is:
>
> However, in the first scan over refs in fetch-pack.c::everything_local,
> this double-check of packfiles is not necessary since we are only
> trying to get a rough estimate of the last time we fetched from this
> remote repository in order to find good candidate common commits ---
> a missed object would only result in a slightly slower fetch.
It is not about a rough estimate nor common commits, though. The
"everything local" check in question is interested in only one
thing: are we _clearly_ up to date without fetching anything from
them?
Loosening the check may miss the rare case where we race against a
simultaneous repack and will cause us to go to the network when we
do not have to, and it becomes a trade off between the common unracy
case going faster by allowing the "Are we clearly up to date" check
to cheat, at the expense of rare racy cases suffering unnecessary
object transfer overhead.
> Avoid that slow second scan in the common case by guarding the object
> lookup with has_sha1_file().
This conclusion is correct.
> I had not read this codepath before. I'm left with a few questions:
>
> * Why is 49bb805e ("Do not ask for objects known to be complete",
> 2005-10-19) trying to do? Are we hurting that in any way?
An earlier fetch may have acquired all the necessary objects but may
not have updated our refs for some reason (e.g. fast-forward check
may have fired). In such a case, we may already have a history that
is good (i.e. not missing paths down to the common history) in our
repository that is not connected to any of our refs, and we can
update our refs (or write to FETCH_HEAD) without asking the remote
end to do any common ancestor computation or object transfer.
That was the primary thing the patch wanted to do.
As a side-effect, we know more objects than just the objects at the
tips of our refs are complete and that may help the later common
history discovery step, but obviously we do not want to dig the
history down to root. The cutoff value is merely a heuristics
chosen without any deep thought.
> * Is has_sha1_file() generally succeptible to the race against repack
> you mentioned? How is that normally dealt with?
By failing to find, so that the user will restart. When the caller
really wants to use the object, parse_objects() => read_sha1_file()
=> read_object() is used and we will see the retry.
> * Can a slow operation get confused if an object is incorporated into
> a pack and then expelled again by two repacks in sequence?
If it checks "the object should be there" first, wait for a long
time, and then tries to find that object's data, the later access
will go to the parse_objects() callpath and I think it should do the
right thing. If that slow opearation stops inside read_object(), it
could find it unable to map the loose object file and then unable to
find it in the pack, either. Is that what you are worried about?
^ permalink raw reply
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: John Keeping @ 2013-01-27 20:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <7vzjzuv224.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 11:49:39AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > When this change was originally made (0846b0c - git-remote-testpy: hash
> > bytes explicitly , I didn't realised that the "hex" encoding we chose is
> > a "bytes to bytes" encoding so it just fails with an error on Python 3
> > in the same way as the original code.
> >
> > It is not possible to provide a single code path that works on Python 2
> > and Python 3 since Python 2.x will attempt to decode the string before
> > encoding it, which fails for strings that are not valid in the default
> > encoding. Python 3.1 introduced the "surrogateescape" error handler
> > which handles this correctly and permits a bytes -> unicode -> bytes
> > round-trip to be lossless.
> >
> > At this point Python 3.0 is unsupported so we don't go out of our way to
> > try to support it.
> >
> > Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> > Signed-off-by: John Keeping <john@keeping.me.uk>
> > ---
>
> Thanks; will queue and wait for an Ack from Michael.
>
> Does the helper function need to be named with leading underscore,
> though?
It's a Python convention for internal functions. Since this is a script
not a library module I don't feel strongly about it in this case.
John
^ permalink raw reply
* Re: mergetool: include custom tools in '--tool-help'
From: John Keeping @ 2013-01-27 19:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Aguilar
In-Reply-To: <7vobgawljs.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 10:03:19AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > 'git mergetool --tool-help' only lists builtin tools, not those that the
> > user has configured via a 'mergetool.<tool>.cmd' config value. Fix this
> > by inspecting the tools configured in this way and adding them to the
> > available and unavailable lists before displaying them.
>
> Although I am not a mergetool user, I would imagine that it would
> make sense to show it as available.
>
> Just like "git help -a" lists subcommands in a way that can be easy
> to tell which ones are the standard ones and which ones are user
> customizations, this may want to give a similar distinction, though.
> I dunno.
I think I'd want to do this with a suffix if at all, so the output would
be like this:
'git mergetool --tool=<tool>' may be set to one of the following:
araxis
gvimdiff
gvimdiff2
mytool (user-defined)
vimdiff
vimdiff2
The following tools are valid, but not currently available:
bc3
codecompare
deltawalker
diffuse
ecmerge
emerge
kdiff3
meld
opendiff
p4merge
tkdiff
tortoisemerge
xxdiff
Some of the tools listed above only work in a windowed
environment. If run in a terminal-only session, they will fail.
Adding more sections for the user-defined tools feels like the output
would be too imposing and would make it hard to immediately identify the
valid option.
John
^ permalink raw reply
* Re: Behavior of stash apply vs merge
From: Robin Rosenberg @ 2013-01-27 19:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vfw1mwibu.fsf@alter.siamese.dyndns.org>
----- Ursprungligt meddelande -----
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>
> > Thanks. Feeling a bit studid now.
> >
> > I was actually thinking about using merge to implement stash apply
> > in JGit. What we have is broken so I tried using merge to implement
> > it and them compared to git merge --no-commit.. FAIL.
>
> Do you have "cherry-pick"?
>
> In short, "stash apply" is a "cherry-pick" in disguise.
Yes, that's what I did. Thanks for confirming this. One for the working
tree and if that succeeds I do another one to restore the index if requested.
-- robin
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox