threads / patch / 18830

v3, 2 partsEnsure consistent usage of mergetool.keepBackup in git-gui

Subject: [PATCH v3 1/2] Ensure consistent usage of mergetool.keepBackup in git-gui

## tl;dr

7 messages between Apr 10, 2009 and Apr 12, 2009. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Ferry Huberts· Apr 10, 2009, 19:33 UTC · lore

In several places merge.keepBackup is used i.s.o. mergetool.keepBackup. This patch makes it all consistent for git-gui

Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
---
Based on pu

Shawn, I think you said you already applied this, I'm still resending to keep the series complete.

 git-gui/git-gui.sh        |    2 +-
 git-gui/lib/mergetool.tcl |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Show changes to 2 files +2 −2

git-gui/git-gui.sh, git-gui/lib/mergetool.tcl

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index e018e07..e4e643a 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -699,7 +699,7 @@ proc apply_config {} {
 
 set default_config(branch.autosetupmerge) true
 set default_config(merge.tool) {}
-set default_config(merge.keepbackup) true
+set default_config(mergetool.keepbackup) true
 set default_config(merge.diffstat) true
 set default_config(merge.summary) false
 set default_config(merge.verbosity) 2
diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index eb2b4b5..b7263b3 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -382,7 +382,7 @@ proc merge_tool_finish {fd} {
 		delete_temp_files $mtool_tmpfiles
 		ui_status [mc "Merge tool failed."]
 	} else {
-		if {[is_config_true merge.keepbackup]} {
+		if {[is_config_true mergetool.keepbackup]} {
 			file rename -force -- $backup "$mtool_target.orig"
 		}

-- 
1.6.0.6

Ferry Huberts· Apr 10, 2009, 19:33 UTC · re: Ferry Huberts · lore

[PATCH v3 2/2] Ensure consistent usage of mergetool.keepBackup in git

In several places merge.keepBackup is used i.s.o. mergetool.keepBackup. This patch makes it all consistent for git

Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
---
Based on pu

 git-mergetool.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to git-mergetool.sh +1 −1

diff --git a/git-mergetool.sh b/git-mergetool.sh
index efa31a2..2e3e02b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -257,7 +257,7 @@ prompt_after_failed_merge() {
 merge_tool=$(get_merge_tool "$merge_tool") || exit
 merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
 merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
-merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
 merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
 merge_tool_trust_exit_code="$(git config --bool mergetool."$merge_tool".trustExitCode || echo false)"

-- 
1.6.0.6

Junio C Hamano· Apr 11, 2009, 19:23 UTC · re: Ferry Huberts · lore

Re: [PATCH v3 2/2] Ensure consistent usage of mergetool.keepBackup in git

Ferry Huberts <ferry.huberts@pelagic.nl> writes:

> In several places merge.keepBackup is used i.s.o.
> mergetool.keepBackup. This patch makes it all
> consistent for git

A silly question. The above makes it sound as if both merge.keepBackup and mergetool.keepBackup are valid, but the latter is preferred, and your patch makes things consistent even though without it there is nothing broken per-se.

Is that really is the case?

Otherwise, perhaps the title and the message should read like this:

    Fix misspelled mergetool.keepBackup
    
    In several places mergetool.keepBackup was misspelled as merge.keepBackup.
    
    Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>

Ferry Huberts (Pelagic)· Apr 11, 2009, 19:41 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 2/2] Ensure consistent usage of mergetool.keepBackup in git

Junio C Hamano wrote:

Show 20 quoted lines

> Ferry Huberts <ferry.huberts@pelagic.nl> writes:
> 
>> In several places merge.keepBackup is used i.s.o.
>> mergetool.keepBackup. This patch makes it all
>> consistent for git
> 
> A silly question.  The above makes it sound as if both merge.keepBackup
> and mergetool.keepBackup are valid, but the latter is preferred, and your
> patch makes things consistent even though without it there is nothing
> broken per-se.
> 
> Is that really is the case?
> 
> Otherwise, perhaps the title and the message should read like this:
> 
>     Fix misspelled mergetool.keepBackup
>     
>     In several places mergetool.keepBackup was misspelled as merge.keepBackup.
>     
>     Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>

feel free to change it :-) imho there has been way too much discussion for such a simple patch.

Ferry Huberts (Pelagic)· Apr 11, 2009, 19:44 UTC · re: Ferry Huberts (Pelagic) · lore

Re: [PATCH v3 2/2] Ensure consistent usage of mergetool.keepBackup in git

Ferry Huberts (Pelagic) wrote:

Show 24 quoted lines

> Junio C Hamano wrote:
>> Ferry Huberts <ferry.huberts@pelagic.nl> writes:
>>
>>> In several places merge.keepBackup is used i.s.o.
>>> mergetool.keepBackup. This patch makes it all
>>> consistent for git
>> A silly question.  The above makes it sound as if both merge.keepBackup
>> and mergetool.keepBackup are valid, but the latter is preferred, and your
>> patch makes things consistent even though without it there is nothing
>> broken per-se.
>>
>> Is that really is the case?
>>
>> Otherwise, perhaps the title and the message should read like this:
>>
>>     Fix misspelled mergetool.keepBackup
>>     
>>     In several places mergetool.keepBackup was misspelled as merge.keepBackup.
>>     
>>     Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
> 
> feel free to change it :-)
> imho there has been way too much discussion for such a simple patch.
>

replying to myself :-)

