From d3621de789ab57739f48b065751089d828b50240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 31 May 2016 22:00:38 +0200 Subject: t4051: rewrite, add more tests Remove the tests that checked against a fixed result and replace them with more focused checks of desired properties of the created diffs. That way we get more detailed and meaningful diagnostics. Store test file contents in files in a subdirectory in order to avoid cluttering the test script with them. Use tagged commits to store the changes to test diff -W against instead of using changes to the worktree. Use the worktree instead to try and apply the generated patch in order to validate it. Document unwanted features: trailing empty lines, too much context for appended functions, insufficient context at the end with -U0. Helped-by: Junio C Hamano Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 216 +++++++++++++++++++++++++-------------- 1 file changed, 142 insertions(+), 74 deletions(-) (limited to 't/t4051-diff-function-context.sh') diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 001d678e09..ca017255ba 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -3,90 +3,158 @@ test_description='diff function context' . ./test-lib.sh -. "$TEST_DIRECTORY"/diff-lib.sh +dir="$TEST_DIRECTORY/t4051" -cat <<\EOF >hello.c -#include - -static int a(void) -{ - /* - * Dummy. - */ +commit_and_tag () { + tag=$1 && + shift && + git add "$@" && + test_tick && + git commit -m "$tag" && + git tag "$tag" } -static int hello_world(void) -{ - /* Classic. */ - printf("Hello world.\n"); - - /* Success! */ - return 0; +first_context_line () { + awk ' + found {print; exit} + /^@@/ {found = 1} + ' } -static int b(void) -{ - /* - * Dummy, too. - */ + +last_context_line () { + sed -ne \$p } -int main(int argc, char **argv) -{ - a(); - b(); - return hello_world(); +check_diff () { + name=$1 + desc=$2 + options="-W $3" + + test_expect_success "$desc" ' + git diff $options "$name^" "$name" >"$name.diff" + ' + + test_expect_success ' diff applies' ' + test_when_finished "git reset --hard" && + git checkout --detach "$name^" && + git apply --index "$name.diff" && + git diff --exit-code "$name" + ' } -EOF test_expect_success 'setup' ' - git add hello.c && - test_tick && - git commit -m initial && - - grep -v Classic hello.c.new && - mv hello.c.new hello.c -' - -cat <<\EOF >expected -diff --git a/hello.c b/hello.c ---- a/hello.c -+++ b/hello.c -@@ -10,8 +10,7 @@ static int a(void) - static int hello_world(void) - { -- /* Classic. */ - printf("Hello world.\n"); - - /* Success! */ - return 0; - } -EOF - -test_expect_success 'diff -U0 -W' ' - git diff -U0 -W >actual && - compare_diff_patch actual expected -' - -cat <<\EOF >expected -diff --git a/hello.c b/hello.c ---- a/hello.c -+++ b/hello.c -@@ -9,9 +9,8 @@ static int a(void) - - static int hello_world(void) - { -- /* Classic. */ - printf("Hello world.\n"); - - /* Success! */ - return 0; - } -EOF - -test_expect_success 'diff -W' ' - git diff -W >actual && - compare_diff_patch actual expected + cat "$dir/includes.c" "$dir/dummy.c" "$dir/dummy.c" "$dir/hello.c" \ + "$dir/dummy.c" "$dir/dummy.c" >file.c && + commit_and_tag initial file.c && + + grep -v "delete me from hello" file.c.new && + mv file.c.new file.c && + commit_and_tag changed_hello file.c && + + grep -v "delete me from includes" file.c.new && + mv file.c.new file.c && + commit_and_tag changed_includes file.c && + + cat "$dir/appended1.c" >>file.c && + commit_and_tag appended file.c && + + cat "$dir/appended2.c" >>file.c && + commit_and_tag extended file.c && + + grep -v "Begin of second part" file.c.new && + mv file.c.new file.c && + commit_and_tag long_common_tail file.c +' + +check_diff changed_hello 'changed function' + +test_expect_success ' context includes begin' ' + grep "^ .*Begin of hello" changed_hello.diff +' + +test_expect_success ' context includes end' ' + grep "^ .*End of hello" changed_hello.diff +' + +test_expect_success ' context does not include other functions' ' + test $(grep -c "^[ +-].*Begin" changed_hello.diff) -le 1 +' + +test_expect_success ' context does not include preceding empty lines' ' + test "$(first_context_line Date: Sat, 28 May 2016 17:00:28 +0200 Subject: xdiff: handle appended chunks better with -W If lines are added at the end of a file, diff -W shows the whole file. That's because get_func_line() only considers the pre-image and gives up if it sees a record index beyond its end. Consider the post-image as well to see if the added lines already make up a full function. If it doesn't then search for the previous function line by starting from the bottom of the pre-image, thereby avoiding to confuse get_func_line(). Reuse the existing label called "again", as it's exactly where we need to jump to when we're done handling the pre-context, but rename it to "post_context_calculation" in order to document its new purpose better. Reported-by: Junio C Hamano Initial-patch-by: Junio C Hamano Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4051-diff-function-context.sh') diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index ca017255ba..4cff119b69 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -131,7 +131,7 @@ test_expect_success ' context includes end' ' grep "^[+].*End of second part" extended.diff ' -test_expect_failure ' context does not include other functions' ' +test_expect_success ' context does not include other functions' ' test $(grep -c "^[ +-].*Begin" extended.diff) -le 2 ' -- cgit v1.2.3 From 392f6d316623e8ecd6210248ba9ae2cabf07352b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 May 2016 17:02:24 +0200 Subject: xdiff: ignore empty lines before added functions with -W If a new function and a preceding empty line is appended, diff -W shows the previous function in full in order to provide context for that empty line. In most languages empty lines between sections are not interesting in and off themselves and showing a whole extra function for them is not what we want. Skip empty lines when checking of the appended chunk starts with a function line, thereby avoiding to extend the context just for them. Helped-by: Ramsay Jones Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4051-diff-function-context.sh') diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 4cff119b69..f7126fc245 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -117,7 +117,7 @@ test_expect_success ' context includes end' ' grep "^[+].*End of first part" appended.diff ' -test_expect_failure ' context does not include other functions' ' +test_expect_success ' context does not include other functions' ' test $(grep -c "^[ +-].*Begin" appended.diff) -le 1 ' -- cgit v1.2.3 From 9e6a4cfc38aa81055d0b7d6fb94dc7b31809daa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 May 2016 17:03:16 +0200 Subject: xdiff: -W: don't include common trailing empty lines in context Empty lines between functions are shown by diff -W, as it considers them to be part of the function preceding them. They are not interesting in most languages. The previous patch stopped showing them in the special case of a function added at the end of a file. Stop extending context to those empty lines by skipping back over them from the start of the next function. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 't/t4051-diff-function-context.sh') diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index f7126fc245..17616fe582 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -85,7 +85,7 @@ test_expect_success ' context does not include preceding empty lines' ' test "$(first_context_line Date: Sat, 28 May 2016 17:04:31 +0200 Subject: xdiff: don't trim common tail with -W The function trim_common_tail() exits early if context lines are requested. If -U0 and -W are specified together then it can still trim context lines that might belong to a changed function. As a result that function is shown incompletely. Fix that by calling trim_common_tail() only if no function context or fixed context is requested. The parameter ctx is no longer needed now; remove it. While at it fix an outdated comment as well. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t4051-diff-function-context.sh') diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 17616fe582..b6bb04ab82 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -145,7 +145,7 @@ test_expect_success ' context includes begin' ' grep "^ .*Begin of first part" long_common_tail.diff ' -test_expect_failure ' context includes end' ' +test_expect_success ' context includes end' ' grep "^ .*End of second part" long_common_tail.diff ' -- cgit v1.2.3 From 6f8d9bccb2c3694c62d14225976689c1e8c50fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Thu, 9 Jun 2016 23:54:48 +0200 Subject: xdiff: fix merging of appended hunk with -W When -W is given we search the lines between the end of the current context and the next change for a function line. If there is none then we merge those two hunks as they must be part of the same function. If the next change is an appended chunk we abort the search early in get_func_line(), however, because its line number is out of range. Fix that by searching from the end of the pre-image in that case instead. Reported-by: Junio C Hamano Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 't/t4051-diff-function-context.sh') diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index b6bb04ab82..b79b87790b 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -64,7 +64,13 @@ test_expect_success 'setup' ' grep -v "Begin of second part" file.c.new && mv file.c.new file.c && - commit_and_tag long_common_tail file.c + commit_and_tag long_common_tail file.c && + + git checkout initial && + grep -v "delete me from hello" file.c.new && + mv file.c.new file.c && + cat "$dir/appended1.c" >>file.c && + commit_and_tag changed_hello_appended file.c ' check_diff changed_hello 'changed function' @@ -157,4 +163,20 @@ test_expect_success ' context does not include preceding empty lines' ' test "$(first_context_line