git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 11, 2013, 20:17 UTC
Message-ID
<7vzjzali6a.fsf@alter.siamese.dyndns.org>
In-Reply-To
<aa1deab1de2e0f998b9ac0bc8c2d76557429a46b.1360610368.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
Show 9 quoted lines
> 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.
Show 14 quoted lines
>  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.
Show 8 quoted lines
> @@ -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?

Show 18 quoted lines
> @@ -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.

Show 21 quoted lines
> +
> +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.
Show 6 quoted lines
> @@ -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='\'.
Show 8 quoted lines
> @@ -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.
Show 6 quoted lines
> @@ -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.
Show 10 quoted lines
>  		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.
Show 6 quoted lines
> @@ -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.
Show 6 quoted lines
> @@ -803,15 +806,15 @@ skip)
>  	do_rest
>  	;;
>  edit-todo)
> -	sed -e '/^#/d' < "$todo" > "$todo".new
> +	sed -e "/^$comment_char/d" < "$todo" > "$todo".new
Unsafe.
Show 9 quoted lines
>  	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.
Show 14 quoted lines
> +
> +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.
Show 12 quoted lines
>  	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.
Show 8 quoted lines
> +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.
Show 24 quoted lines
> +
> +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.
Show 13 quoted lines
> +	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
Previous: John KeepingNext: John Keeping
Message 2 of 13 in “rebase -i: respect core.commentchar”
  1. rebase -i: respect core.commentcharJohn Keeping, Feb 11, 2013
  2. Junio C HamanoFeb 11, 2013
  3. John KeepingFeb 11, 2013
  4. Junio C HamanoFeb 11, 2013
  5. rebase -i: respect core.commentcharJohn Keeping, Feb 11, 2013
  6. Junio C HamanoFeb 12, 2013
  7. John KeepingFeb 12, 2013
  8. Junio C HamanoFeb 12, 2013
  9. John KeepingFeb 12, 2013
  10. Junio C HamanoFeb 12, 2013
  11. Junio C HamanoFeb 12, 2013
  12. John KeepingFeb 12, 2013
  13. Junio C HamanoFeb 12, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.