Re: Odd results writing a Git pre-receive hook to syntax check PHP files.

[Chris Patti <cpatti@gmail.com>]

On Tue, Aug 31, 2010 at 7:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Patti <cpatti@gmail.com> writes:
>
>> function parseCommit($old_sha1,$new_sha1) {
>>         # if this is the first commit on a new branch, $old_sha1 will
>> be a bunch of zeroes, and so
>>         # git diff --raw will fail, since there's no old ref to
>> compare against.  So, we parse the
>>         # results of git diff-tree -root=$new_sha1 instead to get the
>> blob and filename we'll need.
>>         if (detectNewBranch($old_sha1,$new_sha1)) {
>>                 $diffcmd="git diff-tree --root $new_sha1";
>
> This somehow feels wrong.  If the tree of the new commit is flat without
> subdirectory, the above would do, but don't you need "diff-tree -r" here?
>

That's a good point, I'll try creating a subdirectory in my testing,
thanks for pointing this out.

>>                 $regex="/\:\w+ \w+ \w+ (\w+) \w (.+)/";
>>         }
>>         else {
>>                 $diffcmd="git diff --raw $old_sha1 $new_sha1";
>
> And you are better off using "diff-tree -r" here instead of "diff --raw"
> here as well, to keep the input to your parser for both cases uniform.
r> The parser would be easier that way, no?
>
>>                 $regex="/\:\d+ \d+ \w+... (\w+)... \w\t(.+)/";
>>         }
>


As you can see, it's just the regex that differs, so I'm not sure what
you mean by 'parser'.  The power of regular expressions handles this
quite nicely, but I'll definitely consider that for a simplifying
rewrite in the future.

-Chris


-- 
Christopher Patti - Geek At Large | GTalk: cpatti@gmail.com | AIM:
chrisfeohpatti | P: (260) 54PATTI
"Technology challenges art, art inspires technology." - John Lasseter, Pixar