threads / patch / 54354

patchImprove diff pattern for PHP files

Subject: [PATCH] Improve diff pattern for PHP files

## tl;dr

7 messages between Oct 6, 2020 and Oct 7, 2020. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Javier Spagnoletti via GitGitGadget· Oct 6, 2020, 10:37 UTC · lore
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(-)
Show changes to userdiff.c +1 −1
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
Johannes Sixt· Oct 6, 2020, 18:27 UTC · re: Javier Spagnoletti via GitGitGadget · lore

Re: [PATCH] Improve diff pattern for PHP files

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.

Show 28 quoted lines
> 
> 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
Junio C Hamano· Oct 6, 2020, 18:55 UTC · re: Johannes Sixt · lore

Re: [PATCH] Improve diff pattern for PHP files

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.
Javier Spagnoletti via GitGitGadget· Oct 7, 2020, 03:38 UTC · re: Javier Spagnoletti via GitGitGadget · lore

[PATCH v2] Improve diff pattern for PHP files

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
Show changes to 3 files +15 −1

t/t4018/php-abstract-method, t/t4018/php-final-method, userdiff.c

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
Junio C Hamano· Oct 7, 2020, 06:12 UTC · re: Javier Spagnoletti via GitGitGadget · lore

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

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

Show 59 quoted lines
> 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
Junio C Hamano· Oct 7, 2020, 18:47 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> "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>
Show changes to 3 files +15 −1

t/t4018/php-abstract-method, t/t4018/php-final-method, userdiff.c

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_]*"
Javier Spagnoletti via GitGitGadget· Oct 7, 2020, 18:07 UTC · re: Javier Spagnoletti via GitGitGadget · lore

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

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
Show changes to 3 files +15 −1

t/t4018/php-abstract-method, t/t4018/php-final-method, userdiff.c

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

← back to recent threads