[PATCH] Improve diff pattern for PHP files

[PATCH] Improve diff pattern for PHP files

[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.

Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
---
    Improve diff pattern for PHP files
    
    Improve the output diff readability for php files by taking into account
    some missing function modifiers.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-864%2Fphansys%2Fuserdiff_php-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-864/phansys/userdiff_php-v1
Pull-Request: https://github.com/git/git/pull/864

 userdiff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/userdiff.c b/userdiff.c
index 1df884ef0b..a99d84a7e3 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -143,7 +143,7 @@ PATTERNS("perl",
 	 "|=~|!~"
 	 "|<<|<>|<=>|>>"),
 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).*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"

base-commit: 47ae905ffb98cc4d4fd90083da6bc8dab55d9ecc
-- 
gitgitgadget

Re: [PATCH] Improve diff pattern for PHP files

[Johannes Sixt]

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.

> 
> Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
> ---
>     Improve diff pattern for PHP files
>     
>     Improve the output diff readability for php files by taking into account
>     some missing function modifiers.
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-864%2Fphansys%2Fuserdiff_php-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-864/phansys/userdiff_php-v1
> Pull-Request: https://github.com/git/git/pull/864
> 
>  userdiff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/userdiff.c b/userdiff.c
> index 1df884ef0b..a99d84a7e3 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -143,7 +143,7 @@ PATTERNS("perl",
>  	 "|=~|!~"
>  	 "|<<|<>|<=>|>>"),
>  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).*)$",
>  	 /* -- */
>  	 "[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.

-- Hannes

Re: [PATCH] Improve diff pattern for PHP files

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> 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) ;-)

>>  	 /* -- */
>>  	 "[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.

[PATCH v2] Improve diff pattern for PHP files

[Javier Spagnoletti via GitGitGadget]

From: Javier Spagnoletti <phansys@gmail.com>

PHP allows some function modifiers that are not recognized in our current hunk header pattern

       final public function foo() { }
       abstract protected function bar() { }

Add "final" and "abstract" to the list of function modifiers.

Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
---
    Improve diff pattern for PHP files
    
    PHP allows some function modifiers that are not recognized in our
    current hunk header pattern
    
       final public function foo() { }
       abstract protected function bar() { }
    
    Add "final" and "abstract" to the list of function modifiers. See:
    
     * https://www.php.net/manual/en/language.oop5.abstract.php#example-213;
     * https://www.php.net/manual/en/language.oop5.final.php#language.oop5.traits.static.ex1
       .
    
    Signed-off-by: Javier Spagnoletti phansys@gmail.com [phansys@gmail.com]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-864%2Fphansys%2Fuserdiff_php-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-864/phansys/userdiff_php-v2
Pull-Request: https://github.com/git/git/pull/864

Range-diff vs v1:

 1:  aa65d43a40 ! 1:  4cb59950ff Improve diff pattern for PHP files
     @@ Metadata
       ## Commit message ##
          Improve diff pattern for PHP files
      
     -    Improve the output diff readability for php files by taking into account some missing function modifiers.
     +    PHP allows some function modifiers that are not recognized in our current hunk header pattern
     +
     +           final public function foo() { }
     +           abstract protected function bar() { }
     +
     +    Add "final" and "abstract" to the list of function modifiers.
      
          Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
      
     + ## t/t4018/php-abstract-method (new) ##
     +@@
     ++abstract class Klass
     ++{
     ++    abstract public function RIGHT(): ?string
     ++    {
     ++        return 'ChangeMe';
     ++    }
     ++}
     +
     + ## t/t4018/php-final-method (new) ##
     +@@
     ++class Klass
     ++{
     ++    final public function RIGHT(): string
     ++    {
     ++        return 'ChangeMe';
     ++    }
     ++}
     +
       ## userdiff.c ##
      @@ userdiff.c: PATTERNS("perl",
       	 "|=~|!~"


 t/t4018/php-abstract-method | 7 +++++++
 t/t4018/php-final-method    | 7 +++++++
 userdiff.c                  | 2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/php-abstract-method
 create mode 100644 t/t4018/php-final-method

diff --git a/t/t4018/php-abstract-method b/t/t4018/php-abstract-method
new file mode 100644
index 0000000000..ce215df75a
--- /dev/null
+++ b/t/t4018/php-abstract-method
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}
diff --git a/t/t4018/php-final-method b/t/t4018/php-final-method
new file mode 100644
index 0000000000..537fb8ad9a
--- /dev/null
+++ b/t/t4018/php-final-method
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}
diff --git a/userdiff.c b/userdiff.c
index 1df884ef0b..a99d84a7e3 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -143,7 +143,7 @@ PATTERNS("perl",
 	 "|=~|!~"
 	 "|<<|<>|<=>|>>"),
 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).*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"

