On 02.08.2012 23:00, Junio C Hamano wrote:
Cool, no probs. I had originally put them at the start of the first test that I added but decided to pull them out as prep. I think that msysGit or something about my Windows shell session may have played a part in my not chaining them with && also (see below). I'll clean them up and wrap them in a test_expect_success.Adam Butcher <dev.lists@xxxxxxxxxxxxxxx> writes:+# create a file containing numbers with no newline at +# the end and modify it such that the starting 10 lines +# are unchanged, the next 101 are rewritten and the last +# line differs only in that in is terminated by a newline. +seq 1 10 > seq +seq 100 +1 200 >> seq +printf 201 >> seq +(git add seq; git commit seq -m seq) >/dev/null +seq 1 10 > seq +seq 300 -1 200 >> seqWe would prefer to have these set-up steps in test_expect_success. That way, we will have more chance to catch potential and unintended breakage to "git add" and "git commit" when people attempt to update them.
Also, the redirect target sticks to redirect operator in our scripts, i.e. "cmd >seq" not "cmd > seq".
Okay, will change.
Hmm, (?confused?) yeah actually I didn't think it did -- I was surprised when git returned 1 for this line. I think it must have been an issue with the version msysGit I was using or something sticking errorlevel in my Windows shell. Git seemed to return 1 ALWAYS! I usually use gnu/linux but on this occasion I wrote the fix and tests blind on a Windows machine testing the logic manually with msysGit. I ran the tests on a linux machine at work and they did what I expected so I left them as was without rechecking this.+test_expect_success 'no newline at eof is on its own line without -B'+ + (git diff seq; true) > res &&What is this subshell and true about? A git diff does not exit with non zero to signal differences,
I'm glad that this can be simplified. It felt wrong -- similar lines elsewhere in the script didn't do it so I wasn't really happy with it. Turns out it looks to be a Windows/environment issue. I cannot reproduce it now.
and even if it did, the right way to write it would be test_might_fail git cmd >res &&
Fair enough. Good to know that's available.
Okay no probs. I was originally going to spell it out only once and use parameter expansion. However I understand the point of not spelling it out at all. The only reason I did so was to catch other potential errors where text may have been 'attached' to either side of the annotation string by some future (or other) bug.to allow us to make sure that the git command that may or may not exit with zero still does not die an uncontrolled death (e.g. segv).+ grep "^\\\\ No newline at end of file$" res && + grep -v "^.\\+\\\\ No newline at end of file" res && + grep -v "\\\\ No newline at end of file.\\+$" res +'It is preferrable not to spell "No newline at ..." part out, so that we won't have to worry about future rewords and i18n.
Agreed. Should I just test for this prefix case (i.e. the bug at hand) only and not preempt future potential issues? Or should I just stick the current string in a variable and keep the logic as is; at least then there would only be one place requiring a fix in the reword case (but still additional rework in the i18n case -- though I assume the test could force a particular locale to evade this).There are older tests that predate i18n and they do spell these out, but that is not a good reason to make things worse than they already are.
"git apply" only looks at the backslash-space at the beginning of line anyway.
+test_expect_success 'no newline at eof is on its own line with -B' '+ + (git diff -B seq; true) > res && + grep "^\\\\ No newline at end of file$" res && + grep -v "^.\\+\\\\ No newline at end of file" res && + grep -v "\\\\ No newline at end of file.\\+$" res +'Likewise.test_doneThanks.
No probs. I will address both your and Jeff's comments sometime tomorrow and hopefully send a well formatted patch in next time.
Cheers, Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
[Newbies FAQ] [Linux Kernel Development] [Free Online Dating] [Gcc Help] [IETF Annouce] [DCCP] [Netdev] [Networking] [Security] [V4L] [Bugtraq] [Free Online Dating] [Photo] [Yosemite] [MIPS Linux] [ARM Linux] [Linux Security] [Linux RAID] [Linux SCSI] [Fedora Users] [Linux Resources]