threads / patch / 8693

patch, 2 partsMove the pick_author code to git-sh-setup

Subject: [PATCH 1/2] Move the pick_author code to git-sh-setup

## tl;dr

5 messages between Jun 23, 2007 and Jun 24, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin· Jun 23, 2007, 23:01 UTC · lore

At the moment, only git-commit uses that code, to pick the author name, email and date from a given commit.

This code will be reused in git rebase --interactive.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-commit.sh   |   30 ++----------------------------
 git-sh-setup.sh |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 28 deletions(-)
Show changes to 2 files +29 −28

git-commit.sh, git-sh-setup.sh

diff --git a/git-commit.sh b/git-commit.sh
index 5547a02..4bef375 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -483,34 +483,8 @@ fi >>"$GIT_DIR"/COMMIT_EDITMSG
 # Author
 if test '' != "$use_commit"
 then
-	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)
-	set_author_env=`git show -s --pretty=raw --encoding="$encoding" "$use_commit" |
-	LANG=C LC_ALL=C sed -ne "$pick_author_script"`
-	eval "$set_author_env"
-	export GIT_AUTHOR_NAME
-	export GIT_AUTHOR_EMAIL
-	export GIT_AUTHOR_DATE
+	eval $(get_author_ident_from_commit "$use_commit")
+	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
 fi
 if test '' != "$force_author"
 then
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index f24c7f2..d861db3 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,6 +53,33 @@ require_work_tree () {
 	die "fatal: $0 cannot be used without a working tree."
 }
 
+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"
+}
+
 if [ -z "$LONG_USAGE" ]
 then
 	LONG_USAGE="Usage: $0 $USAGE"

-- 
1.5.2.2.279.g74fb3
Junio C Hamano· Jun 24, 2007, 06:39 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/2] Move the pick_author code to git-sh-setup

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 5 quoted lines
> -	set_author_env=`git show -s --pretty=raw --encoding="$encoding" "$use_commit" |
> -	LANG=C LC_ALL=C sed -ne "$pick_author_script"`
> -	eval "$set_author_env"
> ...
> +	eval $(get_author_ident_from_commit "$use_commit")

Are you sure about this part of the change? I suspect that you are losing IFS by not dq'ing the argument you give to the eval.

    #!/bin/sh
    test1 () {
        echo "$1=' d  e'"
    }
    eval $(test1 A)
    eval "$(test1 B)"
    echo "A=$A"
    echo "B=$B"
Junio C Hamano· Jun 24, 2007, 07:09 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Move the pick_author code to git-sh-setup

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> -	set_author_env=`git show -s --pretty=raw --encoding="$encoding" "$use_commit" |
>> -	LANG=C LC_ALL=C sed -ne "$pick_author_script"`
>> -	eval "$set_author_env"
>> ...
>> +	eval $(get_author_ident_from_commit "$use_commit")
>
> Are you sure about this part of the change?  I suspect that you
> are losing IFS by not dq'ing the argument you give to the eval.
>
> ...

If you care about your data in your variable and do not want word-splitting at $IFS to happen, you should always dq your variable. A quick rule of thumb is that the only place that you can get away by not quoting is straight assignment to another variable, like so:

	var='a  b c '
        another=$var	;# another="$var" is fine but unnecessary.
Johannes Schindelin· Jun 24, 2007, 10:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Move the pick_author code to git-sh-setup

Hi,
On Sat, 23 Jun 2007, Junio C Hamano wrote:
Show 9 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > -	set_author_env=`git show -s --pretty=raw --encoding="$encoding" "$use_commit" |
> > -	LANG=C LC_ALL=C sed -ne "$pick_author_script"`
> > -	eval "$set_author_env"
> > ...
> > +	eval $(get_author_ident_from_commit "$use_commit")
> 
> Are you sure about this part of the change?
No, I am not.
> I suspect that you are losing IFS by not dq'ing the argument you give to 
> the eval.

That is well possible. Quoting in shell is such a hassle, and I never seem to get it right.

Therefore I did a minimal test, namely committing with another GIT_AUTHOR_NAME (which has spaces in it), and then commiting again, with "-c HEAD". Which did what I expected: the second commit had the same author name as the first one.

So I thought that it was okay.

Ciao, Dscho

Junio C Hamano· Jun 24, 2007, 11:14 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/2] Move the pick_author code to git-sh-setup

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 6 quoted lines
> That is well possible. Quoting in shell is such a hassle, and I never seem 
> to get it right.
>
> Therefore I did a minimal test,...
>
> So I thought that it was okay.

Try the small test script in the message you are responding to. It gives me:

        $ sh ./1.sh
        A= d e
        B= d  e

A quick rule of thumb is that the only place that you can get away by not quoting is straight assignment to another variable, like so:

	var='a  b c '
        another=$var	;# another="$var" is fine but unnecessary.
Similarly for
	another=$(some command)
which is Ok not to dquote.

← back to recent threads