base-commit: 47ae905ffb98cc4d4fd90083da6bc8dab55d9ecc
-- 
gitgitgadget

Re: [PATCH v2] Improve diff pattern for PHP files

[Junio C Hamano]

"Javier Spagnoletti via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Subject: Re: [PATCH v2] Improve diff pattern for PHP files

You seem to have read J6t's excellent suggestion well but forgot to
retitle.

> From: Javier Spagnoletti <phansys@gmail.com>
>
> PHP allows some function modifiers that are not recognized in our current hunk header pattern
>
>        final public function foo() { }
>        abstract protected function bar() { }
>
> Add "final" and "abstract" to the list of function modifiers.
>
> Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
> ...
>  t/t4018/php-abstract-method | 7 +++++++
>  t/t4018/php-final-method    | 7 +++++++
>  userdiff.c                  | 2 +-
>  3 files changed, 15 insertions(+), 1 deletion(-)
>  create mode 100644 t/t4018/php-abstract-method
>  create mode 100644 t/t4018/php-final-method
>
> diff --git a/t/t4018/php-abstract-method b/t/t4018/php-abstract-method
> new file mode 100644
> index 0000000000..ce215df75a
> --- /dev/null
> +++ b/t/t4018/php-abstract-method
> @@ -0,0 +1,7 @@
> +abstract class Klass
> +{
> +    abstract public function RIGHT(): ?string
> +    {
> +        return 'ChangeMe';
> +    }
> +}
> diff --git a/t/t4018/php-final-method b/t/t4018/php-final-method
> new file mode 100644
> index 0000000000..537fb8ad9a
> --- /dev/null
> +++ b/t/t4018/php-final-method
> @@ -0,0 +1,7 @@
> +class Klass
> +{
> +    final public function RIGHT(): string
> +    {
> +        return 'ChangeMe';
> +    }
> +}
> diff --git a/userdiff.c b/userdiff.c
> index 1df884ef0b..a99d84a7e3 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -143,7 +143,7 @@ PATTERNS("perl",
>  	 "|=~|!~"
>  	 "|<<|<>|<=>|>>"),
>  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).*)$",
>  	 /* -- */
>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>
> base-commit: 47ae905ffb98cc4d4fd90083da6bc8dab55d9ecc

[PATCH v3] userdiff: php: Add "final" and "abstract" to the list of function modifiers

[Javier Spagnoletti via GitGitGadget]

From: Javier Spagnoletti <phansys@gmail.com>

PHP allows some function modifiers that are not recognized in our current hunk header pattern

       final public function foo() { }
       abstract protected function bar() { }

Some context about the PHP semantics:

In PHP a class can be declared as abstract and final, while methods can be declared as final,
static and abstract, along with its visibility (public, protected and private).
Regarding classes, an abstract class can not be declared as final.
For the methods, from a strict syntax perspective, the order of the modifiers is irrelevant.

For more information, see:

* https://www.php.net/manual/en/language.oop5.abstract.php#example-213;
* https://www.php.net/manual/en/language.oop5.final.php#language.oop5.traits.static.ex1.

This commit adds "final" and "abstract" to the list of function modifiers.

Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
---
    userdiff: php: Add "final" and "abstract" to the list of function
    modifiers
    
    PHP allows some function modifiers that are not recognized in our
    current hunk header pattern
    
       final public function foo() { }
       abstract protected function bar() { }
    
    Some context about the PHP semantics:
    
    In PHP a class can be declared as abstract and final, while methods can
    be declared as final, static and abstract, along with its visibility
    (public, protected and private). Regarding classes, an abstract class
    can not be declared as final. For the methods, from a strict syntax
    perspective, the order of the modifiers is irrelevant.
    
    For more information, see:
    
     * https://www.php.net/manual/en/language.oop5.abstract.php#example-213;
     * https://www.php.net/manual/en/language.oop5.final.php#language.oop5.traits.static.ex1
       .
    
    This commit adds "final" and "abstract" to the list of function
    modifiers.
    
    Signed-off-by: Javier Spagnoletti phansys@gmail.com [phansys@gmail.com]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-864%2Fphansys%2Fuserdiff_php-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-864/phansys/userdiff_php-v3
