All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR
@ 2010-07-09  5:20 Nazri Ramliy
  2010-07-09  5:20 ` [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header " Nazri Ramliy
  2010-07-09 22:30 ` [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines " Junio C Hamano
  0 siblings, 2 replies; 5+ messages in thread
From: Nazri Ramliy @ 2010-07-09  5:20 UTC (permalink / raw
  To: git; +Cc: johannes.schindelin, Nazri Ramliy

Signed-off-by: Nazri Ramliy <ayiehere@gmail.com>
---
This is in preparation of patch 2/2 in this series.

nazri.

 t/lib-rebase.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6aefe27..e1a33fc 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -4,6 +4,10 @@
 #
 # - override the commit message with $FAKE_COMMIT_MESSAGE
 # - amend the commit message with $FAKE_COMMIT_AMEND
+# - check that the non-comment text shown in the editor matches that in the
+#   file $EXPECT_NON_COMMENT_LINES. To make the comparison ignore the commit ids
+#   use the string "COMMIT_ID" in place of the sha1 in your 'expect' file and set
+#   REPLACE_COMMIT_ID=COMMIT_ID
 # - check that non-commit messages have a certain line count with $EXPECT_COUNT
 # - check the commit count in the commit message header with $EXPECT_HEADER_COUNT
 # - rewrite a rebase -i script as directed by $FAKE_LINES.
@@ -34,6 +38,17 @@ case "$1" in
 	exit
 	;;
 esac
+test -z "$EXPECT_NON_COMMENT_LINES" ||
+	(
+		grep -v '^#' < "$1" > out_non_comment.$$ &&
+		if test -n "$REPLACE_COMMIT_ID"
+		then
+			sed -i "s/^\([a-z]\+\) [0-9a-f]\+ /\1 $REPLACE_COMMIT_ID /" \
+			out_non_comment.$$
+		fi &&
+		diff "$EXPECT_NON_COMMENT_LINES" out_non_comment.$$ > /dev/null
+	) ||
+	exit
 test -z "$EXPECT_COUNT" ||
 	test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
 	exit
-- 
1.7.2.rc2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header in $GIT_EDITOR
  2010-07-09  5:20 [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR Nazri Ramliy
@ 2010-07-09  5:20 ` Nazri Ramliy
  2010-07-09 22:37   ` Junio C Hamano
  2010-07-09 22:30 ` [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines " Junio C Hamano
  1 sibling, 1 reply; 5+ messages in thread
From: Nazri Ramliy @ 2010-07-09  5:20 UTC (permalink / raw
  To: git; +Cc: johannes.schindelin, Nazri Ramliy

During "git rebase -i", when the commit headers are shown in the editor
for action (pick/squash/etc), the whitespace (if any) at the beginning
of commit headers are stripped out due to the use of "read shortsha1 rest"
for reading the output of "git rev-list".

The missing beginning whitespace do not pose any harm but this could be
annoying when you want to identify these commits to perform actions on,
for example, rewording them to remove the beginning whitespace.

Not having the whitespace makes them hard to spot, and you have to rely
on something like "git log --oneline" to identify them.

This patch:

	1. Ensures that the commit header shown in $GIT_EDITOR is
	   identical to actual commit header
	2. Add a test for it

Signed-off-by: Nazri Ramliy <ayiehere@gmail.com>
---
 git-rebase--interactive.sh    |   25 ++++++++++++++-----------
 t/t3404-rebase-interactive.sh |   13 +++++++++++++
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 31e6860..726cb6a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -889,14 +889,15 @@ first and then run 'git rebase --continue' again."
 		fi
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse --left-right --topo-order \
-			$REVISIONS | \
-			sed -n "s/^>//p" |
-		while read -r shortsha1 rest
-		do
-			if test t != "$PRESERVE_MERGES"
-			then
-				echo "pick $shortsha1 $rest" >> "$TODO"
-			else
+			$REVISIONS | sed -n "s/^>//p" > "$TODO.tmp"
+
+		if test t != "$PRESERVE_MERGES"
+		then
+			cat "$TODO.tmp" | sed "s/^/pick /" > "$TODO"
+		else
+			cat "$TODO.tmp" |
+			while read -r shortsha1 rest
+			do
 				sha1=$(git rev-parse $shortsha1)
 				if test -z "$REBASE_ROOT"
 				then
@@ -914,10 +915,12 @@ first and then run 'git rebase --continue' again."
 				if test f = "$preserve"
 				then
 					touch "$REWRITTEN"/$sha1
-					echo "pick $shortsha1 $rest" >> "$TODO"
+					grep "^$shortsha1" "$TODO.tmp" | sed "s/^/pick /" >> "$TODO"
 				fi
-			fi
-		done
+			done
+
+		fi
+		rm -f "$TODO.tmp"
 
 		# Watch for commits that been dropped by --cherry-pick
 		if test t = "$PRESERVE_MERGES"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 47ca88f..2c3a0b9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -648,4 +648,17 @@ test_expect_success 'rebase-i history with funny messages' '
 	test_cmp expect actual
 '
 
+cat >expect <<EOF
+pick COMMIT_ID   Commit header with beginning whitespace
+
+EOF
+
+test_expect_success 'preserve whitespace in commit summary' '
+	git checkout -b preserve-whitespace master &&
+	echo a >whitespace_test &&
+	git add whitespace_test &&
+	git commit -m"  Commit header with beginning whitespace" &&
+	REPLACE_COMMIT_ID=COMMIT_ID EXPECT_NON_COMMENT_LINES="expect" git rebase -i HEAD~1
+'
+
 test_done
-- 
1.7.2.rc2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR
  2010-07-09  5:20 [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR Nazri Ramliy
  2010-07-09  5:20 ` [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header " Nazri Ramliy
@ 2010-07-09 22:30 ` Junio C Hamano
  1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2010-07-09 22:30 UTC (permalink / raw
  To: Nazri Ramliy; +Cc: git, johannes.schindelin

Nazri Ramliy <ayiehere@gmail.com> writes:

> +			sed -i "s/^\([a-z]\+\) [0-9a-f]\+ /\1 $REPLACE_COMMIT_ID /" \

This is not portable. Escaping an ERE element with a backslash does not
make it suitable for use in BRE that sed uses.

Do we use in-place replacement anywhere else with sed?  I don't think it
is portable, either.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header in $GIT_EDITOR
  2010-07-09  5:20 ` [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header " Nazri Ramliy
@ 2010-07-09 22:37   ` Junio C Hamano
  2010-07-10 12:22     ` Nazri Ramliy
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2010-07-09 22:37 UTC (permalink / raw
  To: Nazri Ramliy; +Cc: git, johannes.schindelin

Nazri Ramliy <ayiehere@gmail.com> writes:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 31e6860..726cb6a 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -889,14 +889,15 @@ first and then run 'git rebase --continue' again."
>  		fi
>  		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
>  			--abbrev=7 --reverse --left-right --topo-order \
> +			$REVISIONS | sed -n "s/^>//p" > "$TODO.tmp"
> +
> +		if test t != "$PRESERVE_MERGES"
> +		then
> +			cat "$TODO.tmp" | sed "s/^/pick /" > "$TODO"

Do not cat a single file into a pipeline.

	sed "s/^/pick /" <"$TODO.tmp" >"$TODO"

> +		else
> +			cat "$TODO.tmp" |

Likewise.

> +			while read -r shortsha1 rest
> +			do
>  				sha1=$(git rev-parse $shortsha1)
>  				if test -z "$REBASE_ROOT"
>  				then
> @@ -914,10 +915,12 @@ first and then run 'git rebase --continue' again."
>  				if test f = "$preserve"
>  				then
>  					touch "$REWRITTEN"/$sha1
> +					grep "^$shortsha1" "$TODO.tmp" | sed "s/^/pick /" >> "$TODO"

Don't pipe output of grep into sed.

	sed -ne "/^$shortsha1 /s/^/pick /p" <"$TODO.tmp" >>"$TODO"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit  header in $GIT_EDITOR
  2010-07-09 22:37   ` Junio C Hamano
@ 2010-07-10 12:22     ` Nazri Ramliy
  0 siblings, 0 replies; 5+ messages in thread
From: Nazri Ramliy @ 2010-07-10 12:22 UTC (permalink / raw
  To: Junio C Hamano; +Cc: git, johannes.schindelin

On Sat, Jul 10, 2010 at 6:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nazri Ramliy <ayiehere@gmail.com> writes:
>
>> +                     sed -i "s/^\([a-z]\+\) [0-9a-f]\+ /\1 $REPLACE_COMMIT_ID /" \
>
> This is not portable. Escaping an ERE element with a backslash does not
> make it suitable for use in BRE that sed uses.
>
> Do we use in-place replacement anywhere else with sed?  I don't think it
> is portable, either.
>

And later ...

On Sat, Jul 10, 2010 at 6:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> +                     cat "$TODO.tmp" | sed "s/^/pick /" > "$TODO"
>
> Do not cat a single file into a pipeline.
>
>        sed "s/^/pick /" <"$TODO.tmp" >"$TODO"
>
>> +             else
>> +                     cat "$TODO.tmp" |
>
> Likewise.
>
[snip]
> Don't pipe output of grep into sed.
>
>        sed -ne "/^$shortsha1 /s/^/pick /p" <"$TODO.tmp" >>"$TODO"

I'll send out a v2 of this series that addresses the above issues in
better ways (i think).

nazri

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-07-10 12:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-09  5:20 [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR Nazri Ramliy
2010-07-09  5:20 ` [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header " Nazri Ramliy
2010-07-09 22:37   ` Junio C Hamano
2010-07-10 12:22     ` Nazri Ramliy
2010-07-09 22:30 ` [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines " Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.