| Message ID | pull.864.git.git.1601980656554.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve diff pattern for PHP files | expand |
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
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.
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_]*"