[PATCH 2/2] rebase -i: use some kind of config file to save author information

Subsystems: the rest

DORMANTno replies

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 2/2] rebase -i: use some kind of config file to save author information

From: Christian Couder <hidden>
Date: 2016-06-15 22:46:58

This is better than saving in a shell script, because it will make
it much easier to port "rebase -i" to C. This also removes some sed
regexps and some "eval"s.

Signed-off-by: Christian Couder <redacted>
---
 git-rebase--interactive.sh |   78 +++++++++++++++++++++++---------------------
 1 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 92e2523..73f888a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh

@@ -40,6 +40,7 @@ MSG="$DOTEST"/message
 SQUASH_MSG="$DOTEST"/message-squash
 REWRITTEN="$DOTEST"/rewritten
 DROPPED="$DOTEST"/dropped
+SAVE_AUTHOR_INFO="$DOTEST"/save-author-info
 PRESERVE_MERGES=
 STRATEGY=
 ONTO=

@@ -117,30 +118,31 @@ mark_action_done () {
 }
 
 get_author_ident_from_commit () {
-	pick_author_script='
-	/^author /{
-		s/'\''/'\''\\'\'\''/g
-		h
-		s/^author \([^<]*\) <[^>]*> .*$/\1/
-		s/'\''/'\''\'\'\''/g
-		s/.*/GIT_AUTHOR_NAME='\''&'\''/p
-
-		g
-		s/^author [^<]* <\([^>]*\)> .*$/\1/
-		s/'\''/'\''\'\'\''/g
-		s/.*/GIT_AUTHOR_EMAIL='\''&'\''/p
-
-		g
-		s/^author [^<]* <[^>]*> \(.*\)$/\1/
-		s/'\''/'\''\'\'\''/g
-		s/.*/GIT_AUTHOR_DATE='\''&'\''/p
-
-		q
-	}
-	'
-	encoding=$(git config i18n.commitencoding || echo UTF-8)
-	git show -s --pretty=raw --encoding="$encoding" "$1" -- |
-	LANG=C LC_ALL=C sed -ne "$pick_author_script"
+	encoding=$(git config i18n.commitencoding || echo UTF-8) &&
+	author_ident_name=$(git show -s --pretty="format:%an" \
+		--encoding="$encoding" "$1" --) &&
+	author_ident_mail=$(git show -s --pretty="format:%ae" \
+		--encoding="$encoding" "$1" --) &&
+	author_ident_date=$(git show -s --pretty="format:%ai" \
+		--encoding="$encoding" "$1" --)
+}
+
+save_author_ident () {
+	GIT_CONFIG="$SAVE_AUTHOR_INFO" git config rebase.author.name \
+		"$author_ident_name" &&
+	GIT_CONFIG="$SAVE_AUTHOR_INFO" git config rebase.author.mail \
+		"$author_ident_mail" &&
+	GIT_CONFIG="$SAVE_AUTHOR_INFO" git config rebase.author.date \
+		"$author_ident_date"
+}
+
+load_author_ident () {
+	GIT_AUTHOR_NAME=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.name) &&
+	GIT_AUTHOR_EMAIL=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.mail) &&
+	GIT_AUTHOR_DATE=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.date)
 }
 
 make_patch () {

@@ -158,8 +160,10 @@ make_patch () {
 	esac > "$DOTEST"/patch
 	test -f "$DOTEST"/message ||
 		git cat-file commit "$1" | sed "1,/^$/d" > "$DOTEST"/message
-	test -f "$DOTEST"/author-script ||
-		get_author_ident_from_commit "$1" > "$DOTEST"/author-script
+	test -f "$SAVE_AUTHOR_INFO" || {
+		get_author_ident_from_commit "$1"
+		save_author_ident
+	}
 }
 
 die_with_patch () {

@@ -294,8 +298,10 @@ pick_one_preserving_merges () {
 			test "a$1" = a-n && die "Refusing to squash a merge: $sha1"
 
 			# redo merge
-			author_script=$(get_author_ident_from_commit $sha1)
-			eval "$author_script"
+			get_author_ident_from_commit $sha1
+			GIT_AUTHOR_NAME="$author_ident_name"
+			GIT_AUTHOR_EMAIL="$author_ident_mail"
+			GIT_AUTHOR_DATE="$author_ident_date"
 			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}

@@ -353,8 +359,7 @@ peek_next_command () {
 }
 
 do_next () {
-	rm -f "$DOTEST"/message "$DOTEST"/author-script \
-		"$DOTEST"/amend || exit
+	rm -f "$DOTEST"/message "$DOTEST"/amend "$SAVE_AUTHOR_INFO" || exit
 	read command sha1 rest < "$TODO"
 	case "$command" in
 	'#'*|''|noop)

@@ -395,7 +400,7 @@ do_next () {
 		mark_action_done
 		make_squash_message $sha1 > "$MSG"
 		failed=f
-		author_script=$(get_author_ident_from_commit HEAD)
+		get_author_ident_from_commit HEAD
 		output git reset --soft HEAD^
 		pick_one -n $sha1 || failed=t
 		case "$(peek_next_command)" in

@@ -414,14 +419,13 @@ do_next () {
 			rm -f "$GIT_DIR"/MERGE_MSG || exit
 			;;
 		esac
-		echo "$author_script" > "$DOTEST"/author-script
+		save_author_ident
 		if test $failed = f
 		then
 			# This is like --amend, but with a different message
-			eval "$author_script"
-			GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
-			GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
-			GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
+			GIT_AUTHOR_NAME="$author_ident_name" \
+			GIT_AUTHOR_EMAIL="$author_ident_mail" \
+			GIT_AUTHOR_DATE="$author_ident_date" \
 			$USE_OUTPUT git commit --no-verify \
 				$MSG_OPT "$EDIT_OR_FILE" || failed=t
 		fi

@@ -536,7 +540,7 @@ do
 		then
 			: Nothing to commit -- skip this
 		else
-			. "$DOTEST"/author-script ||
+			load_author_ident ||
 				die "Cannot find the author identity"
 			amend=
 			if test -f "$DOTEST"/amend

-- 
1.6.3.GIT

Re: [PATCH 2/2] rebase -i: use some kind of config file to save author information

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:46:58

Christian Couder [off-list ref] writes:

Subject: [PATCH 2/2] rebase -i: use some kind of config file to save author information

Errr... "kind of" config file?  I'd say use config file format for
saving author information.

This is better than saving in a shell script, because it will make
it much easier to port "rebase -i" to C. This also removes some sed
regexps and some "eval"s.

Would it?  Well, I guess that at least we will avoid problems with
shell quoting and shell variable expansion rules.

quoted hunk

Signed-off-by: Christian Couder <redacted>
---
 git-rebase--interactive.sh |   78 +++++++++++++++++++++++---------------------
 1 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
+save_author_ident () {
+	GIT_CONFIG="$SAVE_AUTHOR_INFO" git config rebase.author.name \
+		"$author_ident_name" &&
+	GIT_CONFIG="$SAVE_AUTHOR_INFO" git config rebase.author.mail \
+		"$author_ident_mail" &&
+	GIT_CONFIG="$SAVE_AUTHOR_INFO" git config rebase.author.date \
+		"$author_ident_date"
+}
+
+load_author_ident () {
+	GIT_AUTHOR_NAME=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.name) &&
+	GIT_AUTHOR_EMAIL=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.mail) &&
+	GIT_AUTHOR_DATE=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.date)
 }

Why not use --file=<filename> option of git-config instead?
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 2/2] rebase -i: use some kind of config file to save author information

