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

Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 7, 2009, 07:12 UTC
Message-ID
<7vskbn9s1k.fsf@alter.siamese.dyndns.org>
In-Reply-To
<ced6765cff6225a05f196a6896ab577850979ab1.1260099005.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 11 quoted lines
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0bd3bf7..a7de5ea 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -302,7 +302,13 @@ nth_string () {
>  
>  make_squash_message () {
>  	if test -f "$SQUASH_MSG"; then
> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> +		# We want to be careful about matching only the commit
> +		# message comment lines generated by this function.
> +		# But supposedly some sed versions don't handle "\|"
> +		# correctly, so instead of "\(st\|nd\|rd\|th\)", use
> +		# the less accurate "[snrt][tdh]" to match the
> +		# nth_string endings.

I'd drop this comment; blaming POSIX-compliant sed without GNU extension is simply wrong.

Show 29 quoted lines
> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>  		echo "# This is a combination of $COUNT commits."
>  		sed -e 1d -e '2,/^./{
> @@ -315,10 +321,26 @@ make_squash_message () {
>  		echo
>  		git cat-file commit HEAD | sed -e '1,/^$/d'
>  	fi
> -	echo
> -	echo "# This is the $(nth_string $COUNT) commit message:"
> -	echo
> -	git cat-file commit $1 | sed -e '1,/^$/d'
> +	case $1 in
> +	squash)
> +		echo
> +		echo "# This is the $(nth_string $COUNT) commit message:"
> +		echo
> +		git cat-file commit $2 | sed -e '1,/^$/d'
> +		;;
> +	fixup)
> +		echo
> +		echo "# The $(nth_string $COUNT) commit message will be skipped:"
> +		echo
> +		# Comment the lines of the commit message out using
> +		# "#" rather than "# " (a) to make them more distinct
> +		# from the explanatory comments added by this function
> +		# and (b) to make it less likely that the sed regexp
> +		# above will be confused by a commented-out commit
> +		# message.

Use "# " as prefix and you won't have to worry about a line in the log message that begins with " Th", no?

In any case, I agree that a comment like this is necessary to warn anybody who will be touching the code that the COUNT=$((...)) needs to avoid matching what is produced here, but I find the above 6-line comment a bit too excessive.

Previous: Michael HaggertyNext: Michael Haggerty
Message 4 of 13 in “[PATCHv2 0/2] Add a "fixup" command to "rebase --interactive"”
  1. Michael HaggertyDec 7, 2009
  2. 1/2 t3404: Use test_commit to set up test repositoryMichael Haggerty, Dec 7, 2009
  3. 2/2 Add a command "fixup" to rebase --interactiveMichael Haggerty, Dec 7, 2009
  4. Junio C HamanoDec 7, 2009
  5. 0/2 Add a "fixup" command to "rebase --interactive"Michael Haggerty, Dec 7, 2009
  6. Johannes SchindelinDec 7, 2009
  7. 1/2 t3404: Use test_commit to set up test repositoryMichael Haggerty, Dec 7, 2009
  8. Junio C HamanoDec 8, 2009
  9. 2/2 Add a command "fixup" to rebase --interactiveMichael Haggerty, Dec 7, 2009
  10. Sverre RabbelierDec 7, 2009
  11. Michael HaggertyDec 7, 2009
  12. Matthieu MoyDec 7, 2009
  13. Sverre RabbelierDec 7, 2009

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.