From: Johannes Sixt <j6t@kdbg.org>
Date: Sun, 27 Jan 2013 22:26:52 GMT
Subject: Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
Message-ID: <5105A9AC.1040808@kdbg.org>
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