maybe 'way too much' is a bit strong. I think the part of the discussion about finding out what actually to do was good. the rest was a bit too much for my taste. but I'm still getting used to ways of the git.devel list :-)

Junio C Hamano· Apr 11, 2009, 20:19 UTC · re: Ferry Huberts (Pelagic) · lore

Re: [PATCH v3 2/2] Ensure consistent usage of mergetool.keepBackup in git

"Ferry Huberts (Pelagic)" <ferry.huberts@pelagic.nl> writes:

Show 23 quoted lines

> Ferry Huberts (Pelagic) wrote:
>> Junio C Hamano wrote:
>>> Ferry Huberts <ferry.huberts@pelagic.nl> writes:
>>>
>>>> In several places merge.keepBackup is used i.s.o.
>>>> mergetool.keepBackup. This patch makes it all
>>>> consistent for git
>>> A silly question.  The above makes it sound as if both merge.keepBackup
>>> and mergetool.keepBackup are valid, but the latter is preferred, and your
>>> patch makes things consistent even though without it there is nothing
>>> broken per-se.
>>>
>>> Is that really is the case?
>>>
>>> Otherwise, perhaps the title and the message should read like this:
>>>
>>>     Fix misspelled mergetool.keepBackup
>>>     
>>>     In several places mergetool.keepBackup was misspelled as merge.keepBackup.
>>>     
>>>     Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
>> 
>> feel free to change it :-)

Notice that I didn't follow the discussion closely, and I am asking if this is a "bugfix" or "changing for consistency even though there is no breakage".

Ferry Huberts (Pelagic)· Apr 12, 2009, 07:04 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 2/2] Ensure consistent usage of mergetool.keepBackup in git

Junio C Hamano wrote:

Show 29 quoted lines

> "Ferry Huberts (Pelagic)" <ferry.huberts@pelagic.nl> writes:
> 
>> Ferry Huberts (Pelagic) wrote:
>>> Junio C Hamano wrote:
>>>> Ferry Huberts <ferry.huberts@pelagic.nl> writes:
>>>>
>>>>> In several places merge.keepBackup is used i.s.o.
>>>>> mergetool.keepBackup. This patch makes it all
>>>>> consistent for git
>>>> A silly question.  The above makes it sound as if both merge.keepBackup
>>>> and mergetool.keepBackup are valid, but the latter is preferred, and your
>>>> patch makes things consistent even though without it there is nothing
>>>> broken per-se.
>>>>
>>>> Is that really is the case?
>>>>
>>>> Otherwise, perhaps the title and the message should read like this:
>>>>
>>>>     Fix misspelled mergetool.keepBackup
>>>>     
>>>>     In several places mergetool.keepBackup was misspelled as merge.keepBackup.
>>>>     
>>>>     Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
>>> feel free to change it :-)
> 
> Notice that I didn't follow the discussion closely, and I am asking if
> this is a "bugfix" or "changing for consistency even though there is no
> breakage".
>

it's the latter. summary: docs say mergetool.keepBackup. code does merge.keepBackup