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