John Keeping [off-list ref] writes:
On Sun, Jan 27, 2013 at 04:52:25PM -0800, David Aguilar wrote:
quoted
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,35 @@
# 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
+}
+
+show_tool_names () {
+ condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
+
+ ( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
Is the '*' necessary here?
No, it was just from a bad habit (I have ls aliased to ls -A or ls
-a in my interactive environment, which trained my fingers to this).
I also think you can lose -1, although it does not hurt.
quoted
+ tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
+ available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
+ "$tool_opt may be set to one of the following:")
+ unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
+ "The following tools are valid, but not currently available:")
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/^/ /'
+ echo "$available"
else
echo "No suitable tool for 'git $cmd_name --tool=<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/^/ /'
+ echo "$unavailable"
fi
if test -n "$unavailable$available"
then
You haven't taken full advantage of the simplification Junio suggested
in response to v1 here. We can change the "unavailable" block to be:
show_tool_names 'mode_ok && ! is_available' "$TAB$TAB" \
"${LF}The following tools are valid, but not currently available:"
Actually I was hoping that we can enhance show_tool_names so that we
can do without the $available and $unavailable variables at all.
If you also add a "not_found_msg" parameter to show_tool_names then the
"available" case is also simplified:
show_tool_names 'mode_ok && is_available' "$TAB$TAB" \
"$tool_opt may be set to one of the following:" \
"No suitable tool for 'git $cmd_name --tool=<tool>' found."
with this at the end of show_tool_names:
test -n "$preamble" && test -n "$not_found_msg" && \
echo "$not_found_msg"
Yes, something along that line.
On Tue, Jan 29, 2013 at 12:22 PM, Junio C Hamano [off-list ref] wrote:
John Keeping [off-list ref] writes:
quoted
On Sun, Jan 27, 2013 at 04:52:25PM -0800, David Aguilar wrote:
quoted
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,35 @@
# 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
+}
+
+show_tool_names () {
+ condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
+
+ ( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
Is the '*' necessary here?
No, it was just from a bad habit (I have ls aliased to ls -A or ls
-a in my interactive environment, which trained my fingers to this).
I also think you can lose -1, although it does not hurt.
quoted
quoted
+ tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
+ available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
+ "$tool_opt may be set to one of the following:")
+ unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
+ "The following tools are valid, but not currently available:")
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/^/ /'
+ echo "$available"
else
echo "No suitable tool for 'git $cmd_name --tool=<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/^/ /'
+ echo "$unavailable"
fi
if test -n "$unavailable$available"
then
You haven't taken full advantage of the simplification Junio suggested
in response to v1 here. We can change the "unavailable" block to be:
show_tool_names 'mode_ok && ! is_available' "$TAB$TAB" \
"${LF}The following tools are valid, but not currently available:"
Actually I was hoping that we can enhance show_tool_names so that we
can do without the $available and $unavailable variables at all.
quoted
If you also add a "not_found_msg" parameter to show_tool_names then the
"available" case is also simplified:
show_tool_names 'mode_ok && is_available' "$TAB$TAB" \
"$tool_opt may be set to one of the following:" \
"No suitable tool for 'git $cmd_name --tool=<tool>' found."
with this at the end of show_tool_names:
test -n "$preamble" && test -n "$not_found_msg" && \
echo "$not_found_msg"
Yes, something along that line.
I don't want to stomp on your feet and poke at this file too much if
you're planning on building on top of it (I already did a few times
;-). My git time is a bit limited for the next few days so I don't
want to hold you up. I can help shepherd through small fixups that
come up until the weekend rolls around and I have more time, but I
also don't want to hold you back until then.
I will have some time tonight. If you guys would prefer an
incremental patch I can send one that changes the "ls" expression and
the way the unavailable block is structured. Otherwise, I can send
replacements to handle the "test -z" thing, $TAB$TAB, and the
simplification of the unavailable block.
Later patches (that are working towards the new feature) can
generalize show_tool_names() further and eliminate the need for the
available/unavailable variables altogether. John, I would imagine
that you'd want to pick that up since you're driving towards having
--tool-help honor custom tools.
What do you think?
--
David
On Tue, Jan 29, 2013 at 02:27:21PM -0800, David Aguilar wrote:
I don't want to stomp on your feet and poke at this file too much if
you're planning on building on top of it (I already did a few times
;-). My git time is a bit limited for the next few days so I don't
want to hold you up. I can help shepherd through small fixups that
come up until the weekend rolls around and I have more time, but I
also don't want to hold you back until then.
I will have some time tonight. If you guys would prefer an
incremental patch I can send one that changes the "ls" expression and
the way the unavailable block is structured. Otherwise, I can send
replacements to handle the "test -z" thing, $TAB$TAB, and the
simplification of the unavailable block.
Later patches (that are working towards the new feature) can
generalize show_tool_names() further and eliminate the need for the
available/unavailable variables altogether. John, I would imagine
that you'd want to pick that up since you're driving towards having
--tool-help honor custom tools.
What do you think?
I was planning to hold off until this series is in a reasonable state -
there's no rush as far as I'm concerned, but if Junio's happy to leave
these patches with just the small fixups I'm happy to build on that with
a patch that removes the available and unavailable variables before
adding the tools from git-config.
John