[PATCH] rebase -i: respect core.commentchar

[PATCH] rebase -i: respect core.commentchar

[John Keeping]

Commit eff80a9 (Allow custom "comment char") introduced a custom comment
character for commit messages but did not teach git-rebase--interactive
to use it.

Change git-rebase--interactive to read core.commentchar and use its
value when generating commit messages and for the todo list.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git-rebase--interactive.sh    | 85 ++++++++++++++++++++++---------------------
 t/t3404-rebase-interactive.sh | 16 ++++++++
 2 files changed, 60 insertions(+), 41 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8ed7fcc..8a37bc1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -80,6 +80,9 @@ rewritten_pending="$state_dir"/rewritten-pending
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 
+comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
+: ${comment_char:=#}
+
 warn () {
 	printf '%s\n' "$*" >&2
 }
@@ -105,8 +108,8 @@ mark_action_done () {
 	sed -e 1q < "$todo" >> "$done"
 	sed -e 1d < "$todo" >> "$todo".new
 	mv -f "$todo".new "$todo"
-	new_count=$(sane_grep -c '^[^#]' < "$done")
-	total=$(($new_count+$(sane_grep -c '^[^#]' < "$todo")))
+	new_count=$(sane_grep -c "^[^${comment_char}]" < "$done")
+	total=$(($new_count+$(sane_grep -c "^[^${comment_char}]" < "$todo")))
 	if test "$last_count" != "$new_count"
 	then
 		last_count=$new_count
@@ -116,19 +119,19 @@ mark_action_done () {
 }
 
 append_todo_help () {
-	cat >> "$todo" << EOF
-#
-# Commands:
-#  p, pick = use commit
-#  r, reword = use commit, but edit the commit message
-#  e, edit = use commit, but stop for amending
-#  s, squash = use commit, but meld into previous commit
-#  f, fixup = like "squash", but discard this commit's log message
-#  x, exec = run command (the rest of the line) using shell
-#
-# These lines can be re-ordered; they are executed from top to bottom.
-#
-# If you remove a line here THAT COMMIT WILL BE LOST.
+	sed -e "s/^/$comment_char /" >>"$todo" <<EOF
+
+Commands:
+ p, pick = use commit
+ r, reword = use commit, but edit the commit message
+ e, edit = use commit, but stop for amending
+ s, squash = use commit, but meld into previous commit
+ f, fixup = like "squash", but discard this commit's log message
+ x, exec = run command (the rest of the line) using shell
+
+These lines can be re-ordered; they are executed from top to bottom.
+
+If you remove a line here THAT COMMIT WILL BE LOST.
 EOF
 }
 
@@ -179,7 +182,7 @@ die_abort () {
 }
 
 has_action () {
-	sane_grep '^[^#]' "$1" >/dev/null
+	sane_grep "^[^${comment_char}]" "$1" >/dev/null
 }
 
 is_empty_commit() {
@@ -363,10 +366,10 @@ update_squash_messages () {
 	if test -f "$squash_msg"; then
 		mv "$squash_msg" "$squash_msg".bak || exit
 		count=$(($(sed -n \
-			-e "1s/^# This is a combination of \(.*\) commits\./\1/p" \
+			-e "1s/^. This is a combination of \(.*\) commits\./\1/p" \
 			-e "q" < "$squash_msg".bak)+1))
 		{
-			echo "# This is a combination of $count commits."
+			echo "$comment_char This is a combination of $count commits."
 			sed -e 1d -e '2,/^./{
 				/^$/d
 			}' <"$squash_msg".bak
@@ -375,8 +378,8 @@ update_squash_messages () {
 		commit_message HEAD > "$fixup_msg" || die "Cannot write $fixup_msg"
 		count=2
 		{
-			echo "# This is a combination of 2 commits."
-			echo "# The first commit's message is:"
+			echo "$comment_char This is a combination of 2 commits."
+			echo "$comment_char The first commit's message is:"
 			echo
 			cat "$fixup_msg"
 		} >"$squash_msg"
@@ -385,21 +388,21 @@ update_squash_messages () {
 	squash)
 		rm -f "$fixup_msg"
 		echo
-		echo "# This is the $(nth_string $count) commit message:"
+		echo "$comment_char This is the $(nth_string $count) commit message:"
 		echo
 		commit_message $2
 		;;
 	fixup)
 		echo
-		echo "# The $(nth_string $count) commit message will be skipped:"
+		echo "$comment_char The $(nth_string $count) commit message will be skipped:"
 		echo
-		commit_message $2 | sed -e 's/^/#	/'
+		commit_message $2 | sed -e "s/^/$comment_char	/"
 		;;
 	esac >>"$squash_msg"
 }
 
 peek_next_command () {
-	sed -n -e "/^#/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
+	sed -n -e "/^$comment_char/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
 }
 
 # A squash/fixup has failed.  Prepare the long version of the squash
@@ -464,7 +467,7 @@ do_next () {
 	rm -f "$msg" "$author_script" "$amend" || exit
 	read -r command sha1 rest < "$todo"
 	case "$command" in
-	'#'*|''|noop)
+	$comment_char*|''|noop)
 		mark_action_done
 		;;
 	pick|p)
@@ -803,15 +806,15 @@ skip)
 	do_rest
 	;;
 edit-todo)
-	sed -e '/^#/d' < "$todo" > "$todo".new
+	sed -e "/^$comment_char/d" < "$todo" > "$todo".new
 	mv -f "$todo".new "$todo"
 	append_todo_help
-	cat >> "$todo" << EOF
-#
-# You are editing the todo file of an ongoing interactive rebase.
-# To continue rebase after editing, run:
-#     git rebase --continue
-#
+	sed -e "s/^/$comment_char /" >>"$todo" <<EOF
+
+You are editing the todo file of an ongoing interactive rebase.
+To continue rebase after editing, run:
+    git rebase --continue
+
 EOF
 
 	git_sequence_editor "$todo" ||
