{"thread":{"id":"24357","subject":"[PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","startedAt":"2010-07-10T12:27:00Z","lastAt":"2010-07-13T05:19:16Z","messageCount":12,"participants":["Nazri Ramliy","Junio C Hamano","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"145274","messageId":"1278764821-32647-1-git-send-email-ayiehere@gmail.com","threadId":"24357","inReplyTo":null,"subject":"[PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-10T12:27:00Z","receivedAt":"2010-07-10T12:27:00Z","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---\n t/lib-rebase.sh |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 6aefe27..21d2fef 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -4,6 +4,8 @@\n #\n # - override the commit message with $FAKE_COMMIT_MESSAGE\n # - amend the commit message with $FAKE_COMMIT_AMEND\n+# - check that the commit header(s) text shown in the editor matches that in the\n+#   file $EXPECT_HEADER.\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 +36,12 @@ case \"$1\" in\n \texit\n \t;;\n esac\n+test -z \"$EXPECT_HEADER\" ||\n+\t(\n+\t\tgrep '^pick' < \"$1\" | cut -d' ' -f3- > commit_headers.$$ &&\n+\t\tdiff \"$EXPECT_HEADER\" commit_headers.$$ > /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":"145275","messageId":"1278764821-32647-2-git-send-email-ayiehere@gmail.com","threadId":"24357","inReplyTo":"1278764821-32647-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-10T12:27:01Z","receivedAt":"2010-07-10T12:27:01Z","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    |   14 ++++++--------\n t/t3404-rebase-interactive.sh |   12 ++++++++++++\n 2 files changed, 18 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 31e6860..b13f42e 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -887,17 +887,15 @@ first and then run 'git rebase --continue' again.\"\n \t\t\tREVISIONS=$ONTO...$HEAD\n \t\t\tSHORTREVISIONS=$SHORTHEAD\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+\n+\t\tgit rev-list $MERGES_OPTION  --reverse --left-right --topo-order \\\n+\t\t\t$REVISIONS | sed -n \"s/^>//p\" |\n+\t\twhile read sha1\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\t\tgit log -1 --abbrev-commit --abbrev=7 --format=\"pick %h %s\" $sha1 > \"$TODO\"\n \t\t\telse\n-\t\t\t\tsha1=$(git rev-parse $shortsha1)\n \t\t\t\tif test -z \"$REBASE_ROOT\"\n \t\t\t\tthen\n \t\t\t\t\tpreserve=t\n@@ -914,7 +912,7 @@ 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\tgit log -1 --abbrev-commit --abbrev=7 --format=\"pick %h %s\" $sha1 > \"$TODO\"\n \t\t\t\tfi\n \t\t\tfi\n \t\tdone\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 47ca88f..78d0041 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -648,4 +648,16 @@ test_expect_success 'rebase-i history with funny messages' '\n \ttest_cmp expect actual\n '\n \n+cat >expect_header <<EOF\n+  Commit header with beginning whitespace\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+\tEXPECT_HEADER=\"expect_header\" git rebase -i HEAD~1\n+'\n+\n test_done\n-- \n1.7.2.rc2\n"},{"id":"145276","messageId":"AANLkTima-RPBCtoxo8ybli4mBCR12qIjbmBmTQd80B46@mail.gmail.com","threadId":"24357","inReplyTo":"1278764821-32647-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":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-10T16:35:04Z","receivedAt":"2010-07-10T16:35:04Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Sat, Jul 10, 2010 at 8:27 PM, Nazri Ramliy <ayiehere@gmail.com> wrote:\n> -                               echo \"pick $shortsha1 $rest\" >> \"$TODO\"\n> +                               git log -1 --abbrev-commit --abbrev=7 --format=\"pick %h %s\" $sha1 > \"$TODO\"\n[snip]\n> -                                       echo \"pick $shortsha1 $rest\" >> \"$TODO\"\n> +                                       git log -1 --abbrev-commit --abbrev=7 --format=\"pick %h %s\" $sha1 > \"$TODO\"\n\nNotice the change from \">>\" to \">\"? I'm stupid :)\n\nPlease drop this patch. I'll resend the corrected one.\n\nnazri.\n"},{"id":"145344","messageId":"7vbpadfd4r.fsf@alter.siamese.dyndns.org","threadId":"24357","inReplyTo":"1278764821-32647-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-12T06:05:56Z","receivedAt":"2010-07-12T06:05:56Z","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> During \"git rebase -i\", when the commit headers are shown in the editor\n> for action (pick/squash/etc), the whitespace (if any) at the beginning\n> of commit headers are stripped out due to the use of \"read shortsha1 rest\"\n> for reading the output of \"git rev-list\".\n>\n> The missing beginning whitespace do not pose any harm but this could be\n\nIf the current code removes whitespaces at the beginning, I would actually\nsay that it is cleaning up the mess while preparing the instruction sheet\nfor you to edit, i.e. it is a good thing, and the patch might be making\nthings worse.\n\nI find it difficult to come up with good reasons to convince myself that I\nshould be interested in what this patch tries to do.  Here are some of the\nthings that came to my mind while doing so.\n\nWhat happens if you have trailing whitespaces, excess whitespaces in the\nmiddle, etc. with or without this patch?  What _should_ happen in an ideal\nworld?\n\nWhat happens if you have a malformed commit object whose first line is\nblank (i.e. no \"Subject\" line), or there is _no_ commit log message\nwhatsoever with or without this patch?  What _should_ happen in an ideal\nworld?\n"},{"id":"145345","messageId":"7v7hl1fd0n.fsf@alter.siamese.dyndns.org","threadId":"24357","inReplyTo":"1278764821-32647-1-git-send-email-ayiehere@gmail.com","subject":"Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-12T06:08:24Z","receivedAt":"2010-07-12T06:08:24Z","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> +test -z \"$EXPECT_HEADER\" ||\n> +\t(\n> +\t\tgrep '^pick' < \"$1\" | cut -d' ' -f3- > commit_headers.$$ &&\n\nSending output from grep to cut does not sound very cool; wouldn't a\nsingle \"sed\" invocation more appropriate?\n\n> +\t\tdiff \"$EXPECT_HEADER\" commit_headers.$$ > /dev/null\n\nIs \"test_cmp\" inappropriate here for some reason?\n\n> +\t) ||\n\nDo you need a subshell for this, or just a grouping {} sufficient?\n\n> +\texit\n"},{"id":"145407","messageId":"AANLkTin03Ea2fIBQiSDpIqRYLYjWRe9U62nZpfczbfST@mail.gmail.com","threadId":"24357","inReplyTo":"7v7hl1fd0n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-13T00:36:59Z","receivedAt":"2010-07-13T00:36:59Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Mon, Jul 12, 2010 at 2:08 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nazri Ramliy <ayiehere@gmail.com> writes:\n>\n>> +test -z \"$EXPECT_HEADER\" ||\n>> +     (\n>> +             grep '^pick' < \"$1\" | cut -d' ' -f3- > commit_headers.$$ &&\n>\n> Sending output from grep to cut does not sound very cool; wouldn't a\n> single \"sed\" invocation more appropriate?\n\nIf I were to use a single \"sed\" invocation here's how it's going to look like:\n\n\tsed -e \"s/^pick [0-9a-f]\\+//\" < \"$1\" > commit_headers.$$\n\nBut doing so reminds me of this:\n\nOn 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\nAren't we back to square one? Or am I missing something?\n\n>> +             diff \"$EXPECT_HEADER\" commit_headers.$$ > /dev/null\n>\n> Is \"test_cmp\" inappropriate here for some reason?\n\nIt seems appropriate, but this will require a \". ./test-lib.sh\" from inside\nlib-rebase.sh, which fails because at that point we are already in the trash\ndirectory for the test, the solution is to do a \". $TEST_DIRECTORY/test-lib.sh\"\ninstead but that gives more errors due to test-lib.sh assumes that it is always\nbeing \"sourced\" when $CWD is $TEST_DIRECTORY, i think.\n\n>\n>> +     ) ||\n>\n> Do you need a subshell for this, or just a grouping {} sufficient?\n\nYes a grouping {} is sufficient.\n\nNote: The only user of the feature that this patch provides is my patch to\nt3404-rebase-interactive.sh\n(1278896663-3922-2-git-send-email-ayiehere@gmail.com),\nthe rationale of which is discussed in 7vbpadfd4r.fsf@alter.siamese.dyndns.org.\n\nI'll send a reply to that one in a bit.\n\n\nnazri.\n"},{"id":"145415","messageId":"AANLkTik7KDM8Vp_nc1NYRZ89cd1IdyfBffq5vUitbrzj@mail.gmail.com","threadId":"24357","inReplyTo":"AANLkTin03Ea2fIBQiSDpIqRYLYjWRe9U62nZpfczbfST@mail.gmail.com","subject":"Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-07-13T01:39:00Z","receivedAt":"2010-07-13T01:39:00Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Jul 12, 2010 at 8:36 PM, Nazri Ramliy <ayiehere@gmail.com> wrote:\n> If I were to use a single \"sed\" invocation here's how it's going to look like:\n>\n>        sed -e \"s/^pick [0-9a-f]\\+//\" < \"$1\" > commit_headers.$$\n>\n> But doing so reminds me of this:\n>\n> 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> Aren't we back to square one? Or am I missing something?\n\nsed 's/^pick [0-9a-f]\\{1,\\} //'\n\nis a valid posix BRE. Alternately + can be expressed as:\n\nsed 's/^pick [0-9a-f][0-9a-f]* //'\n\n\n>>> +             diff \"$EXPECT_HEADER\" commit_headers.$$ > /dev/null\n>>\n>> Is \"test_cmp\" inappropriate here for some reason?\n>\n> It seems appropriate, but this will require a \". ./test-lib.sh\" from inside\n> lib-rebase.sh, which fails because at that point we are already in the trash\n> directory for the test, the solution is to do a \". $TEST_DIRECTORY/test-lib.sh\"\n> instead but that gives more errors due to test-lib.sh assumes that it is always\n> being \"sourced\" when $CWD is $TEST_DIRECTORY, i think.\n\nDon't the scripts that source lib-rebase.sh all source test-lib.sh\nahead of it? lib-rebase.sh shouldn't need to re-source test-lib.sh.\nNo?\n\nj.\n"},{"id":"145416","messageId":"AANLkTin9vUliwD5Ufa1vMTC2RZ7TSiyNaPmnDNtz1FQd@mail.gmail.com","threadId":"24357","inReplyTo":"7vbpadfd4r.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-13T01:46:58Z","receivedAt":"2010-07-13T01:46:58Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Mon, Jul 12, 2010 at 2:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> If the current code removes whitespaces at the beginning, I would actually\n> say that it is cleaning up the mess while preparing the instruction sheet\n> for you to edit, i.e. it is a good thing, and the patch might be making\n> things worse.\n\nConsider this situation:\n\n\t$ git log --reverse --oneline HEAD~3..\n\tcafebabe a commit header\n\tdeadbeef   badly formatted commit header\n\tfalaafe1 another commit header\n\nWithout the patch (let's call this \"exhibit 1\"):\n\n\t$ GIT_EDITOR=cat git rebase -i HEAD~3\n\tcafebabe a commit header\n\tdeadbeef badly formatted commit header\n\tfa1aafe1 another commit header\n\nWith the patch (and let's call this \"exhibit 2\"):\n\n\t$ GIT_EDITOR=cat git rebase -i HEAD~3\n\tcafebabe a commit header\n\tdeadbeef    badly formatted commit header\n\tfa1aafe1 another commit header\n\n> I find it difficult to come up with good reasons to convince myself that I\n> should be interested in what this patch tries to do.\n\nI guess it boils down to your answer to this:\n\nWhen you want to reword the badly formatted commit header, would you\nprefer the see the output like the one shown in exhibit 1 over that of\nexhibit 2 in your $GIT_EDITOR?\n\nThe example I gave has only one commit header with beginning whitespace.\nWhat if for some reason you end up with more than a couple of commits\nheaders that have whitespaces at the beginning and you would like to\nrebase them to fix it and what you see is the one similar to exhibit 1?\n\nIsn't it harder to distinguish those badly formatted commits in that case?\n\n> Here are some of the things that came to my mind while doing so.\n>\n> What happens if you have trailing whitespaces, excess whitespaces in the\n> middle, etc. with or without this patch?  What _should_ happen in an ideal\n> world?\n\nTrailing/excess whitespaces are non issue. The intention of this patch is to\npresent the commit headers \"as-is\".\n\n> What happens if you have a malformed commit object whose first line is\n> blank (i.e. no \"Subject\" line),or there is _no_ commit log message\n> whatsoever with or without this patch?  What _should_ happen in an ideal\n> world?\n\nWhen there is no commit log message the behavior is the same with or\nwithout this patch.\n\nI can't test for when the commit has no \"Subject\" line because I don't\nknow how to make one.  One thing for sure that in this case, with this\npatch, what is shown in $GIT_EDITOR will be exactly the same as what\n\n        $ git log $sha1_for_that_commit --format=\"pick %h %s\"\n\nshows.\n\nIf you agree with my arguments here then I'll proceed with modifying the\nfirst patch in this series as per the discussion in the thread started by\n1278764821-32647-1-git-send-email-ayiehere@gmail.com.\n\nnazri\n"},{"id":"145417","messageId":"AANLkTikZaPR0iaUkREU83EAyYPbuTvsB2frksHqLSX41@mail.gmail.com","threadId":"24357","inReplyTo":"AANLkTik7KDM8Vp_nc1NYRZ89cd1IdyfBffq5vUitbrzj@mail.gmail.com","subject":"Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-07-13T01:58:35Z","receivedAt":"2010-07-13T01:58:35Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Tue, Jul 13, 2010 at 8:50 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Come on ;-) The most portable, traditional and straightforward way to\n> spell that would be \"[a-z][a-z]*\", no?\n\nOn Tue, Jul 13, 2010 at 9:39 AM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> sed 's/^pick [0-9a-f]\\{1,\\} //'\n>\n> is a valid posix BRE. Alternately + can be expressed as:\n>\n> sed 's/^pick [0-9a-f][0-9a-f]* //'\n\nThanks for the cluestick guys.\n\nOn Tue, Jul 13, 2010 at 8:50 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Somebody is already using test_set_editor inside lib-rebase.sh --- where\n> else does it come from other than test-lib?  Don't the scripts that\n> include lib-rebase already include test-lib.sh to make test_cmp available\n> to you?\n\nOn Tue, Jul 13, 2010 at 9:39 AM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> Don't the scripts that source lib-rebase.sh all source test-lib.sh\n> ahead of it? lib-rebase.sh shouldn't need to re-source test-lib.sh.\n> No?\n\nAh, sorry for not reporting the exact cause of the problem, it is\nactually due to the fact that test_cmp will be called (later) by\n\"fake-editor.sh\", which does not source test-lib.sh, and sourcing\ntest-lib.sh will not be a correct solution in that case yes?\n\nnazri.\n"},{"id":"145421","messageId":"7vzkxw3r8f.fsf@alter.siamese.dyndns.org","threadId":"24357","inReplyTo":"AANLkTin9vUliwD5Ufa1vMTC2RZ7TSiyNaPmnDNtz1FQd@mail.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-13T05:06:40Z","receivedAt":"2010-07-13T05:06: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> I guess it boils down to your answer to this:\n>\n> When you want to reword the badly formatted commit header, would you\n> prefer the see the output like the one shown in exhibit 1 over that of\n> exhibit 2 in your $GIT_EDITOR?\n\nWhat I was getting at was if you want to add a special case _only_ for\ncommits that have indented subject line, and do not care about ones that\nare missing the subject line, excess whitespace in the middle or at the\nend, or no commit log message.  That is why I asked the \"What happens\"\nquestions.  For example, if neither the current nor the updated code\nremoves excess whitespace in the middle or at the end, then that becomes a\nnon-issue.\n\nI just checked.  You can create \"funny commits\" of various shape by\nmisusing hash-object (and presumably importing from foreign SCMs with\ncustom importers).  Since the topic here is about _fixing_ such mistakes,\nseeing how we handle commits that have different but similar breakages is\nvery relevant before going forward.\n\n - We do keep excess whitespaces in the middle, so they are easy to spot.\n\n - A commit without any message is shown without _any_ message in the\n   list, so presumably we can identify them (if there are many, you would\n   want to fix them all anyway).\n\n - We drop trailing whitespaces, so it is impossible to spot a commit with\n   such a subject line.  The user who wants to fix these breakages in the\n   commit log messages presumably is intelligent enough to check with \"git\n   cat-file commit $that_commit | cat -e\" to find such a commit, make a\n   mental note of what the log message of the broken commit said, and then\n   identify the one with the same message when editing the instruction\n   sheet for the rebase-i, so it is not the end of the world, though.\n\n - A commit with an extra blank line is not shown any differently, so it\n   is impossible to identify.  The \"remember the message to identify\"\n   trick would work very well, though.\n\n - A commit with a space at the begining gets that excess space stripped,\n   so it is impossible to identify.  Again, the \"remember the message to\n   identify\" trick would work very well.\n\nSo the bigger picture laid out here shows that there are five cases,\nincluding your \"SP at the beginning\" which is the last one.\n\nAmong these five, two are non-issues, and the remaining three share the\nsame \"impossible to spot if you _only_ look at the insn sheet, but it is\neasy to remember which ones to edit\" characteristics.  Is the change that\nonly keeps the excess whitespaces at the beginning such an improvement\nworth the code churn?\n\nTo put it differently, if we really do perceive this \"impossible to spot\nif you _only_ look at the insn sheet, but it is easy to remember which\nones to edit\" as an issue, shouldn't the patch at least mention that it\nattempts to solve only one and punts on the other two that are still to be\nfixed later (so that other people can come back to help)?\n"},{"id":"145423","messageId":"7vvd8k3qwm.fsf@alter.siamese.dyndns.org","threadId":"24357","inReplyTo":"AANLkTikZaPR0iaUkREU83EAyYPbuTvsB2frksHqLSX41@mail.gmail.com","subject":"Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-13T05:13:45Z","receivedAt":"2010-07-13T05:13:45Z","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> On Tue, Jul 13, 2010 at 9:39 AM, Jay Soffian <jaysoffian@gmail.com> wrote:\n>> Don't the scripts that source lib-rebase.sh all source test-lib.sh\n>> ahead of it? lib-rebase.sh shouldn't need to re-source test-lib.sh.\n>> No?\n>\n> Ah, sorry for not reporting the exact cause of the problem, it is\n> actually due to the fact that test_cmp will be called (later) by\n> \"fake-editor.sh\", which does not source test-lib.sh, and sourcing\n> test-lib.sh will not be a correct solution in that case yes?\n\nAhh, I see.\n\nAs we _only_ care about the files being equal (and we do not want any\ninput from that context even when we are running in verbose mode), I think\nit is probably Ok to use bare \"diff\" like your patch did.  Thanks for an\nexplanation.\n\nPeople with platforms with a broken \"diff\" implementations, PLEASE\ncomplain LOUDLY if your \"diff\" is not even capable of comparing two\nregular files and report if they are the same with its exit status.\n"},{"id":"145424","messageId":"AANLkTinY9LGB1v8WdwJvFr8f-InaqFx5BozsAH2xlVBQ@mail.gmail.com","threadId":"24357","inReplyTo":"7vzkxw3r8f.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-13T05:19:16Z","receivedAt":"2010-07-13T05:19:16Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Tue, Jul 13, 2010 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> To put it differently, if we really do perceive this \"impossible to spot\n> if you _only_ look at the insn sheet, but it is easy to remember which\n> ones to edit\" as an issue, shouldn't the patch at least mention that it\n> attempts to solve only one and punts on the other two that are still to be\n> fixed later (so that other people can come back to help)?\n\nThanks for spending your time to clarify the issues.  I'll see if I\ncan improve the commit\nmessage and/or the patch to handle those issues.\n\nnazri.\n"}]}