Re: [PATCH] Improve diff pattern for PHP files

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2020, 18:55 UTC
Message-ID: <xmqqpn5v5gml.fsf@gitster.c.googlers.com>
In-Reply-To: <d262c797-29eb-1e08-8de0-6a2dd2a5d47f@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 24 quoted lines

> Please choose a commit summary line of the form "area: summary", for
> example:
>
>    userdiff: PHP: catch "abstract" and "final" functions
>
> Am 06.10.20 um 12:37 schrieb Javier Spagnoletti via GitGitGadget:
>> From: Javier Spagnoletti <phansys@gmail.com>
>> 
>> Improve the output diff readability for php files by taking into account some missing function modifiers.
>
> "Improve" is a noise word and need not be mentioned; a non-improvement
> would not be accepted. Also, wrap long lines to at most 72 characters.
> Perhaps:
>
>    PHP permits functions to be defined like
>
>        public final function foo() { }
>        protected abstract function bar() { }
>
>    but our hunk header pattern does not recognize these decorations.
>    Add "final" and "abstract" to the list of function modifiers.
>
> I am not a PHP expert, so I cannot tell whether what I have written
> above makes sense.

Me neither, but the new pattern does look like it was written tp specifically allow these combinations. Thanks for a good example log message to guide new developers with.

>>  PATTERNS("php",
>> -	 "^[\t ]*(((public|protected|private|static)[\t ]+)*function.*)$\n"
>> +	 "^[\t ]*(((public|protected|private|static|abstract|final)[\t ]+)*function.*)$\n"
>>  	 "^[\t ]*((((final|abstract)[\t ]+)?class|interface|trait).*)$",

It probably does not matter in practice, but I wondered what the reason for the ordering between final and abstract here (and one line above) ;-)

Show 5 quoted lines

>>  	 /* -- */
>>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>
> It would be great if you could add test cases to t/4018/ that
> demonstrate how these new keywords are handled.

Yes. It seems that we saw another userdiff patch with a test update recently and that may serve as an example, too.

Thanks, both.