From: Christian Couder <hidden>
Date: 2016-06-15 22:46:58

On Saturday 20 June 2009, Jakub Narebski wrote:

Christian Couder [off-list ref] writes:

quoted

Subject: [PATCH 2/2] rebase -i: use some kind of config file to save
author information

Errr... "kind of" config file?  I'd say use config file format for
saving author information.

Ok, I changed this in the patch series I just sent.

quoted

This is better than saving in a shell script, because it will make
it much easier to port "rebase -i" to C. This also removes some sed
regexps and some "eval"s.

Would it?  Well, I guess that at least we will avoid problems with
shell quoting and shell variable expansion rules.

Yeah.

quoted

+load_author_ident () {
+	GIT_AUTHOR_NAME=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.name) &&
+	GIT_AUTHOR_EMAIL=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.mail) &&
+	GIT_AUTHOR_DATE=$(GIT_CONFIG="$SAVE_AUTHOR_INFO" \
+		git config rebase.author.date)
 }

Why not use --file=<filename> option of git-config instead?

That's a good idea. I changed that too.

Thanks,
Christian.

Re: [PATCH 2/2] rebase -i: use some kind of config file to save author information

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:58

Hi,

On Sat, 20 Jun 2009, Christian Couder wrote:

This is better than saving in a shell script, because it will make
it much easier to port "rebase -i" to C. This also removes some sed
regexps and some "eval"s.

It will not make it easier to port "rebase -i" to C, as this is an 
internal file.  The user is not supposed to touch it at all.  Only "rebase 
-i".  So it would be very easy to just use a different on-disk format when 
turning "rebase -i" into a builtin.

Ciao,
Dscho

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help