Re: [PATCH] mergetools: Simplify how we handle "vim" and "defaults"

> John Keeping <john@keeping.me.uk> writes:
>
>> 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.
>
> Is 'include' really used for such a purpose?  It only houses defaults.sh
> as far as I can see.
>
> As that defaults.sh file is used only to define trivially empty
> shell functions, I wonder if it is better to get rid of it, and
> define these functions in line in git-mergetool--lib.sh.  Such a
> change would like the attached on top of the entire series.

>  Makefile                       |  6 +-----
>  git-mergetool--lib.sh          | 24 ++++++++++++++++++++++--
>  mergetools/include/defaults.sh | 22 ----------------------
>  3 files changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 26f217f..f69979e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2724,11 +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 $(filter-out mergetools/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'
> +       $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
>  ifndef NO_GETTEXT
>         $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
>         (cd po/build/locale && $(TAR) cf - .) | \
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 7ea7510..1d0fb12 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -57,8 +57,28 @@ setup_tool () {
>                 return 2
>         fi
>
> -       # Load the default functions
> -       . "$MERGE_TOOLS_DIR/include/defaults.sh"
> +       # Fallback definitions, to be overriden by tools.
> +       can_merge () {
> +               return 0
> +       }
> +
> +       can_diff () {
> +               return 0
> +       }
> +
> +       diff_cmd () {
> +               status=1
> +               return $status
> +       }
> +
> +       merge_cmd () {
> +               status=1
> +               return $status
> +       }
> +
> +       translate_merge_tool_path () {
> +               echo "$1"
> +       }
>
>         # Load the redefined functions
>         . "$MERGE_TOOLS_DIR/$tool"
> diff --git a/mergetools/include/defaults.sh b/mergetools/include/defaults.sh
> deleted file mode 100644
> index 21e63ec..0000000
> --- a/mergetools/include/defaults.sh
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -# Redefined by builtin tools
> -can_merge () {
> -       return 0
> -}
> -
> -can_diff () {
> -       return 0
> -}
> -
> -diff_cmd () {
> -       status=1
> -       return $status
> -}
> -
> -merge_cmd () {
> -       status=1
> -       return $status
> -}
> -
> -translate_merge_tool_path () {
> -       echo "$1"
> -}