git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools

From
John Keeping <john@keeping.me.uk>
Date
Jan 29, 2013, 19:48 UTC
Message-ID
<20130129194846.GD1342@serenity.lan>
In-Reply-To
<1359334346-5879-4-git-send-email-davvid@gmail.com>
On Sun, Jan 27, 2013 at 04:52:25PM -0800, David Aguilar wrote:
Show 20 quoted lines
> --- 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? I would expect ls to list the current directory if given no arguments, but perhaps some platforms behave differently?

Show 61 quoted lines
> +	while read toolname
> +	do
> +		if setup_tool "$toolname" 2>/dev/null &&
> +			(eval "$condition" "$toolname")
> +		then
> +			if test -n "$preamble"
> +			then
> +				echo "$preamble"
> +				preamble=
> +			fi
> +			printf "%s%s\n" "$per_line_prefix" "$tool"
> +		fi
> +	done
> +}
> +
>  diff_mode() {
>  	test "$TOOL_MODE" = diff
>  }
> @@ -199,35 +228,21 @@ list_merge_tool_candidates () {
>  }
>  
>  show_tool_help () {
> -	unavailable= available= LF='
> -'
> -	for i in "$MERGE_TOOLS_DIR"/*
> -	do
> -		tool=$(basename "$i")
> -		setup_tool "$tool" 2>/dev/null || continue
> -
> -		merge_tool_path=$(translate_merge_tool_path "$tool")
> -		if type "$merge_tool_path" >/dev/null 2>&1
> -		then
> -			available="$available$tool$LF"
> -		else
> -			unavailable="$unavailable$tool$LF"
> -		fi
> -	done
> -
> -	cmd_name=${TOOL_MODE}tool
> +	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:"

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"
John
Previous: Junio C HamanoNext: Junio C Hamano
Message 9 of 36 in “Auto-generate mergetool lists”
  1. 0/4 Auto-generate mergetool listsDavid Aguilar, Jan 28, 2013
  2. 1/4 mergetool--lib: Simplify command expressionsDavid Aguilar, Jan 28, 2013
  3. 2/4 mergetool--lib: Improve the help text in guess_merge_tool()David Aguilar, Jan 28, 2013
  4. 3/4 mergetool--lib: Add functions for finding available toolsDavid Aguilar, Jan 28, 2013
  5. 4/4 doc: Generate a list of valid merge toolsDavid Aguilar, Jan 28, 2013
  6. Junio C HamanoJan 28, 2013
  7. John KeepingJan 28, 2013
  8. Junio C HamanoJan 28, 2013
  9. John KeepingJan 29, 2013
  10. Junio C HamanoJan 29, 2013
  11. David AguilarJan 29, 2013
  12. Junio C HamanoJan 29, 2013
  13. John KeepingJan 29, 2013
  14. Junio C HamanoJan 30, 2013
  15. Junio C HamanoJan 30, 2013
  16. John KeepingJan 29, 2013
  17. John KeepingJan 29, 2013
  18. David AguilarJan 29, 2013
  19. Junio C HamanoJan 29, 2013
  20. 1/4 mergetool--lib: simplify command expressionsDavid Aguilar, Jan 30, 2013
  21. Junio C HamanoJan 30, 2013
  22. Junio C HamanoJan 28, 2013
  23. David AguilarJan 28, 2013
  24. Junio C HamanoJan 28, 2013
  25. David AguilarJan 28, 2013
  26. Junio C HamanoJan 28, 2013
  27. John KeepingJan 28, 2013
  28. Junio C HamanoJan 28, 2013
  29. John KeepingJan 28, 2013
  30. Joachim SchmitzJan 29, 2013
  31. John KeepingJan 29, 2013
  32. Junio C HamanoJan 29, 2013
  33. John KeepingJan 29, 2013
  34. Philip OakleyJan 28, 2013
  35. David AguilarJan 28, 2013
  36. Philip OakleyJan 28, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.