git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Apr 5, 2022, 02:22 UTC
Message-ID
<220405.861qycmfdv.gmgdl@evledraar.gmail.com>
In-Reply-To
<660d068f-1c8c-7057-0a92-5100791daf80@kdbg.org>
On Mon, Apr 04 2022, Johannes Sixt wrote:
Show 8 quoted lines
> 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.

Show 32 quoted lines
>> 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.

Show 23 quoted lines
>> 	+	 /* 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.

Show 6 quoted lines
>> 
>> I haven't tested, but see the LC_CTYPE in gettext.c, so I'm fairly sure
>> that'll happen...
>> 
>
> -- Hannes
Previous: Junio C HamanoNext: Glen Choo
Message 24 of 25 in “userdiff: add buildin diff driver for JavaScript language”
  1. xing zhi jiangMar 4, 2022
  2. [GSoC][PATCH 1/1] Add a diff driver for JavaScript languages.xing zhi jiang, Mar 4, 2022
  3. Johannes SixtMar 5, 2022
  4. xing-zhi jiangMar 7, 2022
  5. Johannes SixtMar 8, 2022
  6. xing zhi jiangMar 12, 2022
  7. Johannes SixtMar 5, 2022
  8. [GSoC][PATCH v2] Add a diff driver for JavaScript languages.xing zhi jiang, Mar 12, 2022
  9. Johannes SixtMar 13, 2022
  10. xing zhi jiangApr 3, 2022
  11. Glen ChooMar 14, 2022
  12. Johannes SixtMar 15, 2022
  13. Glen ChooMar 15, 2022
  14. Junio C HamanoMar 15, 2022
  15. Glen ChooMar 15, 2022
  16. xing zhi jiangApr 3, 2022
  17. xing zhi jiangApr 3, 2022
  18. xing zhi jiangApr 3, 2022
  19. [GSoC][PATCH v3] Add a diff driver for JavaScript languages.xing zhi jiang, Apr 3, 2022
  20. Johannes SixtApr 3, 2022
  21. Ævar Arnfjörð BjarmasonApr 4, 2022
  22. Johannes SixtApr 4, 2022
  23. Junio C HamanoApr 4, 2022
  24. Ævar Arnfjörð BjarmasonApr 5, 2022
  25. Glen ChooApr 4, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.