@@ -881,7 +884,7 @@ do
 
 	if test -z "$keep_empty" && is_empty_commit $shortsha1 && ! is_merge_commit $shortsha1
 	then
-		comment_out="# "
+		comment_out="$comment_char "
 	else
 		comment_out=
 	fi
@@ -942,20 +945,20 @@ test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
 test -n "$cmd" && add_exec_commands "$todo"
 
-cat >> "$todo" << EOF
-
-# Rebase $shortrevisions onto $shortonto
+echo >>"$todo"
+sed -e "s/^/$comment_char /" >> "$todo" << EOF
+Rebase $shortrevisions onto $shortonto
 EOF
 append_todo_help
-cat >> "$todo" << EOF
-#
-# However, if you remove everything, the rebase will be aborted.
-#
+sed -e "s/^/$comment_char /" >> "$todo" << EOF
+
+However, if you remove everything, the rebase will be aborted.
+
 EOF
 
 if test -z "$keep_empty"
 then
-	echo "# Note that empty commits are commented out" >>"$todo"
+	echo "$comment_char Note that empty commits are commented out" >>"$todo"
 fi
 
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8462be1..1043cdc 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -934,4 +934,20 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '
 	test L = $(git cat-file commit HEAD | sed -ne \$p)
 '
 
+test_expect_success 'rebase -i respects core.commentchar' '
+	git reset --hard &&
+	git checkout E^0 &&
+	git config core.commentchar \; &&
+	test_when_finished "git config --unset core.commentchar" &&
+	cat >comment-lines.sh <<EOF &&
+#!$SHELL_PATH
+sed -e "2,\$ s/^/;/" "\$1" >"\$1".tmp
+mv "\$1".tmp "\$1"
+EOF
+	chmod a+x comment-lines.sh &&
+	test_set_editor "$(pwd)/comment-lines.sh" &&
+	git rebase -i B &&
+	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
+'
+
 test_done
-- 
1.8.1.2

Re: [PATCH] rebase -i: respect core.commentchar

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> Commit eff80a9 (Allow custom "comment char") introduced a custom comment
> character for commit messages but did not teach git-rebase--interactive
> to use it.
>
> Change git-rebase--interactive to read core.commentchar and use its
> value when generating commit messages and for the todo list.
>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---

It always is better late than never, but I would have appreciated
this was caught while the topic was still in 'next'.  That is the
whole point of cooking in-flight topics in 'next'.

Yikes....   and thanks.

