Re: [PATCH 1bis/2] Diff patterns for POSIX shells

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 3, 2011 at 11:32 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Wed, Aug 03, 2011 at 07:26:16AM +0200, Giuseppe Bilotta wrote:
>
>> All diffs following a function definition will have that function name
>> as chunck header, but this is the best we can do with the current
>> userdiff capabilities.
>
> Curious as to how this would look in git.git, I tried "git log -p"
> before and after your patches, and diffed the result. I noticed two
> things:
>
>  1. Given a block of shell code like this:
>
>        foo() {
>          ... do something ...
>        }
>
>        test_expect_success 'test foo' '
>          ... the actual test ...
>        '
>
>     if we add new code after the test, the old regex would print:
>
>        @@ -1,2 +3,4 @@ test_expect_success 'test foo' '
>
>     and now we say:
>
>        @@ -1,2 +3,4 @@ foo
>
>     which seems more misleading. I know the function-matching code has
>     no way to say "look for ^}, which signals end of function", so we
>     can't be entirely accurate. But I wonder if the new heuristic
>     (which seems to look for a name followed by parentheses) is
>     actually any better than the old.

I'm not too satisfied with the solution either. I've been thinking
about adding some important keywords such as for, while, if, until,
case etc, but decided it would be too much overkill. And, it still
woudln't work 'correctly' in a case such as this one you presented
above.

>  2. What would have printed before:
>
>       @@ -1,2 +3,4 @@ foo() {
>
>     now prints
>
>       @@ -1,2 +3,4 @@ foo
>
>     without the parentheses or brace. It looks like the similar C one
>     keeps the parentheses, at least. I find that a bit more readable,
>     as it is more clear that the line indicates a function, and not
>     simply some top-level command.

Indeed. I'll change the regexp to include the parenthesis.

-- 
Giuseppe "Oblomov" Bilotta
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]