Re: [PATCH] Replace misleading message during interactive rebasing
- From
Wincent Colaiuta <win@wincent.com>
- Date
- Nov 28, 2007, 08:26 UTC
- Message-ID
- <47D3817B-AA0D-4949-99ED-C36611604D5E@wincent.com>
- In-Reply-To
- <7vve7n9eaa.fsf@gitster.siamese.dyndns.org>
El 28/11/2007, a las 5:37, Junio C Hamano escribió:
Show 11 quoted lines
> Johannes Sixt <j.sixt@viscovery.net> writes: > >> Wincent Colaiuta schrieb: >>> + export _GIT_CHERRY_PICK_HELP="run 'git rebase --continue'" >> >> Isn't this a bashism? > > Being an old-fashioned shell programmer myself, "export VAR=VAL" does > raise my eyebrows. It is in POSIX but are there shells that we do > support (dash, bash, ksh -- /bin/sh on Solaris does not count) that > want their exports old-fashioned way?
As I noted earlier in thread, the idiom is used elsewhere in Git already. Below is a list of the instances I found where shell scripts use the idiom. There are also other instances (documentation, test scripts, even a tcl file) where the idiom is used. As far as its suitability I only have directory experience with Bash, and it's always worked in all versions that I've tried in the 2.x and 3.x range. The other shells, I don't know.
In any case, below I'll paste in a revised patch that doesn't use the export FOO=... idiom. I can also submit another patch changing the other usages of the idiom elsewhere in the tree, but I'd need guidance on how far you'd want to go (only the Git commands? also the tests? don't worry about the docs? etc).
El 27/11/2007, a las 9:49, Wincent Colaiuta escribió:
Show 24 quoted lines
> I wondered that myself before submitting the patch, but then saw > that the same pattern was used at other places as well: > > git-clone.sh: > > W=$(cd "$GIT_WORK_TREE" && pwd) && export GIT_WORK_TREE="$W" > > git-filter-branch.sh: > > export GIT_INDEX_FILE="$(pwd)/../index" > export GIT_COMMIT=$commit > export GIT_COMMIT="$sha1" > > git-quiltimport.sh: > > export GIT_AUTHOR_NAME=$(sed -ne 's/Author: //p' "$tmp_info") > export GIT_AUTHOR_EMAIL=$(sed -ne 's/Email: //p' "$tmp_info") > export GIT_AUTHOR_DATE=$(sed -ne 's/Date: //p' "$tmp_info") > export SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info") > > So if this is a problem, those sites will need to be changed as well. > > Cheers, > Wincent
Here's the revised patch: --------8<--------- From cd11e1355011796fe16d0eb7116fae4070f2a30d Mon Sep 17 00:00:00 2001 From: Wincent Colaiuta <win@wincent.com> Date: Mon, 26 Nov 2007 13:35:46 +0100 Subject: [PATCH] Replace misleading message during interactive rebasing
git-rebase--interactive uses git-cherry-pick under the covers to reorder commits, which in turn means that in the event of a conflict a message will be shown advising the user to commit the results and use the -c switch to retain authorship after fixing the conflict.
The message is misleading because what the user really needs to do is run "git rebase --continue"; the committing is handled by git-rebase and the authorship of the commit message is retained automatically.
We solve this problem by using an environment variable to communicate to git-cherry-pick that rebasing is underway and replace the misleading error message with a more helpful one.
Signed-off-by: Wincent Colaiuta <win@wincent.com> --- builtin-revert.c | 8 +++++--- git-rebase--interactive.sh | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c index a0586f9..5a57574 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -229,7 +229,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) unsigned char head[20]; struct commit *base, *next, *parent; int i; - char *oneline, *reencoded_message = NULL; + char *oneline, *reencoded_message = NULL, *help_message; const char *message, *encoding; const char *defmsg = xstrdup(git_path("MERGE_MSG")); @@ -352,11 +352,13 @@ static int revert_or_cherry_pick(int argc, const char **argv) } if (close(msg_fd) || commit_lock_file(&msg_file) < 0) die ("Error wrapping up %s", defmsg); + help_message = getenv("_GIT_CHERRY_PICK_HELP"); fprintf(stderr, "Automatic %s failed. " "After resolving the conflicts,\n" "mark the corrected paths with 'git add <paths>' " - "and commit the result.\n", me); - if (action == CHERRY_PICK) { + "and %s.\n", me, + help_message ? help_message : "commit the result"); + if (action == CHERRY_PICK && !help_message) { fprintf(stderr, "When commiting, use the option " "'-c %s' to retain authorship and message.\n", find_unique_abbrev(commit->object.sha1, diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index bf44b6a..3552c89 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -117,6 +117,8 @@ pick_one () { sha1=$(git rev-parse --short $sha1) output warn Fast forward to $sha1 else + _GIT_CHERRY_PICK_HELP="run 'git rebase --continue'" + export _GIT_CHERRY_PICK_HELP && output git cherry-pick "$@" fi } @@ -187,6 +189,8 @@ pick_one_preserving_merges () { fi ;; *) + _GIT_CHERRY_PICK_HELP="run 'git rebase --continue'" + export _GIT_CHERRY_PICK_HELP && output git cherry-pick "$@" || die_with_patch $sha1 "Could not pick $sha1" ;;
-- 1.5.3.6.953.gdffc