Improve diff pattern for PHP files

PATCH Superseded Javier Spagnoletti via GitGitGadget 3 participants 7 messages Oct 6, 2020 – Oct 7, 2020

lore

Share & tools

lore

Javier Spagnoletti via GitGitGadget

author

2020-10-06 10:37 UTC #0 lore raw

PATCH 1/1 Improve diff pattern for PHP files

From: Javier Spagnoletti <[email protected]>

Improve the output diff readability for php files by taking into account some missing function modifiers.

Signed-off-by Javier Spagnoletti <[email protected]>

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

userdiff.c +1 -1

index 1df884ef0b..a99d84a7e3 100644
@@ -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

Signature · 1 line

gitgitgadget

Johannes Sixt

2020-10-06 18:27 UTC #1 lore raw

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:

Quoted 3 lines

From: Javier Spagnoletti <[email protected]>
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.

Quoted 20 lines

Signed-off-by Javier Spagnoletti <[email protected]>
---
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(-)
userdiff.c +0 -0
index 1df884ef0b..a99d84a7e3 100644

userdiff.c PATTERNS("perl", · line 149

Quoted 8 lines

userdiff.c +1 -1

@@ -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

2020-10-06 18:55 UTC #2 lore raw

Quoted 16 lines · 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:
Quoted 3 lines
From: Javier Spagnoletti <[email protected]>
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

Quoted 8 lines

       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.

Quoted 4 lines

userdiff.c PATTERNS("perl", · line 147

Quoted 4 lines

PATTERNS("php",

userdiff.c +1 -1

-	 "^[\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) ;-)

Quoted 5 lines

userdiff.c PATTERNS("perl", · line 149
Quoted 2 lines
userdiff.c +0 -0
 	 /* -- */
 	 "[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

author

2020-10-07 03:38 UTC #3 lore raw

PATCH v2 1/1 Improve diff pattern for PHP files

In reply to Javier Spagnoletti via GitGitGadget

From: Javier Spagnoletti <[email protected]>

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 <[email protected]>

---
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 [email protected] [[email protected]]

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 <[email protected]>
      
     + ## 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

t/t4018/php-abstract-method new file +7 -0

new file mode 100644
index 0000000000..ce215df75a
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}

t/t4018/php-final-method new file +7 -0

new file mode 100644
index 0000000000..537fb8ad9a
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}

userdiff.c +1 -1

index 1df884ef0b..a99d84a7e3 100644
@@ -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

Signature · 1 line

gitgitgadget

Junio C Hamano

2020-10-07 06:12 UTC #4 lore raw

"Javier Spagnoletti via GitGitGadget" <[email protected]>

Quoted 1 line · 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.

Quoted 59 lines

From: Javier Spagnoletti <[email protected]>

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 <[email protected]>

...

 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

t/t4018/php-abstract-method new file +7 -0

new file mode 100644
index 0000000000..ce215df75a
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}

t/t4018/php-final-method new file +7 -0

new file mode 100644
index 0000000000..537fb8ad9a
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}

userdiff.c +1 -1

index 1df884ef0b..a99d84a7e3 100644
@@ -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

2020-10-07 18:47 UTC #5 lore raw

Quoted 7 lines · Junio C Hamano

"Javier Spagnoletti via GitGitGadget" <[email protected]>
Quoted 1 line · 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 <[email protected]>
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 <[email protected]>

Signed-off-by Javier Spagnoletti <[email protected]>

Signed-off-by Junio C Hamano <[email protected]>

t/t4018/php-abstract-method new file +7 -0

new file mode 100644
index 0000000000..ce215df75a
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}

t/t4018/php-final-method new file +7 -0

new file mode 100644
index 0000000000..537fb8ad9a
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}

userdiff.c +1 -1

index fde02f225b..4e2b55a5e0 100644
@@ -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

author

2020-10-07 18:07 UTC #6 lore raw

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

In reply to Javier Spagnoletti via GitGitGadget

From: Javier Spagnoletti <[email protected]>

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 <[email protected]>

---
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 [email protected] [[email protected]]

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 <[email protected]>

## 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 <[email protected]>

 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

t/t4018/php-abstract-method new file +7 -0

new file mode 100644
index 0000000000..ce215df75a
@@ -0,0 +1,7 @@
+abstract class Klass
+{
+    abstract public function RIGHT(): ?string
+    {
+        return 'ChangeMe';
+    }
+}

t/t4018/php-final-method new file +7 -0

new file mode 100644
index 0000000000..537fb8ad9a
@@ -0,0 +1,7 @@
+class Klass
+{
+    final public function RIGHT(): string
+    {
+        return 'ChangeMe';
+    }
+}

userdiff.c +1 -1

index 1df884ef0b..a99d84a7e3 100644
@@ -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

Signature · 1 line

gitgitgadget