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

From: Johannes Sixt <j6t@kdbg.org>
Date: Mar 8, 2022, 06:46 UTC
Message-ID: <504f9f0d-a1ab-5ce7-d744-9398b0e7e68a@kdbg.org>
In-Reply-To: <CAN_FRNK-FD+4_Nf_VdmU1N+TBapQ7ZnWo5Bn8M9YDWWhMfBypg@mail.gmail.com>

Am 07.03.22 um 16:10 schrieb xing-zhi jiang:

Show 27 quoted lines

> On Sat, 5 Mar 2022 at 18:16, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 04.03.22 um 14:08 schrieb xing zhi jiang:
>>> diff --git a/t/t4018/javascript-function-in-class-2 b/t/t4018/javascript-function-in-class-2
>>> new file mode 100644
>>> index 0000000000..402c4aecc3
>>> --- /dev/null
>>> +++ b/t/t4018/javascript-function-in-class-2
>>> @@ -0,0 +1,11 @@
>>> +class Test {
>>> +  RIGHT(
>>> +      aaaaaaaaaa,
>>> +      bbbbbbbbbb,
>>> +      cccccccccc,
>>> +      dddddddddd
>>> +  ) {
>>> +    let a = 4;
>>> +    let b = ChangeMe;
>>> +  }
>>> +}
>>> \ No newline at end of file
>>
>> In the above two, we see class member functions. Is there a test case
>> where the function parameter is on the same line as the function name or
>> is that one of the difficult cases?
> I can't get what you mean, but the second Test case wants to make sure we
> can match the function with a long parameter list, and its parameters are
> formatted to multiline.

I meant a multi-line function call like so, where the first argument is on the same line with the function name:

  RIGHT(aaaaaaaaaa,
      bbbbbbbbbb,
      cccccccccc,
      dddddddddd
  ) {
  ...

Show 26 quoted lines

>>> diff --git a/userdiff.c b/userdiff.c
>>> index 8578cb0d12..a6a341e3c1 100644
>>> --- a/userdiff.c
>>> +++ b/userdiff.c
>>> @@ -168,6 +168,44 @@ PATTERNS("java",
>>>        "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
>>>        "|[-+*/<>%&^|=!]="
>>>        "|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
>>> +
>>> +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"
>>
>> These will not match
>>
>>         }while (expr)
>>
>> note the absent blank before the keyword, but that is an acceptable
>> trade-off to keep things simple. Good.
>>
> I thought not to match with `}while (expr)` is OK because this rule is
> mainly to prevent matching too many
>  wrong things about the regex for 「the function in the class」.
> And before keywords maybe need blanks, because some javascript
> formatting style is using space
> instead of tab.

I don't recall why I emphasize the absent blank between '}' and "while". I must have misread something. At any rate, the regex would not match, either, if there is whitespace between the two, so, it is totally OK.

Show 16 quoted lines

>> Can you not have
>>
>>         var f = foo=>{
>>
>> because I see that whitespace is required between the identifier and "=>"?
> It is my mistake. I should not match tab or space between identifier and =>.
> Thanks for pointing it out. I will fix it.
> 
>>> +      /* match exports for anonymous fucntion */
>>> +      "^[\t ]*(exports\\.[$_[:alpha:]][$_[:alnum:]]*[\t ]*=[\t ]*(\\(.*\\)|[$_[:alpha:]][$_[:alnum:]]*)[\t ]*=>.*)\n"
>>
>> Here, whitespace is not required. Is the above an oversight?
>>
>> BTW, can keyword "exports" be used for something other than functions?
>>
> 「Whitespace is not required」 is only referred to 「^[\t ]*(exports」?

No, I refer to the whitespace around "=>" that is not required here, but was required in the previous expression.

Show 6 quoted lines

> I
> would remove the [\t ]* before exports
> keyword because exports must be top-level.
> The exports keyword can be used for exporting many things, such as
> string. ex: exports.SimpleMessage = 'Hello world';
> , So I define a more precise regex for only matching anonymous functions.

Understood. "exports" is a keyword only in certain contexts. Since it can be used as (or like?) a variable name, it cannot be used as match that always identifies a function header.

-- Hannes