Re: [PATCH] mergetools: Simplify how we handle "vim" and "defaults"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 27, 2013, 04:57 UTC
- Message-ID
- <7v8v7fz0ii.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130126121202.GH7498@serenity.lan>
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" -}