Re: [PATCH] Offer to print changes while running git-mergetool
- From
Charles Bailey <charles@hashpling.org>
- Date
- Feb 8, 2009, 01:24 UTC
- Message-ID
- <498E3456.1080509@hashpling.org>
- In-Reply-To
- <57518fd10902070401x14cc7cacrfb8bc88bbf2999cd@mail.gmail.com>
Jonathan del Strother wrote:
Show 15 quoted lines
> On Sat, Feb 7, 2009 at 8:11 AM, Junio C Hamano <gitster@pobox.com> wrote: >> Jonathan del Strother <jon.delStrother@bestbefore.tv> writes: >> >>> Add a "Show changes" option to each prompt in mergetool. This prints the >>> conflicted changes on the current file, using 'git log -p --merge >>> <file>' >> I think the patch should look like this, given the recent conversation I >> had with you. It seems that the script thinks the unit of indentation is >> 4-places, and case arms are indented from case/esac (neither of which is >> the standard git shell script convention), and I tried to match that style >> used in the existing code. >> >> No, I didn't test it. >> >> Charles volunteered to take over mergetool, so he is on the Cc: list.
At the moment, I'm slightly cool towards this patch, but perhaps I don't really understand the underlying issue. I understand wanting to check something (logs) in the middle of a mergetool run but I can't say that I've ever wanted to specifically run 'git log -p --merge'. Perhaps some users of mergetool - being visual people - would more naturally reach for gitk?
Given that mergetool picks up from where it left off when run a second time, what does this patch offer over Ctrl-c, run log tool of your choice, re-run mergetool? Or just running git log in a different terminal instance?
I've made couple of minor comments on the patch below.
Charles.
Show 25 quoted lines
>> git-mergetool.sh | 60 ++++++++++++++++++++++++++++++++++++++++++-----------
>> 1 files changed, 47 insertions(+), 13 deletions(-)
>>
>> diff --git c/git-mergetool.sh w/git-mergetool.sh
>> index 87fa88a..b8604d6 100755
>> --- c/git-mergetool.sh
>> +++ w/git-mergetool.sh
>> @@ -14,6 +14,13 @@ OPTIONS_SPEC=
>> . git-sh-setup
>> require_work_tree
>>
>> +if test -f "$GIT_DIR/MERGE_HEAD"
>> +then
>> + in_merge=t show_changes=", (s)how changes"
>> +else
>> + in_merge=f show_changes=
>> +fi
>> +
>> # Returns true if the mode reflects a symlink
>> is_symlink () {
>> test "$1" = 120000
>> @@ -62,22 +69,28 @@ describe_file () {
>>
>> resolve_symlink_merge () {
>> while true; do--- snip ---
Show 8 quoted lines
>> ;; >> - [aA]*) >> + t,[sS]*) >> + git log -p --merge "$MERGED" >> + printf "\n" >> + resolve_symlink_merge >> + return >> + ;;
Slightly unusual recursion, why not just drop out to the 'while true' loop?
>> esac
>> @@ -87,23 +100,29 @@ resolve_symlink_merge () {
>> resolve_deleted_merge () {
>> while true; do--- snip ---
Show 8 quoted lines
>> ;; >> - [aA]*) >> + t,[sS]*) >> + git log -p --merge "$MERGED" >> + printf "\n" >> + resolve_deleted_merge >> + return >> + ;;
Resursion as above, but why not fall out to the 'while true' again?
Show 18 quoted lines
>> @@ -184,8 +203,23 @@ merge_file () {
>> describe_file "$local_mode" "local" "$LOCAL"
>> describe_file "$remote_mode" "remote" "$REMOTE"
>> if "$prompt" = true; then
>> - printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
>> - read ans
>> + while true; do
>> + case $in_merge in
>> + t) msg_head="(S)how changes, or h" ;;
>> + f) msg_head="H" ;;
>> + esac
>> + print "${msg_head}it return to start merge resolution tool (%s): " "$merge_tool"
>> + read ans
>> + case "$in_merge,$ans" in
>> + t,[sS]*)
>> + git log -p --merge "$MERGED"
>> + printf "\n"
>> + ;;No recursion here, this feels a but more natural to me.