All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: "David A. Dalrymple (and Bhushan G. Lodha)" <dad-bgl@mit.edu>
Cc: git@vger.kernel.org, peff@peff.net, l.s.r@web.de,
	"David Dalrymple (on zayin)" <davidad@alum.mit.edu>
Subject: Re: [PATCH 09/10] t4213: test --function-name option
Date: Fri, 28 Mar 2014 08:25:45 +0100	[thread overview]
Message-ID: <533523F9.2000108@viscovery.net> (raw)
In-Reply-To: <4eb91822043e730bf8a47f12a9129d02f6fc135d.1395942768.git.davidad@alum.mit.edu>

Am 3/27/2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha):
> From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
> 
> This test builds a sample C file, adding and removing functions, and
> checks that the right commits are filtered by --function-name matching.

This is probably the most important patch in your series as it documents
the expected behavior. Unfortunately, I find its clarity very lacking. :(

This new feature uses the userdiff driver, IIUC. Does it do so in all
respects? In particular, does it also evaluate the negative patterns? For
example, when there is a label in the code, is it not mistaken as the
beginning of a function? A test for this case would be very instructive.

Furthermore, consider a patch for a change at the very beginning of a
function. Then the function name would appear in the pre-context of the
hunk, but the hunk header would show the function before the one with the
change. Would such a change confuse your implementation? I guess not.
Again, a test case would remove any doubts.

Is it possible to search for a change that is before any functions? It
would be useful to enumerate commits that change #include lines.

> 
> Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
> ---
>  t/t4213-log-function-name.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100755 t/t4213-log-function-name.sh
> 
> diff --git a/t/t4213-log-function-name.sh b/t/t4213-log-function-name.sh
> new file mode 100755
> index 0000000..1243ce5
> --- /dev/null
> +++ b/t/t4213-log-function-name.sh
> @@ -0,0 +1,73 @@
> +#!/bin/sh
> +
> +test_description='log --function-name'
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	echo "* diff=cpp" > .gitattributes
> +
> +	>file &&
> +	git add file &&
> +	test_tick &&
> +	git commit -m initial &&
> +
> +	printf "int main(){\n\treturn 0;\n}\n" >> file &&
> +	test_tick &&
> +	git commit -am second

Broken && chain here and later as well. Please be careful.

> +
> +	printf "void newfunc(){\n\treturn;\n}\n" >> file &&
> +	test_tick &&
> +	git commit -am third

	git commit -am "append a function" &&

> +
> +	printf "void newfunc2(){\n\treturn;\n}\n" | cat - file > temp &&
> +	mv temp file &&
> +	test_tick &&
> +	git commit -am fourth

	git commit -am "prepend a function" &&

etc. You get the picture.

> +
> +	printf "void newfunc3(){\n\treturn;\n}\n" | cat - file > temp &&
> +	mv temp file &&
> +	test_tick &&
> +	git commit -am fifth
> +
> +	sed -i -e "s/void newfunc2/void newfunc4/" file &&
> +	test_tick &&
> +	git commit -am sixth
> +'
> +
> +test_expect_success 'log --function-name=main' '

test_expect_success 'log --function-name finds a function with a change' '

> +	git log --function-name=main >actual &&
> +	git log --grep=second >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc\W"' '

test_expect_success 'log --function-name with extended regexp' '

etc. You get the picture.

> +	git log --function-name "newfunc\W" >actual &&
> +	git log --grep=third >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc2"' '
> +	git log --function-name newfunc2 >actual &&
> +	git log -E --grep "sixth|fourth" >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc3"' '
> +	git log --function-name newfunc3 >actual &&
> +	git log --grep=fifth >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc4"' '
> +	git log --function-name newfunc4 >actual &&
> +	git log --grep=sixth >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc"' '
> +	git log --function-name newfunc >actual &&
> +	git log -E --grep "third|fourth|fifth|sixth" >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_done
> 

  reply	other threads:[~2014-03-28  7:25 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-27 18:50 [PATCH 00/10] [RFC] pickaxe for function names David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50 ` [PATCH 01/10] .gitattributes: specify the language used David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50   ` [PATCH 02/10] diffcore-pickaxe.c: refactor regex compilation David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50   ` [PATCH 03/10] diffcore-pickaxe.c: Refactor pickaxe_fn signature David A. Dalrymple (and Bhushan G. Lodha)
2014-04-04 11:09     ` Jakub Narębski
2014-03-27 18:50   ` [PATCH 04/10] diff.c/diff.h: expose userdiff_funcname David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50   ` [PATCH 05/10] diffcore-pickaxe.c: set up funcname pattern David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50   ` [PATCH 06/10] log: --function-name pickaxe David A. Dalrymple (and Bhushan G. Lodha)
2014-04-03 21:25     ` René Scharfe
2014-04-03 21:44       ` Junio C Hamano
2014-04-04 11:15         ` Jakub Narębski
2014-04-04 18:46           ` Junio C Hamano
2014-04-28 20:04             ` Bhushan Lodha
2014-03-27 18:50   ` [PATCH 07/10] xdiff: add XDL_EMIT_MOREFUNCNAMES David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50   ` [PATCH 08/10] xdiff: add XDL_EMIT_MOREHUNKHEADS David A. Dalrymple (and Bhushan G. Lodha)
2014-03-27 18:50   ` [PATCH 09/10] t4213: test --function-name option David A. Dalrymple (and Bhushan G. Lodha)
2014-03-28  7:25     ` Johannes Sixt [this message]
2014-03-28  8:21       ` Eric Sunshine
2014-03-28 11:45       ` Johannes Sixt
2014-04-04 11:21     ` Jakub Narębski
2014-03-27 18:50   ` [PATCH 10/10] Documentation: Document --function-name usage David A. Dalrymple (and Bhushan G. Lodha)
2014-03-28  0:30     ` Eric Sunshine
2014-03-27 19:03 ` [PATCH 00/10] [RFC] pickaxe for function names Jeff King

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=533523F9.2000108@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=dad-bgl@mit.edu \
    --cc=davidad@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.