Pull-Request: https://github.com/git/git/pull/864

Range-diff vs v2:

 1:  4cb59950ff ! 1:  43acd70aba Improve diff pattern for PHP files
     @@ Metadata
      Author: Javier Spagnoletti <phansys@gmail.com>
      
       ## Commit message ##
     -    Improve diff pattern for PHP files
     +    userdiff: php: Add "final" and "abstract" to the list of function modifiers
      
          PHP allows some function modifiers that are not recognized in our current hunk header pattern
      
                 final public function foo() { }
                 abstract protected function bar() { }
      
     -    Add "final" and "abstract" to the list of function modifiers.
     +    Some context about the PHP semantics:
     +
     +    In PHP a class can be declared as abstract and final, while methods can be declared as final,
     +    static and abstract, along with its visibility (public, protected and private).
     +    Regarding classes, an abstract class can not be declared as final.
     +    For the methods, from a strict syntax perspective, the order of the modifiers is irrelevant.
     +
     +    For more information, see:
     +
     +    * https://www.php.net/manual/en/language.oop5.abstract.php#example-213;
     +    * https://www.php.net/manual/en/language.oop5.final.php#language.oop5.traits.static.ex1.
     +
     +    This commit adds "final" and "abstract" to the list of function modifiers.
      
          Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
      


 t/t4018/php-abstract-method | 7 +++++++
 t/t4018/php-final-method    | 7 +++++++
 userdiff.c                  | 2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/php-abstract-method
 create mode 100644 t/t4018/php-final-method

diff --git a/t/t4018/php-abstract-method b/t/t4018/php-abstract-method
new file mode 100644
index 0000000000..ce215df75a
--- /dev/null
+++ b/t/t4018/php-abstract-method
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}
diff --git a/t/t4018/php-final-method b/t/t4018/php-final-method
new file mode 100644
index 0000000000..537fb8ad9a
--- /dev/null
+++ b/t/t4018/php-final-method
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}
diff --git a/userdiff.c b/userdiff.c
index 1df884ef0b..a99d84a7e3 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -143,7 +143,7 @@ PATTERNS("perl",
 	 "|=~|!~"
 	 "|<<|<>|<=>|>>"),
 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).*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"

base-commit: 47ae905ffb98cc4d4fd90083da6bc8dab55d9ecc
-- 
gitgitgadget

Re: [PATCH v2] Improve diff pattern for PHP files

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Javier Spagnoletti via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> Subject: Re: [PATCH v2] Improve diff pattern for PHP files
>
> You seem to have read J6t's excellent suggestion well but forgot to
> retitle.

Will queue with minimum copy-editing (reproduced below).

Thanks.

commit aff92827b54f4b7f9e339982a49bab4bdbd1fc55
Author: Javier Spagnoletti <phansys@gmail.com>
Date:   Wed Oct 7 03:38:18 2020 +0000

    userdiff: PHP: catch "abstract" and "final" functions
    
    PHP permits functions to be defined like
    
           final public function foo() { }
           abstract protected function bar() { }
    
    but our hunk header pattern does not recognize these decorations.
    Add "final" and "abstract" to the list of function modifiers.
    
    Helped-by: Johannes Sixt <j6t@kdbg.org>
    Signed-off-by: Javier Spagnoletti <phansys@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t4018/php-abstract-method b/t/t4018/php-abstract-method
new file mode 100644
index 0000000000..ce215df75a
--- /dev/null
+++ b/t/t4018/php-abstract-method
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}
diff --git a/t/t4018/php-final-method b/t/t4018/php-final-method
new file mode 100644
index 0000000000..537fb8ad9a
--- /dev/null
+++ b/t/t4018/php-final-method
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}
diff --git a/userdiff.c b/userdiff.c
index fde02f225b..4e2b55a5e0 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -147,7 +147,7 @@ PATTERNS("perl",
 	 "|=~|!~"
 	 "|<<|<>|<=>|>>"),
 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).*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"