David Aguilar wrote:
> Hi
>
> On 0, Ferry Huberts <ferry.huberts@xxxxxxxxxx> wrote:
>> 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@xxxxxxxxxx>
>> ---
>> contrib/difftool/git-difftool.txt | 2 +-
>> git-mergetool.sh | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>
> You missed the usage of merge.keepBackup in
> contrib/difftool/git-difftool-helper.
>
I did miss it. didn't read the grep good enough I guess. thanks
> But.. (for better or worse) the "keep backup" feature
> was completely removed from difftool, so patching this for
> difftool now isn't very useful.
>
> See 2e8af7e4 which is currently in origin/pu:
>
> difftool: remove the backup file feature
>
>
> Also, we just got through a very large round of
> mergetool/difftool changes. The latest version is sitting in
> Junio's pu branch. The "da/difftool" branch's head is
> currently pointing at 21d0ba7e.
>
> It might be worth basing your work on top of that series since
> that'd make things much easier to merge.
>
on pu? I'll do that
>
> My only other comment is:
>
> Aside from git-gui, there are other scripts out there that
> use merge.keepBackup instead of mergetool.keepBackup,
> so changing the name of the config variable has negligable
> user benefit and will cause problems for people that:
>
> A) already have merge.keepBackup configured and then get
> surprised when one day all of a sudden git starts leaving
> around these ".orig turds" (technical term, overheard from an
> actual user) for no apparent reason even though they had
> already configured it to do otherwise in the past, or
>
> B) have written GUIs or scripts that know about
> merge.keepBackup.
>
> Aside from the naming, there's little user benefit to this
> change in my opinion.
>
I patched it this way because contrib/completion/git-completion.bash and
Documentation/config.txt talk about mergetool.keepBackup while only
contrib/difftool/git-difftool.txt talks about merge.keepBackup. That
seemed the most logical way of doing it.
I agree that some users might be surprised after this patch, otoh: I was
quite surprised that I still had turds even when I set
mergetool.keepBackup, which is what the documentation told me to do :-)
Do we really want to keep using 2 names for the same thing?
[rebasing now...]
I'm seeing the following grep on pu:
contrib/completion/git-completion.bash: mergetool.keepBackup
Documentation/config.txt:mergetool.keepBackup::
git-gui/lib/mergetool.tcl:if {[is_config_true merge.keepbackup]} {
git-gui/git-gui:set default_config(merge.keepbackup) true
git-gui/git-gui.sh:set default_config(merge.keepbackup) true
git-mergetool.sh:merge_keep_backup="$(git config --bool merge.keepBackup
|| echo true)"
So it seems that merge.keepBackup is actually used consistently in the
code while the completion and documentation talk about mergetool.keepBackup.
Shall I just patch the completion and documentation instead?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html