Re: [GSoC][PATCH v3] Add a diff driver for JavaScript languages.

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Apr 04 2022, Johannes Sixt wrote:

> Am 04.04.22 um 09:12 schrieb Ævar Arnfjörð Bjarmason:
>> While we don't use helper macros for these currently there's no reason
>> we can't, I thin the above might be more readable with e.g.:
>> 
>> 	#define JS_AA "[$_[:alpha:]][$_[:alnum:]]"
>
> Please consider including "identifier" somehow in the macro name. And
> add the trailing '*', which...

Indeed, although for something like this a cute short name is probably
OK, and we can just #undef it right afer.

>> Which would make this:
>> 	
>> 	+PATTERNS("javascript",
>> 	+	 /* don't match the expression may contain parenthesis, because it is not a function declaration */
>> 	+	 "!^[ \t]*(if|do|while|for|with|switch|catch|import|return)\n"
>> 	+	 /* don't match statement */
>> 	+	 "!;\n"
>> 	+	 /* match normal function or named export for function in ECMA2015 */
>> 	+	 "^((export[\t ]+)?(async[\t ]+)?function[\t ]*[\t *]*" JS_AA "*[\t ]*\\(.*)\n"
>> 	+	 /* match JavaScript variable declaration with a lambda expression at top level */
>> 	+	 "^((const|let|var)[\t ]*" JS_AA "*[\t ]*=[\t ]*"
>> 	+		"(\\(.*\\)|" JS_AA "*)[\t ]*=>[\t ]*\\{?)\n"
>> 	+	 /* match object's property assignment by anonymous function and CommonJS exports for named function */
>> 	+	 "^((module\\.)?" JS_AA "*\\." JS_AA "*[\t ]*=[\t ]*(async[\t ]+)?(\\(.*\\)|" JS_AA "*)[\t ]*=>.*)\n"
>> 	+	 /* match assign function to LHS with explicit function keyword */
>> 	+	 "^(.*=[\t ]*function[\t ]*([$_[:alnum:]]+[\t ]*)?\\(.*)\n"
>> 	+	 /* popular unit testing framework test case pattern. Most of framework pattern is match by regex for "function in class" */
>> 
>> Wry try to stick to wrapping at 80 characters, so some of these comments
>> should really be wrapped (see CodingGuidelines for the multi-line
>> comment style we use).
>> 
>> 	+	 "^[\t ]*(QUnit.test\\(.*)\n"
>> 	+	 /* don't match the function in class or in object literal, which has more than one ident level */
>> 	+	 "!^(\t{2,}|[ ]{5,})\n"
>> 	+	 /* match normal function in object literal */
>> 	+	 "^[\t ]*(" JS_AA "*[\t ]*:[\t ]*function.*)\n"
>> 	+	 /* don't match chained method call */
>> 	+	 "!^[\t ]*" JS_AA "[\t ]*\\(.*\\)\\.\n"
>
> ... which makes me wonder why it is not present here. If that's an
> oversight: nice catch!

*Nod*, I just did a dumb search replace and didn't notice that myself,
 but it's clearly making things easier to read.

Asanother thing I noticed: shouldn't that '.' in QUnit.test be escaped?
Presumably we don't want QUnitXtest or whatever.

>> 	+	 /* match function in class and ES5 method shorthand */
>> 	+	 "^[\t ]*((static[\t ]+)?((async|get|set)[\t ]+)?" JS_AA "*[\t ]*\\(.*)",
>> 	+	 /* word regex */
>> 	+	 /* hexIntegerLiteral, octalIntegerLiteral, binaryIntegerLiteral, and its big version */
>> 	+	 "0[xXoObB][_0-9a-fA-F]+n?"
>> 	+	 /* DecimalLiteral and its big version*/
>> 	+	 "|[0-9][_0-9]*(\\.[0-9][_0-9]*|n)?([eE][+-]?[_0-9]+)?"
>> 	+	 "|\\.[0-9][_0-9]*([eE][+-]?[_0-9]+)?"
>> 	+	 /* punctuations */
>> 	+	 "|\\.{3}|<=|>=|==|!=|={3}|!==|\\*{2}|\\+{2}|--|<<|>>"
>> 	+	 "|>>>|&&|\\|{2}|\\?{2}|\\+=|-=|\\*=|%=|\\*{2}="
>> 	+	 "|<<=|>>=|>>>=|&=|\\|=|\\^=|&&=|\\|{2}=|\\?{2}=|=>"
>> 	+	 /* identifiers */
>> 	+	 "|" JS_AA "*"),
>> 	
>> Just a thought, I wonder how much line-noisy we could make this thing in
>> general if we defined some common patterns with such helpers.
>> 
>> Anyway, insted of :alnum:and :alpha: don't you really mean [a-zA-Z0-9]
>> and [a-zA-Z]. I.e. do you *really* want to have this different depending
>> on the user's locale?
>
> That's worth considering.

If it's intentional it makes sense to add some locale-stressing tests to
the tests, i.e. if we really mean to match non-ASCII identifiers etc.

>> 
>> I haven't tested, but see the LC_CTYPE in gettext.c, so I'm fairly sure
>> that'll happen...
>> 
>
> -- Hannes