From: Markus Heidelberg <markus.heidelberg@web.de>
To: David Aguilar <davvid@gmail.com>
Cc: gitster@pobox.com, git@vger.kernel.org, charles@hashpling.org
Subject: Re: [PATCH v3 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
Date: Tue, 7 Apr 2009 20:11:14 +0200 [thread overview]
Message-ID: <200904072011.15503.markus.heidelberg@web.de> (raw)
In-Reply-To: <1239092483-14973-6-git-send-email-davvid@gmail.com>
David Aguilar, 07.04.2009:
> This consolidates the common functionality from git-mergetool and
> git-difftool--helper into a single git-mergetool--lib scriptlet.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>
> Includes suggestions from Markus.
> We also moved opendiff up to the front so that it's
> preferred when on Mac OS.
>
> .gitignore | 1 +
> Documentation/git-mergetool--lib.txt | 56 +++++
> Makefile | 1 +
> git-difftool--helper.sh | 186 +----------------
> git-mergetool--lib.sh | 387 ++++++++++++++++++++++++++++++++++
> git-mergetool.sh | 224 ++------------------
> 6 files changed, 467 insertions(+), 388 deletions(-)
> create mode 100644 Documentation/git-mergetool--lib.txt
> create mode 100644 git-mergetool--lib.sh
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> +check_unchanged () {
> + if merge_mode; then
> + if test "$MERGED" -nt "$BACKUP"; then
> + status=0
> + else
> + while true; do
> + echo "$MERGED seems unchanged."
> + printf "Was the merge successful? [y/n] "
> + read answer < /dev/tty
> + case "$answer" in
> + y*|Y*) status=0; break ;;
> + n*|N*) status=1; break ;;
> + esac
> + done
> + fi
> + else
> + status=0
> + fi
> +}
This function is now only called when $merge_mode is set.
> +run_merge_tool () {
> + base_present="$2"
> + if diff_mode; then
> + status=0
> + base_present="false"
> + fi
$base_present also is only used when $merge_mode is set. And I think we
don't need to set status=0 conditionally then.
> + case "$1" in
> + kdiff3)
> + if merge_mode; then
> + if $base_present; then
> + ("$merge_tool_path" --auto \
> + --L1 "$MERGED (Base)" \
> + --L2 "$MERGED (Local)" \
> + --L3 "$MERGED (Remote)" \
> + -o "$MERGED" \
> + "$BASE" "$LOCAL" "$REMOTE" \
> + > /dev/null 2>&1)
> + else
> + ("$merge_tool_path" --auto \
> + --L1 "$MERGED (Local)" \
> + --L2 "$MERGED (Remote)" \
> + -o "$MERGED" \
> + "$LOCAL" "$REMOTE" \
> + > /dev/null 2>&1)
> + fi
> + status=$?
> + else
> + ("$merge_tool_path" --auto \
> + --L1 "$MERGED (A)" \
> + --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
> + > /dev/null 2>&1)
> + fi
> + ;;
> + kompare)
> + "$merge_tool_path" "$LOCAL" "$REMOTE"
> + ;;
> + tkdiff)
> + if merge_mode; then
> + if $base_present; then
> + "$merge_tool_path" -a "$BASE" \
> + -o "$MERGED" "$LOCAL" "$REMOTE"
> + else
> + "$merge_tool_path" \
> + -o "$MERGED" "$LOCAL" "$REMOTE"
> + fi
> + status=$?
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE"
> + fi
> + ;;
> + meld)
> + if merge_mode; then
> + touch "$BACKUP"
> + "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
> + check_unchanged
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE"
> + fi
> + ;;
> + diffuse)
> + if merge_mode; then
> + touch "$BACKUP"
> + if $base_present; then
> + "$merge_tool_path" \
> + "$LOCAL" "$MERGED" "$REMOTE" \
> + "$BASE" | cat
> + else
> + "$merge_tool_path" \
> + "$LOCAL" "$MERGED" "$REMOTE" | cat
> + fi
> + check_unchanged
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE" | cat
> + fi
> + ;;
> + vimdiff)
> + if merge_mode; then
> + touch "$BACKUP"
> + "$merge_tool_path" -d -c "wincmd l" \
> + "$LOCAL" "$MERGED" "$REMOTE"
> + check_unchanged
> + else
> + "$merge_tool_path" -d -c "wincmd l" \
> + "$LOCAL" "$REMOTE"
> + fi
> + ;;
> + gvimdiff)
> + if merge_mode; then
> + touch "$BACKUP"
> + "$merge_tool_path" -d -c "wincmd l" -f \
> + "$LOCAL" "$MERGED" "$REMOTE"
> + check_unchanged
> + else
> + "$merge_tool_path" -d -c "wincmd l" -f \
> + "$LOCAL" "$REMOTE"
> + fi
> + ;;
> + xxdiff)
> + if merge_mode; then
> + touch "$BACKUP"
> + if $base_present; then
> + "$merge_tool_path" -X --show-merged-pane \
> + -R 'Accel.SaveAsMerged: "Ctrl-S"' \
> + -R 'Accel.Search: "Ctrl+F"' \
> + -R 'Accel.SearchForward: "Ctrl-G"' \
> + --merged-file "$MERGED" \
> + "$LOCAL" "$BASE" "$REMOTE"
> + else
> + "$merge_tool_path" -X $extra \
> + -R 'Accel.SaveAsMerged: "Ctrl-S"' \
> + -R 'Accel.Search: "Ctrl+F"' \
> + -R 'Accel.SearchForward: "Ctrl-G"' \
> + --merged-file "$MERGED" \
> + "$LOCAL" "$REMOTE"
> + fi
> + check_unchanged
> + else
> + "$merge_tool_path" \
> + -R 'Accel.Search: "Ctrl+F"' \
> + -R 'Accel.SearchForward: "Ctrl-G"' \
> + "$LOCAL" "$REMOTE"
> + fi
> + ;;
> + opendiff)
> + if merge_mode; then
> + touch "$BACKUP"
> + if $base_present; then
> + "$merge_tool_path" "$LOCAL" "$REMOTE" \
> + -ancestor "$BASE" \
> + -merge "$MERGED" | cat
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE" \
> + -merge "$MERGED" | cat
> + fi
> + check_unchanged
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE" | cat
> + fi
> + ;;
> + ecmerge)
> + if merge_mode; then
> + touch "$BACKUP"
> + if $base_present; then
> + "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
> + --default --mode=merge3 --to="$MERGED"
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE" \
> + --default --mode=merge2 --to="$MERGED"
> + fi
> + check_unchanged
> + else
> + "$merge_tool_path" "$LOCAL" "$REMOTE" \
> + --default --mode=merge2 --to="$MERGED"
> + fi
> + ;;
> + emerge)
> + if merge_mode; then
> + if $base_present; then
> + "$merge_tool_path" \
> + -f emerge-files-with-ancestor-command \
> + "$LOCAL" "$REMOTE" "$BASE" \
> + "$(basename "$MERGED")"
> + else
> + "$merge_tool_path" \
> + -f emerge-files-command \
> + "$LOCAL" "$REMOTE" \
> + "$(basename "$MERGED")"
> + fi
> + status=$?
> + else
> + "$merge_tool_path" -f emerge-files-command \
> + "$LOCAL" "$REMOTE" "$(basename "$MERGED")"
> + fi
> + ;;
> + tortoisemerge)
> + if $base_present; then
> + touch "$BACKUP"
> + "$merge_tool_path" \
> + -base:"$BASE" -mine:"$LOCAL" \
> + -theirs:"$REMOTE" -merged:"$MERGED"
> + check_unchanged
> + else
> + echo "TortoiseMerge cannot be used without a base" 1>&2
> + status=1
> + fi
> + ;;
Looks nice.
> + *)
> + if test -z "$merge_tool_cmd"; then
> + if merge_mode; then
> + status=1
> + fi
> + break
> + fi
> + if merge_mode; then
> + if test "$merge_tool_trust_exit_code" = "false"; then
> + touch "$BACKUP"
> + ( eval $merge_tool_cmd )
> + check_unchanged
> + else
> + ( eval $merge_tool_cmd )
> + fi
> + status=$?
"fi" and "status=$?" have to be swapped, else it overwrites the value
set by check_unchanged():
+ status=$?
+ fi
> + else
> + ( eval $merge_tool_cmd )
> + fi
> + ;;
> + esac
> + return $status
> +}
> +
> +guess_merge_tool () {
> + tools="ecmerge"
> + if merge_mode; then
> + tools="$tools tortoisemerge"
> + else
> + kompare=" kompare "
> + fi
> + if test -n "$DISPLAY"; then
> + if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> + tools="meld opendiff kdiff3""$kompare""tkdiff $tools"
> + tools="$tools xxdiff gvimdiff diffuse"
> + else
> + tools="opendiff kdiff3""$kompare""tkdiff xxdiff $tools"
> + tools="$tools meld gvimdiff diffuse"
> + fi
> + fi
The setting of $tools looks a bit weird. Maybe we can just ignore the
additional spaces here...
> + if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> + # $EDITOR is emacs so add emerge as a candidate
> + tools="$tools emerge vimdiff"
> + elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> + # $EDITOR is vim so add vimdiff as a candidate
> + tools="$tools vimdiff emerge"
> + else
> + tools="$tools emerge vimdiff"
> + fi
...and delete them here with something like
sed -e 's/ \+/ /g'
to have it look nice here?:
> + echo >&2 "merge tool candidates: $tools"
> +
> + # Loop over each candidate and stop when a valid merge tool is found.
> + for i in $tools
> + do
> + merge_tool_path="$(translate_merge_tool_path "$i")"
> + if type "$merge_tool_path" > /dev/null 2>&1; then
> + merge_tool="$i"
> + break
> + fi
> + done
> +
> + if test -z "$merge_tool" ; then
> + echo >&2 "No known merge resolution program available."
> + return 1
> + fi
> + echo "$merge_tool"
> +}
next prev parent reply other threads:[~2009-04-07 18:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-07 8:21 Updates to the latest {diff,merge}tool series David Aguilar
2009-04-07 8:21 ` [PATCH v2 07/14] difftool: add a -y shortcut for --no-prompt David Aguilar
2009-04-07 8:21 ` [PATCH v2 09/14] difftool: move 'git-difftool' out of contrib David Aguilar
2009-04-07 8:21 ` [PATCH v2 10/14] difftool: add various git-difftool tests David Aguilar
2009-04-07 8:21 ` [PATCH v2 11/14] difftool: add support for a difftool.prompt config variable David Aguilar
2009-04-07 8:21 ` [PATCH v3 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib David Aguilar
2009-04-07 18:11 ` Markus Heidelberg [this message]
2009-04-07 17:52 ` [PATCH v2 10/14] difftool: add various git-difftool tests Markus Heidelberg
2009-04-07 22:38 ` David Aguilar
2009-04-07 23:32 ` David Aguilar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200904072011.15503.markus.heidelberg@web.de \
--to=markus.heidelberg@web.de \
--cc=charles@hashpling.org \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).