On Fri, Jan 25, 2013 at 10:50:58PM -0800, David Aguilar wrote:
> 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 <davvid@xxxxxxxxx>
> ---
>
> 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)'
Shouldn't this be more like this?
$(INSTALL) -m 644 $(subst include,,$(wildcard mergetools/*)) \
'$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
$(INSTALL) -m 644 mergetools/include/* \
'$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
I'm not sure creating an 'include' directory actually buys us much over
declaring that 'vimdiff' is the real script and the others just source
it. Either way there is a single special entry in the mergetools
directory.
> 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"
Unnecessary quoting.
> - # 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
This isn't related to this change (and I think tool-specific is more
correct anyway).
> 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
I'd prefer to see my change to setup_tool done before this so that we
can just use:
setup_tool "$tool" 2>/dev/null || continue
for the above block. I'll send a fixed version in a couple of minutes.
> + 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 0000000..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 0000000..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 0000000..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 0000000..f5890b1
> --- /dev/null
> +++ b/mergetools/vimdiff2
> @@ -0,0 +1 @@
> +. "$MERGE_TOOLS_DIR/include/vim.sh"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html