>  git-rebase--interactive.sh    | 85 ++++++++++++++++++++++---------------------
>  t/t3404-rebase-interactive.sh | 16 ++++++++
>  2 files changed, 60 insertions(+), 41 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 8ed7fcc..8a37bc1 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -80,6 +80,9 @@ rewritten_pending="$state_dir"/rewritten-pending
>  GIT_CHERRY_PICK_HELP="$resolvemsg"
>  export GIT_CHERRY_PICK_HELP
>  
> +comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> +: ${comment_char:=#}

Hmm, somewhat ugly.  I wonder if we can do this without pipe and cut.

> @@ -105,8 +108,8 @@ mark_action_done () {
>  	sed -e 1q < "$todo" >> "$done"
>  	sed -e 1d < "$todo" >> "$todo".new
>  	mv -f "$todo".new "$todo"
> -	new_count=$(sane_grep -c '^[^#]' < "$done")
> -	total=$(($new_count+$(sane_grep -c '^[^#]' < "$todo")))
> +	new_count=$(sane_grep -c "^[^${comment_char}]" < "$done")
> +	total=$(($new_count+$(sane_grep -c "^[^${comment_char}]" < "$todo")))

I do not think we would want to worry about comment_char being a
funny character that can possibly interfere with regexp.  Can't we
do this with "git stripspace" piped to "wc -l" or something?

> @@ -116,19 +119,19 @@ mark_action_done () {
>  }
>  
>  append_todo_help () {
> -	cat >> "$todo" << EOF
> -#
> -# Commands:
> -#  p, pick = use commit
> -#  r, reword = use commit, but edit the commit message
> -#  e, edit = use commit, but stop for amending
> -#  s, squash = use commit, but meld into previous commit
> -#  f, fixup = like "squash", but discard this commit's log message
> -#  x, exec = run command (the rest of the line) using shell
> -#
> -# These lines can be re-ordered; they are executed from top to bottom.
> -#
> -# If you remove a line here THAT COMMIT WILL BE LOST.
> +	sed -e "s/^/$comment_char /" >>"$todo" <<EOF

When $comment_char is slash or backslash this will break.
Perhaps "stripspace --comment-lines" can be used here.

> +
> +Commands:
> + p, pick = use commit
> + r, reword = use commit, but edit the commit message
> + e, edit = use commit, but stop for amending
> + s, squash = use commit, but meld into previous commit
> + f, fixup = like "squash", but discard this commit's log message
> + x, exec = run command (the rest of the line) using shell
> +
> +These lines can be re-ordered; they are executed from top to bottom.
> +
> +If you remove a line here THAT COMMIT WILL BE LOST.
>  EOF
>  }
>  
> @@ -179,7 +182,7 @@ die_abort () {
>  }
>  
>  has_action () {
> -	sane_grep '^[^#]' "$1" >/dev/null
> +	sane_grep "^[^${comment_char}]" "$1" >/dev/null

Likewise.

> @@ -363,10 +366,10 @@ update_squash_messages () {
>  	if test -f "$squash_msg"; then
>  		mv "$squash_msg" "$squash_msg".bak || exit
>  		count=$(($(sed -n \
> -			-e "1s/^# This is a combination of \(.*\) commits\./\1/p" \
> +			-e "1s/^. This is a combination of \(.*\) commits\./\1/p" \

This one is safe.

>  			-e "q" < "$squash_msg".bak)+1))
>  		{
> -			echo "# This is a combination of $count commits."
> +			echo "$comment_char This is a combination of $count commits."

But you need to do "printf" to be safe here, I think, for comment_char='\'.

> @@ -375,8 +378,8 @@ update_squash_messages () {
>  		commit_message HEAD > "$fixup_msg" || die "Cannot write $fixup_msg"
>  		count=2
>  		{
> -			echo "# This is a combination of 2 commits."
> -			echo "# The first commit's message is:"
> +			echo "$comment_char This is a combination of 2 commits."
> +			echo "$comment_char The first commit's message is:"

Likewise.

> @@ -385,21 +388,21 @@ update_squash_messages () {
>  	squash)
>  		rm -f "$fixup_msg"
>  		echo
> -		echo "# This is the $(nth_string $count) commit message:"
> +		echo "$comment_char This is the $(nth_string $count) commit message:"

Likewise.

>  		echo
>  		commit_message $2
>  		;;
>  	fixup)
>  		echo
> -		echo "# The $(nth_string $count) commit message will be skipped:"
> +		echo "$comment_char The $(nth_string $count) commit message will be skipped:"
>  		echo
> -		commit_message $2 | sed -e 's/^/#	/'
> +		commit_message $2 | sed -e "s/^/$comment_char	/"

Likewise.

>  peek_next_command () {
> -	sed -n -e "/^#/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
> +	sed -n -e "/^$comment_char/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
>  }

Likewise.

> @@ -464,7 +467,7 @@ do_next () {
>  	rm -f "$msg" "$author_script" "$amend" || exit
>  	read -r command sha1 rest < "$todo"
>  	case "$command" in
> -	'#'*|''|noop)
> +	$comment_char*|''|noop)

This is OK.

> @@ -803,15 +806,15 @@ skip)
>  	do_rest
>  	;;
>  edit-todo)
> -	sed -e '/^#/d' < "$todo" > "$todo".new
> +	sed -e "/^$comment_char/d" < "$todo" > "$todo".new

Unsafe.

>  	mv -f "$todo".new "$todo"
>  	append_todo_help
> -	cat >> "$todo" << EOF
> -#
> -# You are editing the todo file of an ongoing interactive rebase.
> -# To continue rebase after editing, run:
> -#     git rebase --continue
> -#
> +	sed -e "s/^/$comment_char /" >>"$todo" <<EOF

Unsafe.

> +
> +You are editing the todo file of an ongoing interactive rebase.
> +To continue rebase after editing, run:
> +    git rebase --continue
> +
>  EOF
>  
>  	git_sequence_editor "$todo" ||
> @@ -881,7 +884,7 @@ do
>  
>  	if test -z "$keep_empty" && is_empty_commit $shortsha1 && ! is_merge_commit $shortsha1
>  	then
> -		comment_out="# "
> +		comment_out="$comment_char "

OK.

>  	else
>  		comment_out=
>  	fi
> @@ -942,20 +945,20 @@ test -s "$todo" || echo noop >> "$todo"
>  test -n "$autosquash" && rearrange_squash "$todo"
>  test -n "$cmd" && add_exec_commands "$todo"
>  
> -cat >> "$todo" << EOF
> -
> -# Rebase $shortrevisions onto $shortonto
> +echo >>"$todo"
> +sed -e "s/^/$comment_char /" >> "$todo" << EOF

Unsafe.

> +Rebase $shortrevisions onto $shortonto
>  EOF
>  append_todo_help
> -cat >> "$todo" << EOF
> -#
> -# However, if you remove everything, the rebase will be aborted.
> -#
> +sed -e "s/^/$comment_char /" >> "$todo" << EOF

Unsafe.

> +
> +However, if you remove everything, the rebase will be aborted.
> +
>  EOF
>  
>  if test -z "$keep_empty"
>  then
> -	echo "# Note that empty commits are commented out" >>"$todo"
> +	echo "$comment_char Note that empty commits are commented out" >>"$todo"
>  fi
>  
>  
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 8462be1..1043cdc 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -934,4 +934,20 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '
>  	test L = $(git cat-file commit HEAD | sed -ne \$p)
>  '
>  
> +test_expect_success 'rebase -i respects core.commentchar' '
> +	git reset --hard &&
> +	git checkout E^0 &&
> +	git config core.commentchar \; &&

Try setting it to '\' or '/' or '-'; they may catch some more breakages.

> +	test_when_finished "git config --unset core.commentchar" &&
> +	cat >comment-lines.sh <<EOF &&
> +#!$SHELL_PATH
> +sed -e "2,\$ s/^/;/" "\$1" >"\$1".tmp
> +mv "\$1".tmp "\$1"
> +EOF
> +	chmod a+x comment-lines.sh &&
> +	test_set_editor "$(pwd)/comment-lines.sh" &&
> +	git rebase -i B &&
> +	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> +'
> +
>  test_done

Re: [PATCH] rebase -i: respect core.commentchar

[John Keeping]

On Mon, Feb 11, 2013 at 12:17:01PM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> > +comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> > +: ${comment_char:=#}
> 
> Hmm, somewhat ugly.  I wonder if we can do this without pipe and cut.

Yeah, but I can't see a better way of doing this if we want to mimic the
behaviour of the C code in taking only the first character of the
configured value.

> > @@ -105,8 +108,8 @@ mark_action_done () {
> >  	sed -e 1q < "$todo" >> "$done"
> >  	sed -e 1d < "$todo" >> "$todo".new
> >  	mv -f "$todo".new "$todo"
> > -	new_count=$(sane_grep -c '^[^#]' < "$done")
> > -	total=$(($new_count+$(sane_grep -c '^[^#]' < "$todo")))
> > +	new_count=$(sane_grep -c "^[^${comment_char}]" < "$done")
> > +	total=$(($new_count+$(sane_grep -c "^[^${comment_char}]" < "$todo")))
> 
> I do not think we would want to worry about comment_char being a
> funny character that can possibly interfere with regexp.  Can't we
> do this with "git stripspace" piped to "wc -l" or something?

I didn't know about "git stripspace", it does make a lot of this
significantly safer.  I'll work up a new version that uses that instead
of grep and with printf used where necessary.

> > @@ -116,19 +119,19 @@ mark_action_done () {
> >  }
> >  
> >  append_todo_help () {
> > -	cat >> "$todo" << EOF
> > -#
> > -# Commands:
> > -#  p, pick = use commit
> > -#  r, reword = use commit, but edit the commit message
> > -#  e, edit = use commit, but stop for amending
> > -#  s, squash = use commit, but meld into previous commit
> > -#  f, fixup = like "squash", but discard this commit's log message
> > -#  x, exec = run command (the rest of the line) using shell
> > -#
> > -# These lines can be re-ordered; they are executed from top to bottom.
> > -#
> > -# If you remove a line here THAT COMMIT WILL BE LOST.
> > +	sed -e "s/^/$comment_char /" >>"$todo" <<EOF
> 
> When $comment_char is slash or backslash this will break.
> Perhaps "stripspace --comment-lines" can be used here.

Not in this case - this is adding the comment char in front of each
line.  Is there a better option than this?

	while read -r line
	do
		printf '%s %s\n' "$comment_char" "$line"
	done >> "$todo" <<EOF
	...
	EOF

> > +
> > +Commands:
> > + p, pick = use commit
> > + r, reword = use commit, but edit the commit message
> > + e, edit = use commit, but stop for amending
> > + s, squash = use commit, but meld into previous commit
> > + f, fixup = like "squash", but discard this commit's log message
> > + x, exec = run command (the rest of the line) using shell
> > +
> > +These lines can be re-ordered; they are executed from top to bottom.
> > +
> > +If you remove a line here THAT COMMIT WILL BE LOST.
> >  EOF
> >  }
> >  
> > @@ -179,7 +182,7 @@ die_abort () {
> >  }
> >  
> >  has_action () {
> > -	sane_grep '^[^#]' "$1" >/dev/null
> > +	sane_grep "^[^${comment_char}]" "$1" >/dev/null
> 
> Likewise.
> 
> > @@ -363,10 +366,10 @@ update_squash_messages () {
> >  	if test -f "$squash_msg"; then
> >  		mv "$squash_msg" "$squash_msg".bak || exit
> >  		count=$(($(sed -n \
> > -			-e "1s/^# This is a combination of \(.*\) commits\./\1/p" \
> > +			-e "1s/^. This is a combination of \(.*\) commits\./\1/p" \
> 
> This one is safe.
> 
> >  			-e "q" < "$squash_msg".bak)+1))
> >  		{
> > -			echo "# This is a combination of $count commits."
> > +			echo "$comment_char This is a combination of $count commits."
> 
> But you need to do "printf" to be safe here, I think, for comment_char='\'.
> 
> > @@ -375,8 +378,8 @@ update_squash_messages () {
> >  		commit_message HEAD > "$fixup_msg" || die "Cannot write $fixup_msg"
> >  		count=2
> >  		{
> > -			echo "# This is a combination of 2 commits."
> > -			echo "# The first commit's message is:"
> > +			echo "$comment_char This is a combination of 2 commits."
> > +			echo "$comment_char The first commit's message is:"
> 
> Likewise.
> 
> > @@ -385,21 +388,21 @@ update_squash_messages () {
> >  	squash)
> >  		rm -f "$fixup_msg"
> >  		echo
> > -		echo "# This is the $(nth_string $count) commit message:"
> > +		echo "$comment_char This is the $(nth_string $count) commit message:"
> 
> Likewise.
> 
> >  		echo
> >  		commit_message $2
> >  		;;
> >  	fixup)
> >  		echo
> > -		echo "# The $(nth_string $count) commit message will be skipped:"
> > +		echo "$comment_char The $(nth_string $count) commit message will be skipped:"
> >  		echo
> > -		commit_message $2 | sed -e 's/^/#	/'
> > +		commit_message $2 | sed -e "s/^/$comment_char	/"
> 
> Likewise.

Again this is adding $comment_char in front of each line, so it may need
a loop like above again.

> >  peek_next_command () {
> > -	sed -n -e "/^#/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
> > +	sed -n -e "/^$comment_char/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
> >  }
> 
> Likewise.
> 
> > @@ -464,7 +467,7 @@ do_next () {
> >  	rm -f "$msg" "$author_script" "$amend" || exit
> >  	read -r command sha1 rest < "$todo"
> >  	case "$command" in
> > -	'#'*|''|noop)
> > +	$comment_char*|''|noop)
> 
> This is OK.
> 
> > @@ -803,15 +806,15 @@ skip)
> >  	do_rest
> >  	;;
> >  edit-todo)
> > -	sed -e '/^#/d' < "$todo" > "$todo".new
> > +	sed -e "/^$comment_char/d" < "$todo" > "$todo".new
> 
> Unsafe.
> 
> >  	mv -f "$todo".new "$todo"
> >  	append_todo_help
> > -	cat >> "$todo" << EOF
> > -#
> > -# You are editing the todo file of an ongoing interactive rebase.
> > -# To continue rebase after editing, run:
> > -#     git rebase --continue
> > -#
> > +	sed -e "s/^/$comment_char /" >>"$todo" <<EOF
> 
> Unsafe.
> 
> > +
> > +You are editing the todo file of an ongoing interactive rebase.
> > +To continue rebase after editing, run:
> > +    git rebase --continue
> > +
> >  EOF
> >  
> >  	git_sequence_editor "$todo" ||
> > @@ -881,7 +884,7 @@ do
> >  
> >  	if test -z "$keep_empty" && is_empty_commit $shortsha1 && ! is_merge_commit $shortsha1
> >  	then
> > -		comment_out="# "
> > +		comment_out="$comment_char "
> 
> OK.
> 
> >  	else
> >  		comment_out=
> >  	fi
> > @@ -942,20 +945,20 @@ test -s "$todo" || echo noop >> "$todo"
> >  test -n "$autosquash" && rearrange_squash "$todo"
> >  test -n "$cmd" && add_exec_commands "$todo"
> >  
> > -cat >> "$todo" << EOF
> > -
> > -# Rebase $shortrevisions onto $shortonto
> > +echo >>"$todo"
> > +sed -e "s/^/$comment_char /" >> "$todo" << EOF
> 
> Unsafe.
> 
> > +Rebase $shortrevisions onto $shortonto
> >  EOF
> >  append_todo_help
> > -cat >> "$todo" << EOF
> > -#
> > -# However, if you remove everything, the rebase will be aborted.
> > -#
> > +sed -e "s/^/$comment_char /" >> "$todo" << EOF
> 
> Unsafe.
> 
> > +
> > +However, if you remove everything, the rebase will be aborted.
> > +
> >  EOF
> >  
> >  if test -z "$keep_empty"
> >  then
> > -	echo "# Note that empty commits are commented out" >>"$todo"
> > +	echo "$comment_char Note that empty commits are commented out" >>"$todo"
> >  fi
> >  
> >  
> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> > index 8462be1..1043cdc 100755
> > --- a/t/t3404-rebase-interactive.sh
> > +++ b/t/t3404-rebase-interactive.sh
> > @@ -934,4 +934,20 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '
> >  	test L = $(git cat-file commit HEAD | sed -ne \$p)
> >  '
> >  
> > +test_expect_success 'rebase -i respects core.commentchar' '
> > +	git reset --hard &&
> > +	git checkout E^0 &&
> > +	git config core.commentchar \; &&
> 
> Try setting it to '\' or '/' or '-'; they may catch some more breakages.
> 
> > +	test_when_finished "git config --unset core.commentchar" &&
> > +	cat >comment-lines.sh <<EOF &&
> > +#!$SHELL_PATH
> > +sed -e "2,\$ s/^/;/" "\$1" >"\$1".tmp
> > +mv "\$1".tmp "\$1"
> > +EOF
> > +	chmod a+x comment-lines.sh &&
> > +	test_set_editor "$(pwd)/comment-lines.sh" &&
> > +	git rebase -i B &&
> > +	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> > +'
> > +
> >  test_done

Re: [PATCH] rebase -i: respect core.commentchar

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

>> I do not think we would want to worry about comment_char being a
>> funny character that can possibly interfere with regexp.  Can't we
>> do this with "git stripspace" piped to "wc -l" or something?
>
> I didn't know about "git stripspace",...
>> > -# If you remove a line here THAT COMMIT WILL BE LOST.
>> > +	sed -e "s/^/$comment_char /" >>"$todo" <<EOF
>> 
>> When $comment_char is slash or backslash this will break.
>> Perhaps "stripspace --comment-lines" can be used here.
>
> Not in this case - this is adding the comment char in front of each
> line.  Is there a better option than this?
>
> 	while read -r line
> 	do
> 		printf '%s %s\n' "$comment_char" "$line"
> 	done >> "$todo" <<EOF
> 	...
> 	EOF

Perhaps

	git stripspace --comment-lines <<EOF
        ...
        EOF

is a better option that that loop.

[PATCH v2] rebase -i: respect core.commentchar

[John Keeping]

Commit eff80a9 (Allow custom "comment char") introduced a custom comment
character for commit messages but did not teach git-rebase--interactive
to use it.

Change git-rebase--interactive to read core.commentchar and use its
value when generating commit messages and for the command list.

Signed-off-by: John Keeping <john@keeping.me.uk>

---
Changes since v1:
- use '\' as the comment character in tests
- use "git stripspace --strip-comments" where appropriate
- use "git stripspace --comment-lines" where appropriate
- use printf instead of echo when the string contains $comment_char
- quote $comment_char in case label

On Mon, Feb 11, 2013 at 01:49:27PM -0800, Junio C Hamano wrote:
> ...
> Perhaps
> 
> 	git stripspace --comment-lines <<EOF
>         ...
>         EOF
> 
> is a better option that that loop.

Yes.  I was confusing myself with out-of-date documentation.

---
 git-rebase--interactive.sh    | 88 ++++++++++++++++++++++---------------------
 t/t3404-rebase-interactive.sh | 16 ++++++++
 2 files changed, 62 insertions(+), 42 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8ed7fcc..b5ce3f2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -80,6 +80,9 @@ rewritten_pending="$state_dir"/rewritten-pending
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 
+comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
+: ${comment_char:=#}
+
 warn () {
 	printf '%s\n' "$*" >&2
 }
@@ -105,8 +108,8 @@ mark_action_done () {
 	sed -e 1q < "$todo" >> "$done"
 	sed -e 1d < "$todo" >> "$todo".new
 	mv -f "$todo".new "$todo"
-	new_count=$(sane_grep -c '^[^#]' < "$done")
-	total=$(($new_count+$(sane_grep -c '^[^#]' < "$todo")))
+	new_count=$(git stripspace --strip-comments < "$done" | wc -l)
+	total=$(($new_count+$(git stripspace --strip-comments < "$todo" | wc -l)))
 	if test "$last_count" != "$new_count"
 	then
 		last_count=$new_count
@@ -116,19 +119,19 @@ mark_action_done () {
 }
 
 append_todo_help () {
-	cat >> "$todo" << EOF
-#
-# Commands:
-#  p, pick = use commit
-#  r, reword = use commit, but edit the commit message
-#  e, edit = use commit, but stop for amending
-#  s, squash = use commit, but meld into previous commit
-#  f, fixup = like "squash", but discard this commit's log message
-#  x, exec = run command (the rest of the line) using shell
-#
-# These lines can be re-ordered; they are executed from top to bottom.
-#
-# If you remove a line here THAT COMMIT WILL BE LOST.
+	git stripspace --comment-lines >>"$todo" <<EOF
+
+Commands:
+ p, pick = use commit
+ r, reword = use commit, but edit the commit message
+ e, edit = use commit, but stop for amending
+ s, squash = use commit, but meld into previous commit
+ f, fixup = like "squash", but discard this commit's log message
+ x, exec = run command (the rest of the line) using shell
+
+These lines can be re-ordered; they are executed from top to bottom.
+
+If you remove a line here THAT COMMIT WILL BE LOST.
 EOF
 }
 
@@ -179,7 +182,9 @@ die_abort () {
 }
 
 has_action () {
-	sane_grep '^[^#]' "$1" >/dev/null
+	echo "space stripped actions:" >&2
+	git stripspace --strip-comments <"$1" >&2
+	test -n "$(git stripspace --strip-comments <"$1")"
 }
 
 is_empty_commit() {
@@ -363,10 +368,10 @@ update_squash_messages () {
 	if test -f "$squash_msg"; then
 		mv "$squash_msg" "$squash_msg".bak || exit
 		count=$(($(sed -n \
-			-e "1s/^# This is a combination of \(.*\) commits\./\1/p" \
+			-e "1s/^. This is a combination of \(.*\) commits\./\1/p" \
 			-e "q" < "$squash_msg".bak)+1))
 		{
-			echo "# This is a combination of $count commits."
+			printf '%s\n' "$comment_char This is a combination of $count commits."
 			sed -e 1d -e '2,/^./{
 				/^$/d
 			}' <"$squash_msg".bak
@@ -375,8 +380,8 @@ update_squash_messages () {
 		commit_message HEAD > "$fixup_msg" || die "Cannot write $fixup_msg"
 		count=2
 		{
-			echo "# This is a combination of 2 commits."
-			echo "# The first commit's message is:"
+			printf '%s\n' "$comment_char This is a combination of 2 commits."
+			printf '%s\n' "$comment_char The first commit's message is:"
 			echo
 			cat "$fixup_msg"
 		} >"$squash_msg"
@@ -385,21 +390,22 @@ update_squash_messages () {
 	squash)
 		rm -f "$fixup_msg"
 		echo
-		echo "# This is the $(nth_string $count) commit message:"
+		printf '%s\n' "$comment_char This is the $(nth_string $count) commit message:"
 		echo
 		commit_message $2
 		;;
 	fixup)
 		echo
-		echo "# The $(nth_string $count) commit message will be skipped:"
+		printf '%s\n' "$comment_char The $(nth_string $count) commit message will be skipped:"
 		echo
-		commit_message $2 | sed -e 's/^/#	/'
+		# Change the space after the comment character to TAB:
+		commit_message $2 | git stripspace --comment-lines | sed -e 's/ /	/'
 		;;
 	esac >>"$squash_msg"
 }
 
 peek_next_command () {
-	sed -n -e "/^#/d" -e '/^$/d' -e "s/ .*//p" -e "q" < "$todo"
+	git stripspace --strip-comments <"$todo" | sed -n -e 's/ .*//p' -e q
 }
 
 # A squash/fixup has failed.  Prepare the long version of the squash
@@ -464,7 +470,7 @@ do_next () {
 	rm -f "$msg" "$author_script" "$amend" || exit
 	read -r command sha1 rest < "$todo"
 	case "$command" in
-	'#'*|''|noop)
+	"$comment_char"*|''|noop)
 		mark_action_done
 		;;
 	pick|p)
@@ -803,15 +809,15 @@ skip)
 	do_rest
 	;;
 edit-todo)
-	sed -e '/^#/d' < "$todo" > "$todo".new
+	git stripspace --strip-comments <"$todo" >"$todo".new
 	mv -f "$todo".new "$todo"
 	append_todo_help
-	cat >> "$todo" << EOF
-#
-# You are editing the todo file of an ongoing interactive rebase.
-# To continue rebase after editing, run:
-#     git rebase --continue
-#
+	git stripspace --comment-lines >>"$todo" <<EOF
+
+You are editing the todo file of an ongoing interactive rebase.
+To continue rebase after editing, run:
+    git rebase --continue
+
 EOF
 
 	git_sequence_editor "$todo" ||
@@ -881,7 +887,7 @@ do
 
 	if test -z "$keep_empty" && is_empty_commit $shortsha1 && ! is_merge_commit $shortsha1
 	then
-		comment_out="# "
+		comment_out="$comment_char "
 	else
 		comment_out=
 	fi
@@ -942,20 +948,18 @@ test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
 test -n "$cmd" && add_exec_commands "$todo"
 
-cat >> "$todo" << EOF
-
-# Rebase $shortrevisions onto $shortonto
-EOF
+echo >>"$todo"
+printf '%s\n' "$comment_char Rebase $shortrevisions onto $shortonto" >>"$todo"
 append_todo_help
-cat >> "$todo" << EOF
-#
-# However, if you remove everything, the rebase will be aborted.
-#
+git stripspace --comment-lines >>"$todo" <<EOF
+
+However, if you remove everything, the rebase will be aborted.
+
 EOF
 
 if test -z "$keep_empty"
 then
-	echo "# Note that empty commits are commented out" >>"$todo"
+	printf '%s\n' "$comment_char Note that empty commits are commented out" >>"$todo"
 fi
 
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8462be1..16d8160 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -934,4 +934,20 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '
 	test L = $(git cat-file commit HEAD | sed -ne \$p)
 '
 
+test_expect_success 'rebase -i respects core.commentchar' '
+	git reset --hard &&
+	git checkout E^0 &&
+	git config core.commentchar "\\" &&
+	test_when_finished "git config --unset core.commentchar" &&
+	cat >comment-lines.sh <<EOF &&
+#!$SHELL_PATH
+sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
+mv "\$1".tmp "\$1"
+EOF
+	chmod a+x comment-lines.sh &&
+	test_set_editor "$(pwd)/comment-lines.sh" &&
+	git rebase -i B &&
+	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
+'
+
 test_done
-- 
1.8.1.3.556.gb3310b5.dirty

Re: [PATCH v2] rebase -i: respect core.commentchar

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> @@ -179,7 +182,9 @@ die_abort () {
>  }
>  
>  has_action () {
> -	sane_grep '^[^#]' "$1" >/dev/null
> +	echo "space stripped actions:" >&2
> +	git stripspace --strip-comments <"$1" >&2
> +	test -n "$(git stripspace --strip-comments <"$1")"
>  }

I'll remove the debugging remnants while queuing.

>  	fixup)
>  		echo
> -		echo "# The $(nth_string $count) commit message will be skipped:"
> +		printf '%s\n' "$comment_char The $(nth_string $count) commit message will be skipped:"
>  		echo
> -		commit_message $2 | sed -e 's/^/#	/'
> +		# Change the space after the comment character to TAB:
> +		commit_message $2 | git stripspace --comment-lines | sed -e 's/ /	/'

I think this changes the behaviour but in a good way.  It used to
show an empty line in the incoming commit message to a hash followed
by a trailing HT before the end of line, but now it only emits the
comment char immediately followed by the end of line.

> @@ -942,20 +948,18 @@ test -s "$todo" || echo noop >> "$todo"
>  test -n "$autosquash" && rearrange_squash "$todo"
>  test -n "$cmd" && add_exec_commands "$todo"
>  
> -cat >> "$todo" << EOF
> -
> -# Rebase $shortrevisions onto $shortonto
> -EOF
> +echo >>"$todo"
> +printf '%s\n' "$comment_char Rebase $shortrevisions onto $shortonto" >>"$todo"

I think you can still do

	cat >>"$todo" <<EOF

	$comment_char Rebase $shortrevisions onto...
	EOF

here with any funny comment character.  Doing this with two separate
I/O does not hurt very much, but the resulting code may be easier to
scan if left as here-text with a single cat.

Please eyeball what is in 'pu' (I have a separate squashable fixup
on top of your patch) and let me know if I made mistakes.

Thanks for working on this.

Re: [PATCH v2] rebase -i: respect core.commentchar

[John Keeping]

On Mon, Feb 11, 2013 at 04:13:31PM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > @@ -179,7 +182,9 @@ die_abort () {
> >  }
> >  
> >  has_action () {
> > -	sane_grep '^[^#]' "$1" >/dev/null
> > +	echo "space stripped actions:" >&2
> > +	git stripspace --strip-comments <"$1" >&2
> > +	test -n "$(git stripspace --strip-comments <"$1")"
> >  }
> 
> I'll remove the debugging remnants while queuing.

Thanks.  I don't think I was fully awake when I finished this last
night - the following fixup is also needed to avoid relying on the shell
emitting a literal backslash when a backslash isn't followed by a known
escape character.

-- >8 --

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index cbe36bf..84bd525 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test_when_finished "git config --unset core.commentchar" &&
 	cat >comment-lines.sh <<EOF &&
 #!$SHELL_PATH
-sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
+sed -e "2,\$ s/^/\\\\\\\\/" "\$1" >"\$1".tmp
 mv "\$1".tmp "\$1"
 EOF
 	chmod a+x comment-lines.sh &&

-- >8 --

> > @@ -942,20 +948,18 @@ test -s "$todo" || echo noop >> "$todo"
> >  test -n "$autosquash" && rearrange_squash "$todo"
> >  test -n "$cmd" && add_exec_commands "$todo"
> >  
> > -cat >> "$todo" << EOF
> > -
> > -# Rebase $shortrevisions onto $shortonto
> > -EOF
> > +echo >>"$todo"
> > +printf '%s\n' "$comment_char Rebase $shortrevisions onto $shortonto" >>"$todo"
> 
> I think you can still do
> 
> 	cat >>"$todo" <<EOF
> 
> 	$comment_char Rebase $shortrevisions onto...
> 	EOF
> 
> here with any funny comment character.  Doing this with two separate
> I/O does not hurt very much, but the resulting code may be easier to
> scan if left as here-text with a single cat.
> 
> Please eyeball what is in 'pu' (I have a separate squashable fixup
> on top of your patch) and let me know if I made mistakes.

The fixup commit looks good to me.


John

Re: [PATCH v2] rebase -i: respect core.commentchar

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> ... the following fixup is also needed to avoid relying on the shell
> emitting a literal backslash when a backslash isn't followed by a known
> escape character.
>
> -- >8 --
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index cbe36bf..84bd525 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
>  	test_when_finished "git config --unset core.commentchar" &&
>  	cat >comment-lines.sh <<EOF &&
>  #!$SHELL_PATH
> -sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
> +sed -e "2,\$ s/^/\\\\\\\\/" "\$1" >"\$1".tmp
>  mv "\$1".tmp "\$1"
>  EOF
>  	chmod a+x comment-lines.sh &&

Yeek.  If you used write_script with here-text that does not
interpolate,

	write_script remove-all-but-the-first.sh <<\EOF
	sed -e '2,$s/^/\\/'  <"$1" >"$1.tmp" &&
        mv "$1.tmp" "$1"
	EOF

the above would be much more readable.

I am not sure if I understand what you meant by "literal backslash
blah blah", though.

Re: [PATCH v2] rebase -i: respect core.commentchar

[John Keeping]

On Tue, Feb 12, 2013 at 09:29:26AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > ... the following fixup is also needed to avoid relying on the shell
> > emitting a literal backslash when a backslash isn't followed by a known
> > escape character.
> >
> > -- >8 --
> >
> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> > index cbe36bf..84bd525 100755
> > --- a/t/t3404-rebase-interactive.sh
> > +++ b/t/t3404-rebase-interactive.sh
> > @@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
> >  	test_when_finished "git config --unset core.commentchar" &&
> >  	cat >comment-lines.sh <<EOF &&
> >  #!$SHELL_PATH
> > -sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
> > +sed -e "2,\$ s/^/\\\\\\\\/" "\$1" >"\$1".tmp
> >  mv "\$1".tmp "\$1"
> >  EOF
> >  	chmod a+x comment-lines.sh &&
> 
> Yeek.  If you used write_script with here-text that does not
> interpolate,
> 
> 	write_script remove-all-but-the-first.sh <<\EOF
> 	sed -e '2,$s/^/\\/'  <"$1" >"$1.tmp" &&
>         mv "$1.tmp" "$1"
> 	EOF
> 
> the above would be much more readable.

Yet another thing for me to learn about ;-)

Do you mean to use that outside the test case, so that the single quotes
work?  Or do I still need some level of escaping?

> I am not sure if I understand what you meant by "literal backslash
> blah blah", though.

It turns out that having this in the script works (in bash and dash
although I haven't checked what Posix has to say about it):

    sed -e "2,$ s/^/\\\/"

and is equivalent to:

    sed -e '2,$ s/^/\\/'

because backslashes that aren't recognised as part of an escape sequence
are not treated specially.


John

Re: [PATCH v2] rebase -i: respect core.commentchar

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>>  	cat >comment-lines.sh <<EOF &&
>>  #!$SHELL_PATH
>> -sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
>> +sed -e "2,\$ s/^/\\\\\\\\/" "\$1" >"\$1".tmp
>>  mv "\$1".tmp "\$1"
>>  EOF
>>  	chmod a+x comment-lines.sh &&
>
> Yeek.  If you used write_script with here-text that does not
> interpolate,
>
> 	write_script remove-all-but-the-first.sh <<\EOF
> 	sed -e '2,$s/^/\\/'  <"$1" >"$1.tmp" &&
>         mv "$1.tmp" "$1"
> 	EOF
>
> the above would be much more readable.

As this is already inside a pair of '', we cannot use single quote
around sed expression without doing the ugly '\''.

So it needs to be more like this, and I think it still is more
readable.

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index cbe36bf..8b3e2cd 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -945,13 +945,11 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	git checkout E^0 &&
 	git config core.commentchar "\\" &&
 	test_when_finished "git config --unset core.commentchar" &&
-	cat >comment-lines.sh <<EOF &&
-#!$SHELL_PATH
-sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
-mv "\$1".tmp "\$1"
-EOF
-	chmod a+x comment-lines.sh &&
-	test_set_editor "$(pwd)/comment-lines.sh" &&
+	write_script remove-all-but-first.sh <<-\EOF &&
+	sed -e "2,\$s/^/\\\\/" "$1" >"$1.tmp" &&
+	mv "$1.tmp" "$1"
+	EOF
+	test_set_editor "$(pwd)/remove-all-but-first.sh" &&
 	git rebase -i B &&
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '

Re: [PATCH v2] rebase -i: respect core.commentchar

[John Keeping]

On Tue, Feb 12, 2013 at 10:00:26AM -0800, Junio C Hamano wrote:
>
> So it needs to be more like this, and I think it still is more
> readable.

Agreed.  Will you squash this in or do you want a re-roll?

> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index cbe36bf..8b3e2cd 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -945,13 +945,11 @@ test_expect_success 'rebase -i respects core.commentchar' '
>  	git checkout E^0 &&
>  	git config core.commentchar "\\" &&
>  	test_when_finished "git config --unset core.commentchar" &&
> -	cat >comment-lines.sh <<EOF &&
> -#!$SHELL_PATH
> -sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
> -mv "\$1".tmp "\$1"
> -EOF
> -	chmod a+x comment-lines.sh &&
> -	test_set_editor "$(pwd)/comment-lines.sh" &&
> +	write_script remove-all-but-first.sh <<-\EOF &&
> +	sed -e "2,\$s/^/\\\\/" "$1" >"$1.tmp" &&
> +	mv "$1.tmp" "$1"
> +	EOF
> +	test_set_editor "$(pwd)/remove-all-but-first.sh" &&
>  	git rebase -i B &&
>  	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
>  '

Re: [PATCH v2] rebase -i: respect core.commentchar

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

>> I am not sure if I understand what you meant by "literal backslash
>> blah blah", though.
>
> It turns out that having this in the script works (in bash and dash
> although I haven't checked what Posix has to say about it):
>
>     sed -e "2,$ s/^/\\\/"
>
> and is equivalent to:
>
>     sed -e '2,$ s/^/\\/'
>
> because backslashes that aren't recognised as part of an escape sequence
> are not treated specially.

That's POSIX.  Inside a dq pair:

        \
        The <backslash> shall retain its special meaning as an escape
        character (see Escape Character (Backslash)) only when followed by
        one of the following characters when considered special:
        $   `   "   \   <newline>

So in your example "\\\/", the first backslash escapes the second
backslash and together they produce a single backslash, the third
backslash is followed by a slash that is not special at all, so it
produces a second backslash, and the slash stands for itself,
resulting in "\\/".

Re: [PATCH v2] rebase -i: respect core.commentchar

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> On Tue, Feb 12, 2013 at 10:00:26AM -0800, Junio C Hamano wrote:
>>
>> So it needs to be more like this, and I think it still is more
>> readable.
>
> Agreed.  Will you squash this in or do you want a re-roll?

I can squash this and the previous one into your original to a
single commit.  Thanks.

>
>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
>> index cbe36bf..8b3e2cd 100755
>> --- a/t/t3404-rebase-interactive.sh
>> +++ b/t/t3404-rebase-interactive.sh
>> @@ -945,13 +945,11 @@ test_expect_success 'rebase -i respects core.commentchar' '
>>  	git checkout E^0 &&
>>  	git config core.commentchar "\\" &&
>>  	test_when_finished "git config --unset core.commentchar" &&
>> -	cat >comment-lines.sh <<EOF &&
>> -#!$SHELL_PATH
>> -sed -e "2,\$ s/^/\\\\\\/" "\$1" >"\$1".tmp
>> -mv "\$1".tmp "\$1"
>> -EOF
>> -	chmod a+x comment-lines.sh &&
>> -	test_set_editor "$(pwd)/comment-lines.sh" &&
>> +	write_script remove-all-but-first.sh <<-\EOF &&
>> +	sed -e "2,\$s/^/\\\\/" "$1" >"$1.tmp" &&
>> +	mv "$1.tmp" "$1"
>> +	EOF
>> +	test_set_editor "$(pwd)/remove-all-but-first.sh" &&
>>  	git rebase -i B &&
>>  	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
>>  '