Subject: Re: [GSOC][PATCH v2]userdiff: improve code quality and add JavaScript language driver.

From: Junio C Hamano <[email-masked]>
Date: Tue, 19 Mar 2024 14:02:10 -0700

Sergius Nyah <[email-masked]> writes:

> This commit introduces several improvements to the codebase:
>
> - replace the use of `test_i18ngrep` with `test_grep` in
> `t/t4018-diff-funcname.sh` for better pattern matching and improved
> performance.
> - add a JavaScript language driver to enhance the functionality of the
> application.
> - clean up the code by removing trailing whitespaces and fix multiline
> comments in `t/t4034-diff-words.sh` and `userdiff.c`.

Doing too many unrelated things that may appear semi-related only
because they are about the same files?  Don't.

>
> Signed-off-by: Sergius Nyah <[email-masked]>
> ---
>  t/t4018-diff-funcname.sh | 14 +++++++-------
>  t/t4034-diff-words.sh    |  1 +
>  userdiff.c               | 23 ++++++++++++-----------
>  3 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index d35cce18a0..e6d2f1c215 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -119,8 +119,6 @@ do
>  	"
>  done
>
> -test_done
> -
>  test_expect_success 'identify builtin patterns in JavaScript' '
>  	# setup
>  	echo "function myFunction() { return true; }" > test.js &&
> @@ -136,9 +134,11 @@ test_expect_success 'identify builtin patterns in JavaScript' '
>  	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_grep "function myFunction() { return true; }" output &&
> +	test_grep "function myFunction() { return false; }" output &&
> +	test_grep "var myVar = function() { return false; }" output &&
> +	test_grep "var myVar = function() { return true; }" output
>  '
> -test_done
> \ No newline at end of file
> +
> +test_done
> +

To which version of Git source code are the above hunks meant to
apply?  Hopefully we do not have a test that says "test_done"
followed by another "test_expect_success" in our tree.

> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
> index 74586f3813..cf2374af2c 100755
> --- a/t/t4034-diff-words.sh
> +++ b/t/t4034-diff-words.sh
> @@ -328,6 +328,7 @@ test_language_driver dts
>  test_language_driver fortran
>  test_language_driver html
>  test_language_driver java
> +test_language_driver javascript
>  test_language_driver kotlin
>  test_language_driver matlab
>  test_language_driver objc

It is unclear what "clean-up" was done to this file, as claimed in
the proposed log message.  Puzzled...

> diff --git a/userdiff.c b/userdiff.c
> index bbe2bcb9a3..15342c8662 100644
> --- a/userdiff.c
> +++ 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]+"
> -      "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"),
> +	/*
> +	 * Look 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 consider functions defined inside blocks with '{...}'.
> +	 */
> +	 "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n",
> +	 /* Match JavaScript identifiers with this pattern */
> +	 "[a-zA-Z_$][0-9a-zA-Z_$]*"
> +	 "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+"
> +	 "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"),

I do not see trailing whitespaces getting fixed, as claimed in the
proposed log message.  Puzzled, again...