* [PATCH] rebase -i: respect core.commentchar
@ 2013-02-11 19:21 John Keeping
2013-02-11 20:17 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: John Keeping @ 2013-02-11 19:21 UTC (permalink / raw)
To: git; +Cc: Ralf Thielow, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] rebase -i: respect core.commentchar
2013-02-11 19:21 [PATCH] rebase -i: respect core.commentchar John Keeping
@ 2013-02-11 20:17 ` Junio C Hamano
2013-02-11 21:39 ` John Keeping
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2013-02-11 20:17 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rebase -i: respect core.commentchar
2013-02-11 20:17 ` Junio C Hamano
@ 2013-02-11 21:39 ` John Keeping
2013-02-11 21:49 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: John Keeping @ 2013-02-11 21:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ralf Thielow
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rebase -i: respect core.commentchar
2013-02-11 21:39 ` John Keeping
@ 2013-02-11 21:49 ` Junio C Hamano
2013-02-11 23:08 ` [PATCH v2] " John Keeping
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2013-02-11 21:49 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] rebase -i: respect core.commentchar
2013-02-11 21:49 ` Junio C Hamano
@ 2013-02-11 23:08 ` John Keeping
2013-02-12 0:13 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: John Keeping @ 2013-02-11 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ralf Thielow
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-11 23:08 ` [PATCH v2] " John Keeping
@ 2013-02-12 0:13 ` Junio C Hamano
2013-02-12 9:53 ` John Keeping
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2013-02-12 0:13 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 0:13 ` Junio C Hamano
@ 2013-02-12 9:53 ` John Keeping
2013-02-12 17:29 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: John Keeping @ 2013-02-12 9:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ralf Thielow
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 9:53 ` John Keeping
@ 2013-02-12 17:29 ` Junio C Hamano
2013-02-12 17:53 ` John Keeping
2013-02-12 18:00 ` Junio C Hamano
0 siblings, 2 replies; 13+ messages in thread
From: Junio C Hamano @ 2013-02-12 17:29 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 17:29 ` Junio C Hamano
@ 2013-02-12 17:53 ` John Keeping
2013-02-12 18:21 ` Junio C Hamano
2013-02-12 18:00 ` Junio C Hamano
1 sibling, 1 reply; 13+ messages in thread
From: John Keeping @ 2013-02-12 17:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Keeping, git, Ralf Thielow
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 17:29 ` Junio C Hamano
2013-02-12 17:53 ` John Keeping
@ 2013-02-12 18:00 ` Junio C Hamano
2013-02-12 18:09 ` John Keeping
1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2013-02-12 18:00 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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)
'
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 18:00 ` Junio C Hamano
@ 2013-02-12 18:09 ` John Keeping
2013-02-12 18:23 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: John Keeping @ 2013-02-12 18:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ralf Thielow
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)
> '
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 17:53 ` John Keeping
@ 2013-02-12 18:21 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2013-02-12 18:21 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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 "\\/".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] rebase -i: respect core.commentchar
2013-02-12 18:09 ` John Keeping
@ 2013-02-12 18:23 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2013-02-12 18:23 UTC (permalink / raw)
To: John Keeping; +Cc: git, Ralf Thielow
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)
>> '
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-02-12 18:23 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-11 19:21 [PATCH] rebase -i: respect core.commentchar John Keeping
2013-02-11 20:17 ` Junio C Hamano
2013-02-11 21:39 ` John Keeping
2013-02-11 21:49 ` Junio C Hamano
2013-02-11 23:08 ` [PATCH v2] " John Keeping
2013-02-12 0:13 ` Junio C Hamano
2013-02-12 9:53 ` John Keeping
2013-02-12 17:29 ` Junio C Hamano
2013-02-12 17:53 ` John Keeping
2013-02-12 18:21 ` Junio C Hamano
2013-02-12 18:00 ` Junio C Hamano
2013-02-12 18:09 ` John Keeping
2013-02-12 18:23 ` 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.