Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
PATCH 1/2 Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
This is for symmetry with git-difftool, and to make should_prompt_merge () reusable from within git-difftool--helper.
--- git-mergetool--lib.sh | 9 +++++++++ git-mergetool.sh | 3 ++- 2 files changed, 11 insertions(+), 1 deletions(-)
git-mergetool--lib.sh +9 -0
@@ -7,6 +7,15 @@ merge_mode() { test "$TOOL_MODE" = merge } +should_prompt_merge () { + local prompt=$(git config --bool mergetool.prompt || echo true) + if test "$prompt" = true; then + test -z "$GIT_MERGETOOL_NO_PROMPT" + else + test -n "$GIT_MERGETOOL_PROMPT" + fi +} + translate_merge_tool_path () { case "$1" in vimdiff)
git-mergetool.sh +2 -1
@@ -202,7 +202,8 @@ merge_file () { return 0 } -prompt=$(git config --bool mergetool.prompt || echo true) +should_prompt_merge +prompt=$? while test $# != 0 do
Signature · 1 line
1.6.5.1.1373.g71d1
Hi,
Quoted 15 lines · Sebastian Schuberth
This is for symmetry with git-difftool, and to make
should_prompt_merge() reusable from within git-difftool--helper.Signed-off-by Sebastian Schuberth <[email protected]>--- git-mergetool--lib.sh | 9 +++++++++ git-mergetool.sh | 3 ++- 2 files changed, 11 insertions(+), 1 deletions(-)git-mergetool--lib.sh +0
-0@@ -7,6 +7,15 @@ merge_mode() { test "$TOOL_MODE" = merge
git-mergetool--lib.sh merge_mode() { · line 17
Quoted 10 lines
git-mergetool--lib.sh +8
-0} +should_prompt_merge () { + local prompt=$(git config --bool mergetool.prompt || echo true) + if test "$prompt" = true; then + test -z "$GIT_MERGETOOL_NO_PROMPT" + else + test -n "$GIT_MERGETOOL_PROMPT" + fi +}
Someone already mentioned dropping "local" here.
The GIT_DIFFTOOL*_PROMPT variables are implementation details about how git-difftool passes options to git-difftool--helper. We don't advertise them in the documentation so we probably shouldn't support them aside from what is needed for git-difftool. We can drop this part.
61ed71dc just recently removed git-difftool--helper's use of GIT_MERGE_TOOL based on this rationale.
Quoted 5 lines
git-mergetool.sh +0
-0
git-mergetool.sh merge_file () { · line 209
Quoted 9 lines
git-mergetool.sh +2
-1@@ -202,7 +202,8 @@ merge_file () { return 0 } -prompt=$(git config --bool mergetool.prompt || echo true) +should_prompt_merge +prompt=$? while test $# != 0 do
git-difftool falling back to mergetool.prompt when difftool.prompt is not available is good, especially since we advertise that feature.
Once you drop the local declarations and the env. variable it should be good to go. Patch v2, soon?
Signature · 1 line
David
Quoted 5 lines · David Aguilar
The GIT_DIFFTOOL*_PROMPT variables are implementation details about how git-difftool passes options to git-difftool--helper. We don't advertise them in the documentation so we probably shouldn't support them aside from what is needed for git-difftool. We can drop this part.
Thanks for the explanation, I already was wondering why they don't appear in the docs.
Quoted 6 lines
git-difftool falling back to mergetool.prompt when difftool.prompt is not available is good, especially since we advertise that feature.
Once you drop the local declarations and the env. variable it should be good to go. Patch v2, soon?
If I drop env. variable support, I don't see much reason to introduce the should_prompt_merge () at all. I'd then leave git-mergetool* as-is, and directly run "git config --bool mergetool.prompt" inside should_prompt_diff (). Would that be OK with you?
Signature · 1 line
Sebastian Schuberth
Quoted 15 lines · Sebastian Schuberth
Quoted 5 lines · David Aguilar
The GIT_DIFFTOOL*_PROMPT variables are implementation details about how git-difftool passes options to git-difftool--helper. We don't advertise them in the documentation so we probably shouldn't support them aside from what is needed for git-difftool. We can drop this part.
Thanks for the explanation, I already was wondering why they don't appear in the docs.
Quoted 4 lines
git-difftool falling back to mergetool.prompt when difftool.prompt is not available is good, especially since we advertise that feature.
Quoted 10 lines
Quoted 2 lines
Once you drop the local declarations and the env. variable it should be good to go. Patch v2, soon?
If I drop env. variable support, I don't see much reason to introduce the
should_prompt_merge() at all. I'd then leave git-mergetool* as-is, and directly run "git config --bool mergetool.prompt" insideshould_prompt_diff(). Would that be OK with you?Signature · 1 line
Sebastian Schuberth
Sounds good.
Thanks,
Signature · 1 line
David
git-mergetool--lib.sh merge_mode() { · line 11
Quoted 2 lines · Sebastian Schuberth
git-mergetool--lib.sh +2
-0+should_prompt_merge () { + local prompt=$(git config --bool mergetool.prompt || echo true)
'local' is not portable. (The next patch shouldn't introduce it, either.)
git-mergetool.sh merge_file () { · line 206
Quoted 3 lines
git-mergetool.sh +2
-1-prompt=$(git config --bool mergetool.prompt || echo true) +should_prompt_merge +prompt=$?
Previously, prompt was either "true" or "false", but now it is a number. This is not what the user (function merge_file) expects.
-- Hannes
This is for symmetry with git-difftool, and to make should_prompt_merge () reusable from within git-difftool--helper.
--- git-mergetool--lib.sh | 9 +++++++++ git-mergetool.sh | 2 +- 2 files changed, 10 insertions(+), 1 deletions(-)
git-mergetool--lib.sh +9 -0
@@ -7,6 +7,15 @@ merge_mode() { test "$TOOL_MODE" = merge } +should_prompt_merge () { + prompt=$(git config --bool mergetool.prompt || echo true) + if test "$prompt" = true; then + test -z "$GIT_MERGETOOL_NO_PROMPT" + else + test -n "$GIT_MERGETOOL_PROMPT" + fi +} + translate_merge_tool_path () { case "$1" in vimdiff)
git-mergetool.sh +1 -1
@@ -202,7 +202,7 @@ merge_file () { return 0 } -prompt=$(git config --bool mergetool.prompt || echo true) +prompt=$(should_prompt_merge && echo true || echo false) while test $# != 0 do
Signature · 1 line
1.6.6.266.g55d982.dirty