Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

git-mergetool: improve error code paths and messages #1827

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

git-mergetool: improve error code paths and messages #1827

wants to merge 5 commits into from
+20 −11

Commits on Nov 11, 2024

  1. completion: complete '--tool-help' in 'git mergetool' 

    Signed-off-by: Philippe Blain <[email protected]>
    @phil-blain
    phil-blain committed Nov 11, 2024
    Configuration menu
    Copy the full SHA
    24933ba View commit details
    Browse the repository at this point in the history

  2. git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool 

    In git-mergetool--lib.sh::get_merge_tool_path, we check if the chosen
tool is valid via valid_tool and exit with an error message if not. This
error message mentions "Unknown merge tool", even if the command the
user tried was 'git difftool --tool=unknown'. Use the global 'TOOL_MODE'
variable for a more correct error message.

Signed-off-by: Philippe Blain <[email protected]>
    @phil-blain
    phil-blain committed Nov 11, 2024
    Configuration menu
    Copy the full SHA
    6f7f553 View commit details
    Browse the repository at this point in the history

  3. git-mergetool--lib.sh: add error message in 'setup_user_tool' 

    In git-mergetool--lib.sh::setup_tool, we check if the given tool is a
known builtin tool, a known variant, or a user-defined tool by calling
setup_user_tool, and we return with the exit code from setup_user_tool
if it was called. setup_user_tool checks if {diff,merge}tool.$tool.cmd
is set and quietly returns with an error if not.

This leads to the following invocation quietly failing:

	git mergetool --tool=unknown

which is not very user-friendly. Adjust setup_user_tool to output an
error message before returning if {diff,merge}tool.$tool.cmd is not set.

Adjust the second call to setup_user_tool in setup_tool to silence this
new error, as this call is only meant to allow users to redefine 'cmd'
for a builtin tool; it is not an error if they have not done so (which
is why we do not check the return status of this call).

Note that this behaviour of quietly failing is a regression dating back
to de8dafb (mergetool: break setup_tool out into separate
initialization function, 2021-02-09), as before this commit an unknown
mergetool would be diagnosed in get_merge_tool_path when called from
run_merge_tool.

Signed-off-by: Philippe Blain <[email protected]>
    @phil-blain
    phil-blain committed Nov 11, 2024
    Configuration menu
    Copy the full SHA
    79c3a6f View commit details
    Browse the repository at this point in the history

  4. git-mergetool--lib.sh: add error message for unknown tool variant 

    In setup_tool, we check if the given tool is a known variant of a tool,
and quietly return with an error if not. This leads to the following
invocation quietly failing:

	git mergetool --tool=vimdiff4

Add an error message before returning in this case.

Signed-off-by: Philippe Blain <[email protected]>
    @phil-blain
    phil-blain committed Nov 11, 2024
    Configuration menu
    Copy the full SHA
    74b83ca View commit details
    Browse the repository at this point in the history

  5. git-difftool--helper.sh: exit upon initialize_merge_tool errors 

    Since the introduction of 'initialize_merge_tool' in de8dafb
(mergetool: break setup_tool out into separate initialization function,
2021-02-09), any errors from this function are ignored in
git-difftool--helper.sh::launch_merge_tool, which is not the case for
its call in git-mergetool.sh::merge_file.

Despite the in-code comment, initialize_merge_tool (via its call to
setup_tool) does different checks than run_merge_tool, so it makes sense
to abort early if it encounters errors. Add exit calls if
initialize_merge_tool fails.

Signed-off-by: Philippe Blain <[email protected]>
    @phil-blain
    phil-blain committed Nov 11, 2024
    Configuration menu
    Copy the full SHA
    be0b86f View commit details
    Browse the repository at this point in the history