* [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
@ 2010-01-20 15:30 Sebastian Schuberth
2010-01-20 15:59 ` Johannes Sixt
2010-01-21 7:44 ` David Aguilar
0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Schuberth @ 2010-01-20 15:30 UTC (permalink / raw)
To: git; +Cc: davvid
This is for symmetry with git-difftool, and to make should_prompt_merge ()
reusable from within git-difftool--helper.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
git-mergetool--lib.sh | 9 +++++++++
git-mergetool.sh | 3 ++-
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 5b62785..16b0343 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -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)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b52a741..d453cb0 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -202,7 +202,8 @@ merge_file () {
return 0
}
-prompt=$(git config --bool mergetool.prompt || echo true)
+should_prompt_merge
+prompt=$?
while test $# != 0
do
--
1.6.5.1.1373.g71d1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
2010-01-20 15:30 [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT Sebastian Schuberth
@ 2010-01-20 15:59 ` Johannes Sixt
2010-01-20 16:10 ` Sebastian Schuberth
2010-01-21 7:44 ` David Aguilar
1 sibling, 1 reply; 6+ messages in thread
From: Johannes Sixt @ 2010-01-20 15:59 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git, davvid
Sebastian Schuberth schrieb:
> +should_prompt_merge () {
> + local prompt=$(git config --bool mergetool.prompt || echo true)
'local' is not portable. (The next patch shouldn't introduce it, either.)
> -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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
2010-01-20 15:59 ` Johannes Sixt
@ 2010-01-20 16:10 ` Sebastian Schuberth
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Schuberth @ 2010-01-20 16:10 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, davvid
This is for symmetry with git-difftool, and to make should_prompt_merge ()
reusable from within git-difftool--helper.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
git-mergetool--lib.sh | 9 +++++++++
git-mergetool.sh | 2 +-
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 5b62785..b0e4156 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -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)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b52a741..69f1627 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -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
--
1.6.6.266.g55d982.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
2010-01-20 15:30 [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT Sebastian Schuberth
2010-01-20 15:59 ` Johannes Sixt
@ 2010-01-21 7:44 ` David Aguilar
2010-01-21 10:10 ` Sebastian Schuberth
1 sibling, 1 reply; 6+ messages in thread
From: David Aguilar @ 2010-01-21 7:44 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git
Hi,
On Wed, Jan 20, 2010 at 04:30:26PM +0100, Sebastian Schuberth wrote:
> This is for symmetry with git-difftool, and to make should_prompt_merge ()
> reusable from within git-difftool--helper.
>
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
> git-mergetool--lib.sh | 9 +++++++++
> git-mergetool.sh | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 5b62785..16b0343 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -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
> +}
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.
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index b52a741..d453cb0 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -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?
--
David
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
2010-01-21 7:44 ` David Aguilar
@ 2010-01-21 10:10 ` Sebastian Schuberth
2010-01-22 1:58 ` David Aguilar
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Schuberth @ 2010-01-21 10:10 UTC (permalink / raw)
To: David Aguilar; +Cc: git
On 21.01.2010 08:44, David Aguilar wrote:
> 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.
> 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?
--
Sebastian Schuberth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT
2010-01-21 10:10 ` Sebastian Schuberth
@ 2010-01-22 1:58 ` David Aguilar
0 siblings, 0 replies; 6+ messages in thread
From: David Aguilar @ 2010-01-22 1:58 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git
On Thu, Jan 21, 2010 at 11:10:15AM +0100, Sebastian Schuberth wrote:
> On 21.01.2010 08:44, David Aguilar wrote:
>
>> 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.
>
>> 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?
>
> --
> Sebastian Schuberth
Sounds good.
Thanks,
--
David
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-22 1:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-20 15:30 [PATCH 1/2] Allow to override mergetool.prompt with $GIT_MERGETOOL*_PROMPT Sebastian Schuberth
2010-01-20 15:59 ` Johannes Sixt
2010-01-20 16:10 ` Sebastian Schuberth
2010-01-21 7:44 ` David Aguilar
2010-01-21 10:10 ` Sebastian Schuberth
2010-01-22 1:58 ` David Aguilar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).