* [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
* [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 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 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 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
* 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
* 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: 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 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 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 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 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] 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: John Keeping @ 2013-01-27 22:54 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <1359321886-80523-5-git-send-email-davvid@gmail.com>
On Sun, Jan 27, 2013 at 01:24:46PM -0800, David Aguilar wrote:
> --- 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
The preferred pattern in Git seems to be this:
: ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools}
John
^ permalink raw reply
* Re: [feature request] git add completion should exclude staged content
From: Junio C Hamano @ 2013-01-27 23:00 UTC (permalink / raw)
To: wookietreiber; +Cc: git, Manlio Perillo
In-Reply-To: <20130126172137.GB5852@mobster.11n_AP_Router>
wookietreiber <kizkizzbangbang@googlemail.com> writes:
> I have a feature request for `git add` auto completion:
>
> `git add` auto completion suggests all files / directories,
> filtered by nothing. I guess it would be much nicer (as in
> increasing productivity) if it would only suggest unstaged
> content, as reported by `git status`, because that would be the
> only content one would be able to add.
I think that is what Manlio Perillo tried to do with the stalled
mp/complete-paths topic that is queued in 'pu'.
Manlio, any progress?
^ permalink raw reply
* Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
From: John Keeping @ 2013-01-27 23:04 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <1359321886-80523-4-git-send-email-davvid@gmail.com>
On Sun, Jan 27, 2013 at 01:24:45PM -0800, David Aguilar wrote:
> +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
> +}
Can we change this so that it does this:
filter_tools () {
filter="$1"
(
cd "$MERGE_TOOLS_DIR" &&
for i in *
do
echo "$i"
done
) |
while read tool
do
setup_tool "$tool" 2>/dev/null &&
(eval "$filter" "$tool") &&
echo "$tool"
done
}
and keep the sorting and prefix in show_tool_help? This will make it
easier to integrate the user-configured tools from git-config.
John
^ permalink raw reply
* Re: [PATCH 2/4] mergetool--lib: Improve the help text in guess_merge_tool()
From: Junio C Hamano @ 2013-01-27 23:12 UTC (permalink / raw)
To: David Aguilar; +Cc: git, John Keeping
In-Reply-To: <1359321886-80523-3-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> 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
This is not wrong per-se, but wouldn't it be much easier to read to
use an indented HERE-text like this?
cat >&2 <<-EOF
... a long message with $var substitution ...
... comes here ...
EOF
^ permalink raw reply
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: Junio C Hamano @ 2013-01-27 23:18 UTC (permalink / raw)
To: John Keeping; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <20130127224208.GV7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> 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".
Yeah, I knew what you meant. I do not think it is so wrong to write
3.0 off as an early 0.x release of Python3 that was not yet usable
for that exact reason.
^ permalink raw reply
* Re: [PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory
From: Jonathan Nieder @ 2013-01-27 23:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vvcaiv14s.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> 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?
[...]
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> * 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.
Interesting.
After I fetch objects for branches a, b, and c which all have different
commit times in a fetch that fails due to the fast-forward check, I
have the following objects:
a commit date = 25 January 2013
b commit date = 26 January 2013
c commit date = 27 January 2013
When I try to fetch again (forcibly this time), git notices that the
objects are available locally and sets "cutoff" to 27 January 2013 as
a hint about the last time I fetched.
mark_recent_complete_commits() is called, and since these objects are
not reachable from any local refs, none are visited in the walk, and
49bb805e does not affect the outcome of the fetch positively or
negatively.
On the other hand, if I fetched a, b, and c to local branches and then
built on top of them, 49bb805e ensures that a second fetch of the same
refs will know right away not to request objects for c. So it brings
some of the benefits of 2759cbc7 (git-fetch-pack: avoid unnecessary
zero packing, 2005-10-18) when the receiving side has either (A)
renamed refs or (B) built new history on top of them.
Correct?
It is only in case (B) that the cutoff matters. If we miscalculate
the cutoff, that means either (i) cutoff == 0, and we would lose the
benefit of the walk that finds complete commits, or (ii) cutoff is a
little earlier, and the walk to find complete commits would take a
little longer. Neither effects the result of the fetch, and neither
is likely to be a significant enough performance difference to offset
the benefit of Jeff's patch.
Thanks for explaining.
Jonathan
^ permalink raw reply
* Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
From: Junio C Hamano @ 2013-01-27 23:32 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:
> +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"
This may happen to be safe as available will not have anything with
a per-cent % in it, but is not a good discipline in general.
printf "%s" "$available"
If you are giving filter_tools an optional "prefix-per-line", I do
not think it is too much of a stretch to introduce another optional
"perfix for the whole thing" and let this call site say something
like this:
cmd_name=${TOOL_MODE}tool
show_tool_names 'mode_ok && is_available' '\t\t' \
"'git $cmd_name --tool=<tool>' may be set to one of these:"
show_tool_names 'mode_ok && !is_available' '\t\t' \
"These are valid but not available:"
without any of the above logic (and the same for unav). It may look like this:
show_tool_names () {
condition=${1?condition} per_line_prefix=${2:-} preamble=${3:-}
( cd "$MERGE_TOOLS_DIR && ls -1 * ) |
while read toolname
do
if setup_tool "$toolname" 2>/dev/null && (eval "$condition")
then
if test -n "$preamble"
then
echo "$preamble"
preamble=
fi
printf "%s%s\n" "$prefix" "$toolname"
fi
done
}
^ permalink raw reply
* Re: [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Jonathan Nieder @ 2013-01-27 23:40 UTC (permalink / raw)
To: Brandon Casey; +Cc: gitster, pclouds, git, Brandon Casey
In-Reply-To: <CA+sFfMc0_QxwtJ6YS6H+GOW+Pzh3tSZs3rW-VSDf-GWeL=3tDA@mail.gmail.com>
Brandon Casey wrote:
> I'll tweak
> the string so it looks like this:
>
> The signed-off-by string should begin with the words Signed-off-by followed
> by a colon and space, and then the signers name and email address. e.g.
> Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
Yep, that looks better than the example I suggested. :)
[...]
> On Tue, Jan 22, 2013 at 12:17 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> One more test would be interesting: what does "-s" do when asked to
>> produce a duplicate signoff with an interspersed signoff by someone else?
[...]
> This one exists as "adds sob when last sob doesn't match committer".
So it does. Thanks for the sanity check.
^ permalink raw reply
* Re: [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2013-01-27 23:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: gitster, pclouds, git, Brandon Casey
In-Reply-To: <20130122081758.GD6085@elie.Belkin>
On Tue, Jan 22, 2013 at 12:17 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Brandon Casey wrote:
>
>> --- /dev/null
>> +++ b/t/t3511-cherry-pick-x.sh
> [...]
>> +test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' '
>
> IIUC this is an illustration of false-positives from messages like
> this one:
>
> base: do something great without a sign-off
>
> If he does that, it will be the best thing in the
> world: or so I think.
>
> A worthy cause. Could the example broken message be tweaked to
> emphasize that use case? With the current example, I'd consider
> either result (blank line or no blank line) to be ok behavior by git.
The primary motivation for this test was to exercise an existing
behavior which fails to append a newline and sob if the last line of
the last paragraph matches the sob of the committer regardless of
whether the entire paragraph would be interpreted as a conforming
footer. Your example is tested as a side-effect of that. I'll tweak
the string so it looks like this:
The signed-off-by string should begin with the words Signed-off-by followed
by a colon and space, and then the signers name and email address. e.g.
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
> [...]
>> +test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' '
>
> And the other side of basic "-s" functionality.
>
> One more test would be interesting: what does "-s" do when asked to
> produce a duplicate signoff with an interspersed signoff by someone else?
>
> test: a patch with a more complicated life
>
> This patch bounced from $GIT_COMMITTER_NAME to Ms. Thor for
> tweaking, then back to $GIT_COMMITTER_NAME who will be
> recording it in permanent history.
>
> Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
> Signed-off-by: A U Thor <author@example.com>
This one exists as "adds sob when last sob doesn't match committer".
In this case an additional sob should be appended to the footer.
-Brandon
^ permalink raw reply
* Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
From: David Aguilar @ 2013-01-27 23:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping, Johannes Sixt
In-Reply-To: <7v8v7ecicm.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 3:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> +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"
>
> This may happen to be safe as available will not have anything with
> a per-cent % in it, but is not a good discipline in general.
>
> printf "%s" "$available"
>
> If you are giving filter_tools an optional "prefix-per-line", I do
> not think it is too much of a stretch to introduce another optional
> "perfix for the whole thing" and let this call site say something
> like this:
>
> cmd_name=${TOOL_MODE}tool
> show_tool_names 'mode_ok && is_available' '\t\t' \
> "'git $cmd_name --tool=<tool>' may be set to one of these:"
> show_tool_names 'mode_ok && !is_available' '\t\t' \
> "These are valid but not available:"
>
> without any of the above logic (and the same for unav). It may look like this:
>
> show_tool_names () {
> condition=${1?condition} per_line_prefix=${2:-} preamble=${3:-}
>
> ( cd "$MERGE_TOOLS_DIR && ls -1 * ) |
> while read toolname
> do
> if setup_tool "$toolname" 2>/dev/null && (eval "$condition")
> then
> if test -n "$preamble"
> then
> echo "$preamble"
> preamble=
> fi
> printf "%s%s\n" "$prefix" "$toolname"
> fi
> done
> }
Thanks guys. I'll re-roll this soon.
--
David
^ permalink raw reply
* Re: [PATCH 4/4] doc: Generate a list of valid merge tools
From: Junio C Hamano @ 2013-01-27 23:46 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:
> 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
The product of files that include these two and doc.dep target have
to depend on these two files being up to date. doc.dep depends on
$(wildcard *.txt) so you may be lucky and $(mergetools_txt) have
been built once in which case they would depend on it, or unlukcy
and end up doc.dep that does not know about them, no?
^ permalink raw reply
* Re: [PATCH v2 06/10] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2013-01-28 0:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: gitster, pclouds, git, Brandon Casey
In-Reply-To: <20130122083825.GG6085@elie.Belkin>
On Tue, Jan 22, 2013 at 12:38 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Brandon Casey wrote:
>
>> Teach append_signoff how to detect a duplicate s-o-b in the commit footer.
>> This is in preparation to unify the append_signoff implementations in
>> log-tree.c and sequencer.c.
> [...]
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -1082,9 +1101,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
>> return pick_commits(todo_list, opts);
>> }
>>
>> -void append_signoff(struct strbuf *msgbuf, int ignore_footer)
>> +void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
>
> Isn't the behavior of passing '1' here just a bug in "format-patch -s"?
I think that is an open question.
> Style: callers will be easier to read if the function takes a flag
> word with named bits, as in:
>
> #define APPEND_SIGNOFF_DEDUP (1 << 0)
> void append_signoff(..., int ignore_footer, unsigned flag)
Can't I just be lazy!!! Ok, you win. :b
-Brandon
^ 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