Re: [GSoC][PATCH v2] Add a diff driver for JavaScript languages.
- From
xing zhi jiang <a97410985new@gmail.com>
- Date
- Apr 3, 2022, 13:21 UTC
- Message-ID
- <CAN_FRNKbQAFr5fL-giffKADHjBN2QUt5fUQriTpjOv-g5=9QHw@mail.gmail.com>
- In-Reply-To
- <kl6lwngwqwm2.fsf@chooglen-macbookpro.roam.corp.google.com>
Sorry for the late reply. Because I was busy last two weeks. And grateful for your review.
On Tue, 15 Mar 2022 at 01:20, Glen Choo <chooglen@google.com> wrote:
Show 38 quoted lines
>
> > diff --git a/t/t4018/javascript-function b/t/t4018/javascript-function
> > new file mode 100644
> > index 0000000000..0cc0bf54e7
> > --- /dev/null
> > +++ b/t/t4018/javascript-function
> > @@ -0,0 +1,4 @@
> > +function RIGHT(a, b) {
> > +
> > + return a + b; // ChangeMe
> > +}
> > diff --git a/t/t4018/javascript-function-2 b/t/t4018/javascript-function-2
> > new file mode 100644
> > index 0000000000..06cfb779f0
> > --- /dev/null
> > +++ b/t/t4018/javascript-function-2
> > @@ -0,0 +1,10 @@
> > +function test(a, b) {
> > + return {
> > + RIGHT: function () {
> > + currentUpdateRemovedChunks.forEach(function (chunkId) {
> > + delete $installedChunks$[chunkId];
> > + });
> > + currentUpdateRemovedChunks = ChangeMe;
> > + }
> > + }
> > +}
>
> There is also the ES2015 'method shorthand' syntax [3], e.g. `bar` in:
>
> const foo = {
> bar() {
> console.log('hi');
> }
> }
>
> [3] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions
>The ES2015 method shorthand is already matched by the regex for function in class "^[\t ]*((static[\t ]+)?((async|get|set)[\t ]+)?[$_[:alpha:]][$_[:alnum:]]*[\t ]*\\(.*)" So I update the comment for this regex in the v3 patch. The comment is "/* match function in class and ES5 method shorthand */"
Show 30 quoted lines
> > diff --git a/t/t4018/javascript-function-belong-to-IIFE b/t/t4018/javascript-function-belong-to-IIFE
> > new file mode 100644
> > index 0000000000..6e5fe858c0
> > --- /dev/null
> > +++ b/t/t4018/javascript-function-belong-to-IIFE
> > @@ -0,0 +1,6 @@
> > +(function () {
> > + this.$RIGHT = function (needle, modifier) {
> > + let a = 5;
> > + return ChangeMe;
> > + };
> > +}).call(aaaa.prototype);
>
> Does the IIFE matter in this case? This line:
>
> this.$RIGHT = function (needle, modifier) {
>
> looks extremely similar to the previous test of `foo = function bar()`.
>
> Or perhaps this is meant to demonstrate the edge case of "matching in a
> complicated construct"? If so, perhaps we should test other edge cases
> like:
>
> function WRONG() {
> let RIGHT = function (needle, modifier) {
> let a = 5;
> return ChangeMe;
> };
> }
>Currently, I realize this test case is redundant. Because I trust the function keyword. So the regex "^(.*=[\t ]*function[\t ]*([$_[:alnum:]]+[\t ]*)?\\(.*)\n" is loose for LHS. I will remove this case in the v3 patch.