{"thread":{"id":"24342","subject":"[PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR","startedAt":"2010-07-09T05:20:42Z","lastAt":"2010-07-10T12:22:41Z","messageCount":5,"participants":["Nazri Ramliy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"145192","messageId":"1278652843-30872-1-git-send-email-ayiehere@gmail.com","threadId":"24342","inReplyTo":null,"subject":"[PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-09T05:20:42Z","receivedAt":"2010-07-09T05:20:42Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"Signed-off-by: Nazri Ramliy <ayiehere@gmail.com>\n---\nThis is in preparation of patch 2/2 in this series.\n\nnazri.\n\n t/lib-rebase.sh |   15 +++++++++++++++\n 1 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 6aefe27..e1a33fc 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -4,6 +4,10 @@\n #\n # - override the commit message with $FAKE_COMMIT_MESSAGE\n # - amend the commit message with $FAKE_COMMIT_AMEND\n+# - check that the non-comment text shown in the editor matches that in the\n+#   file $EXPECT_NON_COMMENT_LINES. To make the comparison ignore the commit ids\n+#   use the string \"COMMIT_ID\" in place of the sha1 in your 'expect' file and set\n+#   REPLACE_COMMIT_ID=COMMIT_ID\n # - check that non-commit messages have a certain line count with $EXPECT_COUNT\n # - check the commit count in the commit message header with $EXPECT_HEADER_COUNT\n # - rewrite a rebase -i script as directed by $FAKE_LINES.\n@@ -34,6 +38,17 @@ case \"$1\" in\n \texit\n \t;;\n esac\n+test -z \"$EXPECT_NON_COMMENT_LINES\" ||\n+\t(\n+\t\tgrep -v '^#' < \"$1\" > out_non_comment.$$ &&\n+\t\tif test -n \"$REPLACE_COMMIT_ID\"\n+\t\tthen\n+\t\t\tsed -i \"s/^\\([a-z]\\+\\) [0-9a-f]\\+ /\\1 $REPLACE_COMMIT_ID /\" \\\n+\t\t\tout_non_comment.$$\n+\t\tfi &&\n+\t\tdiff \"$EXPECT_NON_COMMENT_LINES\" out_non_comment.$$ > /dev/null\n+\t) ||\n+\texit\n test -z \"$EXPECT_COUNT\" ||\n \ttest \"$EXPECT_COUNT\" = $(sed -e '/^#/d' -e '/^$/d' < \"$1\" | wc -l) ||\n \texit\n-- \n1.7.2.rc2\n"},{"id":"145193","messageId":"1278652843-30872-2-git-send-email-ayiehere@gmail.com","threadId":"24342","inReplyTo":"1278652843-30872-1-git-send-email-ayiehere@gmail.com","subject":"[PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header in $GIT_EDITOR","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-09T05:20:43Z","receivedAt":"2010-07-09T05:20:43Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"During \"git rebase -i\", when the commit headers are shown in the editor\nfor action (pick/squash/etc), the whitespace (if any) at the beginning\nof commit headers are stripped out due to the use of \"read shortsha1 rest\"\nfor reading the output of \"git rev-list\".\n\nThe missing beginning whitespace do not pose any harm but this could be\nannoying when you want to identify these commits to perform actions on,\nfor example, rewording them to remove the beginning whitespace.\n\nNot having the whitespace makes them hard to spot, and you have to rely\non something like \"git log --oneline\" to identify them.\n\nThis patch:\n\n\t1. Ensures that the commit header shown in $GIT_EDITOR is\n\t   identical to actual commit header\n\t2. Add a test for it\n\nSigned-off-by: Nazri Ramliy <ayiehere@gmail.com>\n---\n git-rebase--interactive.sh    |   25 ++++++++++++++-----------\n t/t3404-rebase-interactive.sh |   13 +++++++++++++\n 2 files changed, 27 insertions(+), 11 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 31e6860..726cb6a 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -889,14 +889,15 @@ first and then run 'git rebase --continue' again.\"\n \t\tfi\n \t\tgit rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \\\n \t\t\t--abbrev=7 --reverse --left-right --topo-order \\\n-\t\t\t$REVISIONS | \\\n-\t\t\tsed -n \"s/^>//p\" |\n-\t\twhile read -r shortsha1 rest\n-\t\tdo\n-\t\t\tif test t != \"$PRESERVE_MERGES\"\n-\t\t\tthen\n-\t\t\t\techo \"pick $shortsha1 $rest\" >> \"$TODO\"\n-\t\t\telse\n+\t\t\t$REVISIONS | sed -n \"s/^>//p\" > \"$TODO.tmp\"\n+\n+\t\tif test t != \"$PRESERVE_MERGES\"\n+\t\tthen\n+\t\t\tcat \"$TODO.tmp\" | sed \"s/^/pick /\" > \"$TODO\"\n+\t\telse\n+\t\t\tcat \"$TODO.tmp\" |\n+\t\t\twhile read -r shortsha1 rest\n+\t\t\tdo\n \t\t\t\tsha1=$(git rev-parse $shortsha1)\n \t\t\t\tif test -z \"$REBASE_ROOT\"\n \t\t\t\tthen\n@@ -914,10 +915,12 @@ first and then run 'git rebase --continue' again.\"\n \t\t\t\tif test f = \"$preserve\"\n \t\t\t\tthen\n \t\t\t\t\ttouch \"$REWRITTEN\"/$sha1\n-\t\t\t\t\techo \"pick $shortsha1 $rest\" >> \"$TODO\"\n+\t\t\t\t\tgrep \"^$shortsha1\" \"$TODO.tmp\" | sed \"s/^/pick /\" >> \"$TODO\"\n \t\t\t\tfi\n-\t\t\tfi\n-\t\tdone\n+\t\t\tdone\n+\n+\t\tfi\n+\t\trm -f \"$TODO.tmp\"\n \n \t\t# Watch for commits that been dropped by --cherry-pick\n \t\tif test t = \"$PRESERVE_MERGES\"\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 47ca88f..2c3a0b9 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -648,4 +648,17 @@ test_expect_success 'rebase-i history with funny messages' '\n \ttest_cmp expect actual\n '\n \n+cat >expect <<EOF\n+pick COMMIT_ID   Commit header with beginning whitespace\n+\n+EOF\n+\n+test_expect_success 'preserve whitespace in commit summary' '\n+\tgit checkout -b preserve-whitespace master &&\n+\techo a >whitespace_test &&\n+\tgit add whitespace_test &&\n+\tgit commit -m\"  Commit header with beginning whitespace\" &&\n+\tREPLACE_COMMIT_ID=COMMIT_ID EXPECT_NON_COMMENT_LINES=\"expect\" git rebase -i HEAD~1\n+'\n+\n test_done\n-- \n1.7.2.rc2\n"},{"id":"145245","messageId":"7vtyo8l248.fsf@alter.siamese.dyndns.org","threadId":"24342","inReplyTo":"1278652843-30872-1-git-send-email-ayiehere@gmail.com","subject":"Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of non-comment lines in $GIT_EDITOR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-09T22:30:31Z","receivedAt":"2010-07-09T22:30:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nazri Ramliy <ayiehere@gmail.com> writes:\n\n> +\t\t\tsed -i \"s/^\\([a-z]\\+\\) [0-9a-f]\\+ /\\1 $REPLACE_COMMIT_ID /\" \\\n\nThis is not portable. Escaping an ERE element with a backslash does not\nmake it suitable for use in BRE that sed uses.\n\nDo we use in-place replacement anywhere else with sed?  I don't think it\nis portable, either.\n"},{"id":"145246","messageId":"7vpqywl1sb.fsf@alter.siamese.dyndns.org","threadId":"24342","inReplyTo":"1278652843-30872-2-git-send-email-ayiehere@gmail.com","subject":"Re: [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header in $GIT_EDITOR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-09T22:37:40Z","receivedAt":"2010-07-09T22:37:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nazri Ramliy <ayiehere@gmail.com> writes:\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 31e6860..726cb6a 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -889,14 +889,15 @@ first and then run 'git rebase --continue' again.\"\n>  \t\tfi\n>  \t\tgit rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \\\n>  \t\t\t--abbrev=7 --reverse --left-right --topo-order \\\n> +\t\t\t$REVISIONS | sed -n \"s/^>//p\" > \"$TODO.tmp\"\n> +\n> +\t\tif test t != \"$PRESERVE_MERGES\"\n> +\t\tthen\n> +\t\t\tcat \"$TODO.tmp\" | sed \"s/^/pick /\" > \"$TODO\"\n\nDo not cat a single file into a pipeline.\n\n\tsed \"s/^/pick /\" <\"$TODO.tmp\" >\"$TODO\"\n\n> +\t\telse\n> +\t\t\tcat \"$TODO.tmp\" |\n\nLikewise.\n\n> +\t\t\twhile read -r shortsha1 rest\n> +\t\t\tdo\n>  \t\t\t\tsha1=$(git rev-parse $shortsha1)\n>  \t\t\t\tif test -z \"$REBASE_ROOT\"\n>  \t\t\t\tthen\n> @@ -914,10 +915,12 @@ first and then run 'git rebase --continue' again.\"\n>  \t\t\t\tif test f = \"$preserve\"\n>  \t\t\t\tthen\n>  \t\t\t\t\ttouch \"$REWRITTEN\"/$sha1\n> +\t\t\t\t\tgrep \"^$shortsha1\" \"$TODO.tmp\" | sed \"s/^/pick /\" >> \"$TODO\"\n\nDon't pipe output of grep into sed.\n\n\tsed -ne \"/^$shortsha1 /s/^/pick /p\" <\"$TODO.tmp\" >>\"$TODO\"\n"},{"id":"145273","messageId":"AANLkTim2cOWWYbco3XYb_dfOCkUp4rfXIvE-8jZWK4a8@mail.gmail.com","threadId":"24342","inReplyTo":"7vpqywl1sb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] rebase -i: Preserve whitespace at beginning of commit header in $GIT_EDITOR","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-10T12:22:41Z","receivedAt":"2010-07-10T12:22:41Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Sat, Jul 10, 2010 at 6:30 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nazri Ramliy <ayiehere@gmail.com> writes:\n>\n>> +                     sed -i \"s/^\\([a-z]\\+\\) [0-9a-f]\\+ /\\1 $REPLACE_COMMIT_ID /\" \\\n>\n> This is not portable. Escaping an ERE element with a backslash does not\n> make it suitable for use in BRE that sed uses.\n>\n> Do we use in-place replacement anywhere else with sed?  I don't think it\n> is portable, either.\n>\n\nAnd later ...\n\nOn Sat, Jul 10, 2010 at 6:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> +                     cat \"$TODO.tmp\" | sed \"s/^/pick /\" > \"$TODO\"\n>\n> Do not cat a single file into a pipeline.\n>\n>        sed \"s/^/pick /\" <\"$TODO.tmp\" >\"$TODO\"\n>\n>> +             else\n>> +                     cat \"$TODO.tmp\" |\n>\n> Likewise.\n>\n[snip]\n> Don't pipe output of grep into sed.\n>\n>        sed -ne \"/^$shortsha1 /s/^/pick /p\" <\"$TODO.tmp\" >>\"$TODO\"\n\nI'll send out a v2 of this series that addresses the above issues in\nbetter ways (i think).\n\nnazri\n"}]}