All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: xing zhi jiang <a97410985new@gmail.com>
Cc: git@vger.kernel.org, j6t@kdbg.org, chooglen@google.com
Subject: Re: [GSoC][PATCH v3] Add a diff driver for JavaScript languages.
Date: Mon, 04 Apr 2022 09:12:41 +0200	[thread overview]
Message-ID: <220404.86lewljovj.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20220403132508.28196-1-a97410985new@gmail.com>


On Sun, Apr 03 2022, xing zhi jiang wrote:

Aside from what Johannes Sixt mentioned:

> +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 *]*[$_[:alpha:]][$_[:alnum:]]*[\t ]*\\(.*)\n"
> +	 /* match JavaScript variable declaration with a lambda expression at top level */
> +	 "^((const|let|var)[\t ]*[$_[:alpha:]][$_[:alnum:]]*[\t ]*=[\t ]*"
> +		"(\\(.*\\)|[$_[:alpha:]][$_[:alnum:]]*)[\t ]*=>[\t ]*\\{?)\n"
> +	 /* match object's property assignment by anonymous function and CommonJS exports for named function */
> +	 "^((module\\.)?[$_[:alpha:]][$_[:alnum:]]*\\.[$_[:alpha:]][$_[:alnum:]]*[\t ]*=[\t ]*(async[\t ]+)?(\\(.*\\)|[$_[:alpha:]][$_[:alnum:]]*)[\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" */
> +	 "^[\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 ]*([$_[:alpha:]][$_[:alnum:]]*[\t ]*:[\t ]*function.*)\n"
> +	 /* don't match chained method call */
> +	 "!^[\t ]*[$_[:alpha:]][$_[:alnum:]][\t ]*\\(.*\\)\\.\n"
> +	 /* match function in class and ES5 method shorthand */
> +	 "^[\t ]*((static[\t ]+)?((async|get|set)[\t ]+)?[$_[:alpha:]][$_[:alnum:]]*[\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 */
> +	 "|[$_[:alpha:]][$_[:alnum:]]*"),
>  PATTERNS("markdown",
>  	 "^ {0,3}#{1,6}[ \t].*",
>  	 /* -- */<

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:]]"

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"
	+	 /* 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?

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


  parent reply	other threads:[~2022-04-04  7:21 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 13:08 [GSoC][PATCH 0/1] userdiff: add buildin diff driver for JavaScript language xing zhi jiang
2022-03-04 13:08 ` [GSoC][PATCH 1/1] Add a diff driver for JavaScript languages xing zhi jiang
2022-03-05 10:16   ` Johannes Sixt
2022-03-07 15:10     ` xing-zhi jiang
2022-03-08  6:46       ` Johannes Sixt
2022-03-12 16:59         ` xing zhi jiang
2022-03-05 13:41 ` [GSoC][PATCH 0/1] userdiff: add buildin diff driver for JavaScript language Johannes Sixt
2022-03-12 16:48 ` [GSoC][PATCH v2] Add a diff driver for JavaScript languages xing zhi jiang
2022-03-13 21:54   ` Johannes Sixt
2022-04-03 13:17     ` xing zhi jiang
2022-03-14 17:20   ` Glen Choo
2022-03-15  7:40     ` Johannes Sixt
2022-03-15 18:51       ` Glen Choo
2022-03-15 19:22         ` Junio C Hamano
2022-03-15 21:34           ` Glen Choo
2022-04-03 13:24             ` xing zhi jiang
2022-04-03 13:20         ` xing zhi jiang
2022-04-03 13:21     ` xing zhi jiang
2022-04-03 13:25 ` [GSoC][PATCH v3] " xing zhi jiang
2022-04-03 14:40   ` Johannes Sixt
2022-04-04  7:12   ` Ævar Arnfjörð Bjarmason [this message]
2022-04-04 20:29     ` Johannes Sixt
2022-04-04 21:44       ` Junio C Hamano
2022-04-05  2:22       ` Ævar Arnfjörð Bjarmason
2022-04-04 17:32   ` Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=220404.86lewljovj.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=a97410985new@gmail.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.