From: Jonathan Nieder <jrnieder@gmail.com>
To: Thomas Rast <trast@student.ethz.ch>
Cc: Junio C Hamano <gitster@pobox.com>,
Scott Johnson <scottj75074@yahoo.com>,
Michael J Gruber <git@drmicha.warpmail.net>,
Matthijs Kooijman <matthijs@stdin.nl>,
git@vger.kernel.org
Subject: [PATCH 3/3] t4034 (diff --word-diff): style suggestions
Date: Tue, 11 Jan 2011 15:49:57 -0600 [thread overview]
Message-ID: <20110111214957.GD29133@burratino> (raw)
In-Reply-To: <20110111214707.GA29133@burratino>
Rearrange code to be easier to browse:
- first data
- then functions
- then test assertions
Mark up inline test vectors as
cat >vector <<-\EOF
data
data
EOF
for visual scannability. Use words like "set up" for tests that set
up for other tests, to make it obvious which tests are safe to skip.
Use repeated function calls instead of a loop for the
language-specific tests, so the invocations can be easily tweaked
individually (for example if one starts to fail).
This means if you add a new subdirectory to t4034/, it will not be
automatically used. I think that's worth it for the added
explicitness.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
t/t4034-diff-words.sh | 476 ++++++++++++++++++++++--------------------------
1 files changed, 218 insertions(+), 258 deletions(-)
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 2647191..c3b1c48 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -4,346 +4,306 @@ test_description='word diff colors'
. ./test-lib.sh
+cat >pre.simple <<-\EOF
+ h(4)
+
+ a = b + c
+EOF
+cat >post.simple <<-\EOF
+ h(4),hh[44]
+
+ a = b + c
+
+ aa = a
+
+ aeff = aeff * ( aaa )
+EOF
+cat >expect.letter-runs-are-words <<-\EOF
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1,3 +1,7 @@<RESET>
+ h(4),<GREEN>hh<RESET>[44]
+
+ a = b + c<RESET>
+
+ <GREEN>aa = a<RESET>
+
+ <GREEN>aeff = aeff * ( aaa<RESET> )
+EOF
+cat >expect.non-whitespace-is-word <<-\EOF
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1,3 +1,7 @@<RESET>
+ h(4)<GREEN>,hh[44]<RESET>
+
+ a = b + c<RESET>
+
+ <GREEN>aa = a<RESET>
+
+ <GREEN>aeff = aeff * ( aaa )<RESET>
+EOF
+
+word_diff () {
+ test_must_fail git diff --no-index "$@" pre post >output &&
+ test_decode_color <output >output.decrypted &&
+ test_cmp expect output.decrypted
+}
+
+test_language_driver () {
+ lang=$1
+ test_expect_success "diff driver '$lang'" '
+ cp "$TEST_DIRECTORY/t4034/'"$lang"'/pre" \
+ "$TEST_DIRECTORY/t4034/'"$lang"'/post" \
+ "$TEST_DIRECTORY/t4034/'"$lang"'/expect" . &&
+ echo "* diff='"$lang"'" >.gitattributes &&
+ word_diff --color-words
+ '
+}
+
test_expect_success setup '
-
git config diff.color.old red &&
git config diff.color.new green &&
git config diff.color.func magenta
-
'
-word_diff () {
- test_must_fail git diff --no-index "$@" pre post > output &&
- test_decode_color <output >output.decrypted &&
- test_cmp expect output.decrypted
-}
-
-cat > pre <<\EOF
-h(4)
-
-a = b + c
-EOF
-
-cat > post <<\EOF
-h(4),hh[44]
-
-a = b + c
-
-aa = a
-
-aeff = aeff * ( aaa )
-EOF
-
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,3 +1,7 @@<RESET>
-<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
-
-a = b + c<RESET>
-
-<GREEN>aa = a<RESET>
-
-<GREEN>aeff = aeff * ( aaa )<RESET>
-EOF
+test_expect_success 'set up pre and post with runs of whitespace' '
+ cp pre.simple pre &&
+ cp post.simple post
+'
test_expect_success 'word diff with runs of whitespace' '
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1,3 +1,7 @@<RESET>
+ <RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
- word_diff --color-words
+ a = b + c<RESET>
-'
-
-test_expect_success '--word-diff=color' '
-
- word_diff --word-diff=color
-
-'
-
-test_expect_success '--color --word-diff=color' '
+ <GREEN>aa = a<RESET>
+ <GREEN>aeff = aeff * ( aaa )<RESET>
+ EOF
+ word_diff --color-words &&
+ word_diff --word-diff=color &&
word_diff --color --word-diff=color
-
'
-sed 's/#.*$//' > expect <<EOF
-diff --git a/pre b/post
-index 330b04f..5ed8eff 100644
---- a/pre
-+++ b/post
-@@ -1,3 +1,7 @@
--h(4)
-+h(4),hh[44]
-~
- # significant space
-~
- a = b + c
-~
-~
-+aa = a
-~
-~
-+aeff = aeff * ( aaa )
-~
-EOF
-
test_expect_success '--word-diff=porcelain' '
-
+ sed 's/#.*$//' >expect <<-\EOF &&
+ diff --git a/pre b/post
+ index 330b04f..5ed8eff 100644
+ --- a/pre
+ +++ b/post
+ @@ -1,3 +1,7 @@
+ -h(4)
+ +h(4),hh[44]
+ ~
+ # significant space
+ ~
+ a = b + c
+ ~
+ ~
+ +aa = a
+ ~
+ ~
+ +aeff = aeff * ( aaa )
+ ~
+ EOF
word_diff --word-diff=porcelain
-
'
-cat > expect <<EOF
-diff --git a/pre b/post
-index 330b04f..5ed8eff 100644
---- a/pre
-+++ b/post
-@@ -1,3 +1,7 @@
-[-h(4)-]{+h(4),hh[44]+}
-
-a = b + c
-
-{+aa = a+}
-
-{+aeff = aeff * ( aaa )+}
-EOF
-
test_expect_success '--word-diff=plain' '
+ cat >expect <<-\EOF &&
+ diff --git a/pre b/post
+ index 330b04f..5ed8eff 100644
+ --- a/pre
+ +++ b/post
+ @@ -1,3 +1,7 @@
+ [-h(4)-]{+h(4),hh[44]+}
- word_diff --word-diff=plain
+ a = b + c
-'
-
-test_expect_success '--word-diff=plain --no-color' '
+ {+aa = a+}
+ {+aeff = aeff * ( aaa )+}
+ EOF
+ word_diff --word-diff=plain &&
word_diff --word-diff=plain --no-color
-
'
-cat > expect <<EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,3 +1,7 @@<RESET>
-<RED>[-h(4)-]<RESET><GREEN>{+h(4),hh[44]+}<RESET>
-
-a = b + c<RESET>
-
-<GREEN>{+aa = a+}<RESET>
-
-<GREEN>{+aeff = aeff * ( aaa )+}<RESET>
-EOF
-
test_expect_success '--word-diff=plain --color' '
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1,3 +1,7 @@<RESET>
+ <RED>[-h(4)-]<RESET><GREEN>{+h(4),hh[44]+}<RESET>
+ a = b + c<RESET>
+
+ <GREEN>{+aa = a+}<RESET>
+
+ <GREEN>{+aeff = aeff * ( aaa )+}<RESET>
+ EOF
word_diff --word-diff=plain --color
-
'
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1 +1 @@<RESET>
-<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
-<CYAN>@@ -3,0 +4,4 @@<RESET> <RESET><MAGENTA>a = b + c<RESET>
-
-<GREEN>aa = a<RESET>
-
-<GREEN>aeff = aeff * ( aaa )<RESET>
-EOF
-
test_expect_success 'word diff without context' '
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1 +1 @@<RESET>
+ <RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
+ <CYAN>@@ -3,0 +4,4 @@<RESET> <RESET><MAGENTA>a = b + c<RESET>
+ <GREEN>aa = a<RESET>
+
+ <GREEN>aeff = aeff * ( aaa )<RESET>
+ EOF
word_diff --color-words --unified=0
-
'
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,3 +1,7 @@<RESET>
-h(4),<GREEN>hh<RESET>[44]
-
-a = b + c<RESET>
-
-<GREEN>aa = a<RESET>
-
-<GREEN>aeff = aeff * ( aaa<RESET> )
-EOF
-cp expect expect.letter-runs-are-words
-
test_expect_success 'word diff with a regular expression' '
-
+ cp expect.letter-runs-are-words expect &&
word_diff --color-words="[a-z]+"
-
'
-test_expect_success 'set a diff driver' '
+test_expect_success 'set up a diff driver' '
git config diff.testdriver.wordRegex "[^[:space:]]" &&
- cat <<EOF > .gitattributes
-pre diff=testdriver
-post diff=testdriver
-EOF
+ cat <<-\EOF >.gitattributes
+ pre diff=testdriver
+ post diff=testdriver
+ EOF
'
test_expect_success 'option overrides .gitattributes' '
-
+ cp expect.letter-runs-are-words expect &&
word_diff --color-words="[a-z]+"
-
'
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,3 +1,7 @@<RESET>
-h(4)<GREEN>,hh[44]<RESET>
-
-a = b + c<RESET>
-
-<GREEN>aa = a<RESET>
-
-<GREEN>aeff = aeff * ( aaa )<RESET>
-EOF
-cp expect expect.non-whitespace-is-word
-
test_expect_success 'use regex supplied by driver' '
-
+ cp expect.non-whitespace-is-word expect &&
word_diff --color-words
-
'
-test_expect_success 'set diff.wordRegex option' '
+test_expect_success 'set up diff.wordRegex option' '
git config diff.wordRegex "[[:alnum:]]+"
'
-cp expect.letter-runs-are-words expect
-
test_expect_success 'command-line overrides config' '
+ cp expect.letter-runs-are-words expect &&
word_diff --color-words="[a-z]+"
'
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,3 +1,7 @@<RESET>
-h(4),<GREEN>{+hh+}<RESET>[44]
-
-a = b + c<RESET>
-
-<GREEN>{+aa = a+}<RESET>
-
-<GREEN>{+aeff = aeff * ( aaa+}<RESET> )
-EOF
-
test_expect_success 'command-line overrides config: --word-diff-regex' '
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1,3 +1,7 @@<RESET>
+ h(4),<GREEN>{+hh+}<RESET>[44]
+
+ a = b + c<RESET>
+
+ <GREEN>{+aa = a+}<RESET>
+
+ <GREEN>{+aeff = aeff * ( aaa+}<RESET> )
+ EOF
word_diff --color --word-diff-regex="[a-z]+"
'
-cp expect.non-whitespace-is-word expect
-
test_expect_success '.gitattributes override config' '
+ cp expect.non-whitespace-is-word expect &&
word_diff --color-words
'
-test_expect_success 'remove diff driver regex' '
- git config --unset diff.testdriver.wordRegex
+test_expect_success 'setup: remove diff driver regex' '
+ test_might_fail git config --unset diff.testdriver.wordRegex
'
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 330b04f..5ed8eff 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,3 +1,7 @@<RESET>
-h(4),<GREEN>hh[44<RESET>]
-
-a = b + c<RESET>
-
-<GREEN>aa = a<RESET>
-
-<GREEN>aeff = aeff * ( aaa<RESET> )
-EOF
-
test_expect_success 'use configured regex' '
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 330b04f..5ed8eff 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1,3 +1,7 @@<RESET>
+ h(4),<GREEN>hh[44<RESET>]
+
+ a = b + c<RESET>
+
+ <GREEN>aa = a<RESET>
+
+ <GREEN>aeff = aeff * ( aaa<RESET> )
+ EOF
word_diff --color-words
'
-echo 'aaa (aaa)' > pre
-echo 'aaa (aaa) aaa' > post
-
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index c29453b..be22f37 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1 +1 @@<RESET>
-aaa (aaa) <GREEN>aaa<RESET>
-EOF
-
test_expect_success 'test parsing words for newline' '
-
+ echo "aaa (aaa)" >pre &&
+ echo "aaa (aaa) aaa" >post &&
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index c29453b..be22f37 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1 +1 @@<RESET>
+ aaa (aaa) <GREEN>aaa<RESET>
+ EOF
word_diff --color-words="a+"
-
-
'
-echo '(:' > pre
-echo '(' > post
-
-cat > expect <<\EOF
-<BOLD>diff --git a/pre b/post<RESET>
-<BOLD>index 289cb9d..2d06f37 100644<RESET>
-<BOLD>--- a/pre<RESET>
-<BOLD>+++ b/post<RESET>
-<CYAN>@@ -1 +1 @@<RESET>
-(<RED>:<RESET>
-EOF
-
test_expect_success 'test when words are only removed at the end' '
-
+ echo "(:" >pre &&
+ echo "(" >post &&
+ cat >expect <<-\EOF &&
+ <BOLD>diff --git a/pre b/post<RESET>
+ <BOLD>index 289cb9d..2d06f37 100644<RESET>
+ <BOLD>--- a/pre<RESET>
+ <BOLD>+++ b/post<RESET>
+ <CYAN>@@ -1 +1 @@<RESET>
+ (<RED>:<RESET>
+ EOF
word_diff --color-words=.
-
'
-cat > expect <<\EOF
-diff --git a/pre b/post
-index 289cb9d..2d06f37 100644
---- a/pre
-+++ b/post
-@@ -1 +1 @@
--(:
-+(
-EOF
-
test_expect_success '--word-diff=none' '
-
+ echo "(:" >pre &&
+ echo "(" >post &&
+ cat >expect <<-\EOF &&
+ diff --git a/pre b/post
+ index 289cb9d..2d06f37 100644
+ --- a/pre
+ +++ b/post
+ @@ -1 +1 @@
+ -(:
+ +(
+ EOF
word_diff --word-diff=plain --word-diff=none
-
'
-word_diff_for_language () {
- cp "$TEST_DIRECTORY/t4034/$1/pre" \
- "$TEST_DIRECTORY/t4034/$1/post" \
- "$TEST_DIRECTORY/t4034/$1/expect" . &&
- echo "* diff=$1" >.gitattributes &&
- word_diff --color-words && cp output output.$1
-}
-
-for lang_dir in $TEST_DIRECTORY/t4034/*; do
- lang=${lang_dir#$TEST_DIRECTORY/t4034/}
- test_expect_success "diff driver '$lang' has sane word regex" "
- word_diff_for_language $lang
- "
-done
+test_language_driver bibtex
+test_language_driver cpp
+test_language_driver csharp
+test_language_driver fortran
+test_language_driver html
+test_language_driver java
+test_language_driver objc
+test_language_driver pascal
+test_language_driver php
+test_language_driver python
+test_language_driver ruby
+test_language_driver tex
test_done
--
1.7.4.rc1
next prev parent reply other threads:[~2011-01-11 21:50 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-15 3:47 html userdiff is not showing all my changes Scott Johnson
2010-12-15 9:06 ` Michael J Gruber
2010-12-15 9:12 ` Matthijs Kooijman
2010-12-15 9:29 ` Michael J Gruber
2010-12-15 15:13 ` [PATCH 0/4] --word-regex sanity checking and such Thomas Rast
2010-12-15 15:13 ` [PATCH 1/4] diff.c: pass struct diff_words into find_word_boundaries Thomas Rast
2010-12-15 15:13 ` [PATCH 2/4] diff.c: implement a sanity check for word regexes Thomas Rast
2010-12-15 15:13 ` [PATCH 3/4] userdiff: fix typo in ruby word regex Thomas Rast
2010-12-15 15:13 ` [PATCH 4/4] t4034: bulk verify builtin word regex sanity Thomas Rast
[not found] ` <913156.57703.qm@web110711.mail.gq1.yahoo.com>
2010-12-15 19:51 ` [PATCH 0/4] --word-regex sanity checking and such Thomas Rast
2010-12-15 20:48 ` Scott Johnson
2010-12-18 16:17 ` [PATCH v2 " Thomas Rast
2010-12-18 16:17 ` [PATCH v2 1/4] diff.c: pass struct diff_words into find_word_boundaries Thomas Rast
2010-12-18 16:17 ` [PATCH v2 2/4] diff.c: implement a sanity check for word regexes Thomas Rast
2010-12-18 21:00 ` Junio C Hamano
2010-12-19 1:59 ` Thomas Rast
2010-12-18 16:17 ` [PATCH v2 3/4] userdiff: fix typo in ruby and python " Thomas Rast
2010-12-18 21:02 ` Junio C Hamano
2010-12-19 2:10 ` Thomas Rast
2010-12-18 16:17 ` [PATCH v2 4/4] t4034: bulk verify builtin word regex sanity Thomas Rast
2011-01-11 21:47 ` [RFC/PATCH 0/3] " Jonathan Nieder
2011-01-11 21:48 ` [PATCH 1/3] " Jonathan Nieder
2011-01-18 18:00 ` Re*: " Junio C Hamano
2011-01-11 21:48 ` [PATCH 2/3] userdiff: simplify word-diff safeguard Jonathan Nieder
2011-01-11 21:49 ` Jonathan Nieder [this message]
2010-12-18 16:24 ` [PATCH v2 0/4] --word-regex sanity checking and such Thomas Rast
2010-12-18 20:48 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110111214957.GD29133@burratino \
--to=jrnieder@gmail.com \
--cc=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=matthijs@stdin.nl \
--cc=scottj75074@yahoo.com \
--cc=trast@student.ethz.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).