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

Re: [PATCH] mergetool: support --tool-help option like difftool does

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 23, 2012, 19:52 UTC
Message-ID
<7vhasyl0gc.fsf@alter.siamese.dyndns.org>
In-Reply-To
<500D9EDB.90603@gmail.com>
Sebastian Schuberth <sschuberth@gmail.com> writes:
Show 25 quoted lines
> This way we do not have to risk the list of tools go out of sync
> between the implementation and the documentation. Adjust the documentation
> accordingly to not explicitly list the tools but refer to --tool-help.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  Documentation/git-difftool.txt  |  7 ++++---
>  Documentation/git-mergetool.txt |  8 ++++----
>  git-mergetool--lib.sh           | 11 ++++++++++-
>  git-mergetool.sh                | 42 ++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 59 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
> index 31fc2e3..0bdfe35 100644
> --- a/Documentation/git-difftool.txt
> +++ b/Documentation/git-difftool.txt
> @@ -36,9 +36,10 @@ OPTIONS
>  
>  -t <tool>::
>  --tool=<tool>::
> -	Use the diff tool specified by <tool>.  Valid values include
> -	emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help`
> -	for the list of valid <tool> settings.
> +	Use the diff tool specified by <tool>.

I do not see how it is an improvement to drop the most common ones. People sometimes read documentation without having an access to shell to run "cmd --tool-help", and a list of handful of well known ones would serve as a good hint to let the reader know the kind of commands the front-end is capable of spawning, which in turn help such a reader to imagine how the command is used to judge if it is something the reader wants to use.

Show 19 quoted lines
> +
> +--tool-help::
> +	List all supported values for <tool>.
>  +
>  If a diff tool is not specified, 'git difftool'
>  will use the configuration variable `diff.tool`.  If the
> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
> index 2a49de7..99e53b1 100644
> --- a/Documentation/git-mergetool.txt
> +++ b/Documentation/git-mergetool.txt
> @@ -26,10 +26,10 @@ OPTIONS
>  -------
>  -t <tool>::
>  --tool=<tool>::
> -	Use the merge resolution program specified by <tool>.
> -	Valid merge tools are:
> -	araxis, bc3, diffuse, ecmerge, emerge, gvimdiff, kdiff3,
> -	meld, opendiff, p4merge, tkdiff, tortoisemerge, vimdiff and xxdiff.
> +	Use the merge tool specified by <tool>.

Likewise. Dropping araxis, bc3, etc. to trim the list to 4-5 items that people would know what they are would make sense, though. The purpose of the list is not to tell if the reader's favorite tool is supported or not---it is to hint the flavor of what kind of things the command spawned would be capable of.

Show 115 quoted lines
> +
> +--tool-help::
> +	List all supported values for <tool>.
>  +
>  If a merge resolution program is not specified, 'git mergetool'
>  will use the configuration variable `merge.tool`.  If the
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index ed630b2..f0865d4 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -111,15 +111,18 @@ run_merge_tool () {
>  	return $status
>  }
>  
> -guess_merge_tool () {
> +list_merge_tool_candidates () {
> +	# Add tools that can either do merging or diffing, but not both.
>  	if merge_mode
>  	then
>  		tools="tortoisemerge"
>  	else
>  		tools="kompare"
>  	fi
> +
>  	if test -n "$DISPLAY"
>  	then
> +		# Prefer GTK-based tools under Gnome.
>  		if test -n "$GNOME_DESKTOP_SESSION_ID"
>  		then
>  			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
> @@ -128,6 +131,8 @@ guess_merge_tool () {
>  		fi
>  		tools="$tools gvimdiff diffuse ecmerge p4merge araxis bc3"
>  	fi
> +
> +	# Prefer vimdiff if vim is the default editor.
>  	case "${VISUAL:-$EDITOR}" in
>  	*vim*)
>  		tools="$tools vimdiff emerge"
> @@ -136,6 +141,10 @@ guess_merge_tool () {
>  		tools="$tools emerge vimdiff"
>  		;;
>  	esac
> +}
> +
> +guess_merge_tool () {
> +	list_merge_tool_candidates
>  	echo >&2 "merge tool candidates: $tools"
>  
>  	# Loop over each candidate and stop when a valid merge tool is found.
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index a9f23f7..0db0c44 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -8,7 +8,7 @@
>  # at the discretion of Junio C Hamano.
>  #
>  
> -USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
> +USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
>  SUBDIRECTORY_OK=Yes
>  OPTIONS_SPEC=
>  TOOL_MODE=merge
> @@ -284,11 +284,51 @@ merge_file () {
>      return 0
>  }
>  
> +show_tool_help () {
> +	TOOL_MODE=merge
> +	list_merge_tool_candidates
> +	unavailable= available= LF='
> +'
> +	for i in $tools
> +	do
> +		merge_tool_path=$(translate_merge_tool_path "$i")
> +		if type "$merge_tool_path" >/dev/null 2>&1
> +		then
> +			available="$available$i$LF"
> +		else
> +			unavailable="$unavailable$i$LF"
> +		fi
> +	done
> +	if test -n "$available"
> +	then
> +		echo "'git mergetool --tool=<tool>' may be set to one of the following:"
> +		echo "$available" | sort | sed -e 's/^/	/'
> +	else
> +		echo "No suitable tool for 'git mergetool --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/^/	/'
> +	fi
> +	if test -n "$unavailable$available"
> +	then
> +		echo
> +		echo "Some of the tools listed above only work in a windowed"
> +		echo "environment. If run in a terminal-only session, they will fail."
> +	fi
> +	exit 0
> +}
> +
>  prompt=$(git config --bool mergetool.prompt || echo true)
>  
>  while test $# != 0
>  do
>      case "$1" in
> +	--tool-help)
> +		show_tool_help
> +		;;
>  	-t|--tool*)
>  	    case "$#,$1" in
>  		*,*=*)
Previous: Sebastian SchuberthNext: Sebastian Schuberth
Message 15 of 40 in “Various merge / diff tool related minor clean-ups and improvements”
  1. 0/4 Various merge / diff tool related minor clean-ups and improvementsSebastian Schuberth, Jul 23, 2012
  2. 0/5 Various merge / diff tool related minor clean-ups and improvementsSebastian Schuberth, Jul 23, 2012
  3. 1/5 Sort the list of tools that support both merging and diffing alphabeticallySebastian Schuberth, Jul 23, 2012
  4. 2/5 Use variables for the lists of tools that support merging / diffingSebastian Schuberth, Jul 23, 2012
  5. Junio C HamanoJul 23, 2012
  6. Sebastian SchuberthJul 23, 2012
  7. 1/4 Use variables for the lists of tools that support merging / diffingSebastian Schuberth, Jul 23, 2012
  8. Junio C HamanoJul 23, 2012
  9. Sebastian SchuberthJul 23, 2012
  10. 3/5 Explicitly list all valid diff tools and document --tool-help as an optionSebastian Schuberth, Jul 23, 2012
  11. Junio C HamanoJul 23, 2012
  12. mergetool: support --tool-help option like difftool doesJunio C Hamano, Jul 23, 2012
  13. Sebastian SchuberthJul 23, 2012
  14. mergetool: support --tool-help option like difftool doesSebastian Schuberth, Jul 23, 2012
  15. Junio C HamanoJul 23, 2012
  16. Sebastian SchuberthJul 23, 2012
  17. David AguilarJul 23, 2012
  18. Junio C HamanoJul 23, 2012
  19. Sebastian SchuberthJul 23, 2012
  20. Junio C HamanoJul 23, 2012
  21. Re*: mergetool: support --tool-help option like difftool doesJunio C Hamano, Aug 23, 2012
  22. David AguilarAug 23, 2012
  23. Junio C HamanoAug 23, 2012
  24. David AguilarAug 24, 2012
  25. Jens LehmannAug 26, 2012
  26. 4/5 Make sure to use Araxis' "compare" and not e.g. ImageMagick'sSebastian Schuberth, Jul 23, 2012
  27. Junio C HamanoJul 23, 2012
  28. Make sure to use Araxis' "compare" and not e.g. ImageMagick'sSebastian Schuberth, Jul 23, 2012
  29. Junio C HamanoJul 23, 2012
  30. Sebastian SchuberthJul 23, 2012
  31. Junio C HamanoJul 23, 2012
  32. Sebastian SchuberthJul 23, 2012
  33. David AguilarJul 23, 2012
  34. Sebastian SchuberthJul 23, 2012
  35. Andreas SchwabJul 23, 2012
  36. Junio C HamanoJul 23, 2012
  37. Sebastian SchuberthJul 23, 2012
  38. Junio C HamanoJul 23, 2012
  39. Sebastian SchuberthJul 23, 2012
  40. 5/5 Add a few more code comments and blank lines in guess_merge_toolSebastian Schuberth, Jul 23, 2012

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.