| Message ID | 20240301074048.188835-1-sergiusnyah@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GSOC] userdiff: Add JavaScript function patterns | expand |
The subject should be "userdiff: add JavaScript function patterns" so with "add" instead of "Add". See the SubmittingPatches document which contains: "The title sentence after the "area:" prefix omits the full stop at the end, and its first word is not capitalized (the omission of capitalization applies only to the word after the "area:" prefix of the title) unless there is a reason to capitalize it other than because it is the first word in the sentence." On Fri, Mar 1, 2024 at 8:40 AM Sergius Nyah <sergiusnyah@gmail.com> wrote: > > This commit adds a patterns used to match JavaScript functions. It should be either "add patterns" or "add a pattern". Also instead of "This commit" please use the imperative mood or "Let's". See the SubmittingPatches document which contains: "Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behavior." > It now correctly identifies function declarations, function expressions, > and functions defined inside blocks. Add test for corresponding change in userdiff. > > Signed-off-by: Sergius Nyah <sergiusnyah@gmail.com> > --- > t/t4018-diff-funcname.sh | 22 ++++++++++++++++++++++ > userdiff.c | 12 ++++++++++++ > 2 files changed, 34 insertions(+) In t4034-diff-words.sh there is: test_language_driver ada test_language_driver bibtex test_language_driver cpp test_language_driver csharp test_language_driver css test_language_driver dts test_language_driver fortran test_language_driver html test_language_driver java test_language_driver kotlin test_language_driver matlab test_language_driver objc test_language_driver pascal test_language_driver perl test_language_driver php test_language_driver python test_language_driver ruby test_language_driver scheme test_language_driver tex So I would have thought that you would just add a `test_language_driver javascript` line in this file and associated material in the t/t4034/ directory. > diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh > index e026fac1f4..d35cce18a0 100755 > --- a/t/t4018-diff-funcname.sh > +++ b/t/t4018-diff-funcname.sh > @@ -120,3 +120,25 @@ do > done > > test_done > + > +test_expect_success 'identify builtin patterns in JavaScript' ' > + # setup > + echo "function myFunction() { return true; }" > test.js && > + echo "var myVar = function() { return false; }" >> test.js && > + git add test.js && > + git commit -m "add test.js" && > + > + # modify the file > + echo "function myFunction() { return false; }" > test.js && > + echo "var myVar = function() { return true; }" >> test.js && > + > + # command under test > + git diff >output && > + > + # check results > + test_i18ngrep "function myFunction() { return true; }" output && > + test_i18ngrep "function myFunction() { return false; }" output && > + test_i18ngrep "var myVar = function() { return false; }" output && > + test_i18ngrep "var myVar = function() { return true; }" output I think we try to use just test_grep instead of test_i18ngrep these days. > +' > +test_done > \ No newline at end of file Please add a new line at the end of this file if you still change it. Thanks!
On Sat, Mar 2, 2024 at 11:28 AM Christian Couder <christian.couder@gmail.com> wrote: > > The subject should be "userdiff: add JavaScript function patterns" so > with "add" instead of "Add". See the SubmittingPatches document which > contains: > > "The title sentence after the "area:" prefix omits the full stop at the > end, and its first word is not capitalized (the omission > of capitalization applies only to the word after the "area:" > prefix of the title) unless there is a reason to > capitalize it other than because it is the first word in the sentence." Also about the subject, please use v2, v3, v4 when you resend a patch or a patch series that you have already sent. `git format-patch` has a "-v <n>" or "--reroll-count=<n>" option for that purpose.
Christian Couder <christian.couder@gmail.com> writes: >> + # check results >> + test_i18ngrep "function myFunction() { return true; }" output && >> + test_i18ngrep "function myFunction() { return false; }" output && >> + test_i18ngrep "var myVar = function() { return false; }" output && >> + test_i18ngrep "var myVar = function() { return true; }" output > > I think we try to use just test_grep instead of test_i18ngrep these days. Thanks for reminding. I am tempted to suggest doing this. ------ >8 ----------- >8 ----------- >8 ----------- >8 ------ Subject: test_i18ngrep: hard deprecate and forbid its use Since v2.44.0-rc0~109 (Merge branch 'sp/test-i18ngrep', 2023-12-27) none of the tests we have, either in 'master' or in flight and collected in 'seen', use test_i18ngrep. Perhaps it is good time to update test_i18ngrep to BUG to avoid people adding new calls to it. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/test-lib-functions.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git c/t/test-lib-functions.sh w/t/test-lib-functions.sh index b5eaf7fdc1..6eaf116346 100644 --- c/t/test-lib-functions.sh +++ w/t/test-lib-functions.sh @@ -1263,9 +1263,8 @@ test_cmp_bin () { cmp "$@" } -# Deprecated - do not use this in new code test_i18ngrep () { - test_grep "$@" + BUG "do not use test_i18ngrep---use test_grep instead" } test_grep () {
On Fri, Mar 01, 2024 at 08:40:48AM +0100, Sergius Nyah wrote: > This commit adds a patterns used to match JavaScript functions. > It now correctly identifies function declarations, function expressions, > and functions defined inside blocks. Add test for corresponding change in userdiff. > > Signed-off-by: Sergius Nyah <sergiusnyah@gmail.com> > --- > t/t4018-diff-funcname.sh | 22 ++++++++++++++++++++++ > userdiff.c | 12 ++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh > index e026fac1f4..d35cce18a0 100755 > --- a/t/t4018-diff-funcname.sh > +++ b/t/t4018-diff-funcname.sh > @@ -120,3 +120,25 @@ do > done > > test_done > + > +test_expect_success 'identify builtin patterns in JavaScript' ' > + # setup > + echo "function myFunction() { return true; }" > test.js && > + echo "var myVar = function() { return false; }" >> test.js && > + git add test.js && > + git commit -m "add test.js" && > + > + # modify the file > + echo "function myFunction() { return false; }" > test.js && > + echo "var myVar = function() { return true; }" >> test.js && > + > + # command under test > + git diff >output && > + > + # check results > + test_i18ngrep "function myFunction() { return true; }" output && > + test_i18ngrep "function myFunction() { return false; }" output && > + test_i18ngrep "var myVar = function() { return false; }" output && > + test_i18ngrep "var myVar = function() { return true; }" output > +' > +test_done > \ No newline at end of file This `test_done` only needs to be added because you add the new test before the preceding `test_done`. Instead, you should move up this test so that it comes before it. > diff --git a/userdiff.c b/userdiff.c > index 2b1dab2649..bbe2bcb9a3 100644 > --- a/userdiff.c > +++ b/userdiff.c > +PATTERNS("javascript", > + /* Looks for lines that start with optional whitespace, followed Multi-line comments should start with their delimiters on separate lines. So the "/*" should be on its own line. Also, the code should be indented with tabs and not spaces. It might help to read through Documentation/CodingGuidelines to get more familiar with Git's coding style. Patrick > + * by 'function'* and any characters (for function declarations), > + * or valid JavaScript identifiers, equals sign '=', 'function' keyword > + * and any characters (for function expressions). > + * Also considers functions defined inside blocks with '{...}'. > + */ > + "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n", > + /* This pattern matches JavaScript identifiers */ > + "[a-zA-Z_$][0-9a-zA-Z_$]*" > + "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+" > + "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"), > -- > 2.43.2 >
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index e026fac1f4..d35cce18a0 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -120,3 +120,25 @@ do done test_done + +test_expect_success 'identify builtin patterns in JavaScript' ' + # setup + echo "function myFunction() { return true; }" > test.js && + echo "var myVar = function() { return false; }" >> test.js && + git add test.js && + git commit -m "add test.js" && + + # modify the file + echo "function myFunction() { return false; }" > test.js && + echo "var myVar = function() { return true; }" >> test.js && + + # command under test + git diff >output && + + # check results + test_i18ngrep "function myFunction() { return true; }" output && + test_i18ngrep "function myFunction() { return false; }" output && + test_i18ngrep "var myVar = function() { return false; }" output && + test_i18ngrep "var myVar = function() { return true; }" output +' +test_done \ No newline at end of file diff --git a/userdiff.c b/userdiff.c index 2b1dab2649..bbe2bcb9a3 100644 --- a/userdiff.c
This commit adds a patterns used to match JavaScript functions. It now correctly identifies function declarations, function expressions, and functions defined inside blocks. Add test for corresponding change in userdiff. Signed-off-by: Sergius Nyah <sergiusnyah@gmail.com> --- t/t4018-diff-funcname.sh | 22 ++++++++++++++++++++++ userdiff.c | 12 ++++++++++++ 2 files changed, 34 insertions(+) +++ b/userdiff.c +PATTERNS("javascript", + /* Looks for lines that start with optional whitespace, followed + * by 'function'* and any characters (for function declarations), + * or valid JavaScript identifiers, equals sign '=', 'function' keyword + * and any characters (for function expressions). + * Also considers functions defined inside blocks with '{...}'. + */ + "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n", + /* This pattern matches JavaScript identifiers */ + "[a-zA-Z_$][0-9a-zA-Z_$]*" + "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+" + "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"), -- 2.43.2