Re: [GSOC][PATCH] userdiff: add support for Scheme

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mar 29, 2021, 10:18 UTC
Message-ID: <09678471-b2a2-8504-2293-e2b34a3a96e8@gmail.com>
In-Reply-To: <5BA00FC6-9810-49AB-8DE2-D4F4010E2F82@gmail.com>

Hi Atharva

On 28/03/2021 13:23, Atharva Raykar wrote:

> On 28-Mar-2021, at 05:16, Johannes Sixt <j6t@kdbg.org> wrote:

 > [...]

Show 15 quoted lines

>>> diff --git a/t/t4018/scheme-local-define b/t/t4018/scheme-local-define
>>> new file mode 100644
>>> index 0000000000..90e75dcce8
>>> --- /dev/null
>>> +++ b/t/t4018/scheme-local-define
>>> @@ -0,0 +1,4 @@
>>> +(define (higher-order)
>>> +  (define local-function RIGHT
>>
>> ... this one, which is also indented and *is* marked as RIGHT.
> 
> In this test case, I was explicitly testing for an indented '(define'
> whereas in the former, I was testing for the top-level '(define-syntax',
> which happened to have an internal define (which will inevitably show up
> in a lot of scheme code).

It would be nice to include indented define forms but including them means that any change to the body of a function is attributed to the last internal definition rather than the actual function. For example

(define (f arg)
   (define (g x)
     (+ 1 x))

   (some-func ...)
   ;;any change here will have '(define (g x)' in the hunk header, not 
'(define (f arg)'

I don't think this can be avoided as we rely on regexs rather than parsing the source so it is probably best to only match toplevel defines.

Best Wishes

Phillip

Show 25 quoted lines

> 
>>> +    (lambda (x)
>>> +     (car "this is" "ChangeMe"))))
>>> \ No newline at end of file
>>
>>> diff --git a/userdiff.c b/userdiff.c
>>> index 3f81a2261c..c51a8c98ba 100644
>>> --- a/userdiff.c
>>> +++ b/userdiff.c
>>> @@ -191,6 +191,14 @@ PATTERNS("rust",
>>> 	 "[a-zA-Z_][a-zA-Z0-9_]*"
>>> 	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"
>>> 	 "|[-+*\\/<>%&^|=!:]=|<<=?|>>=?|&&|\\|\\||->|=>|\\.{2}=|\\.{3}|::"),
>>> +PATTERNS("scheme",
>>> +         "^[\t ]*(\\(define-?.*)$",
>>
>> This "optional hyphen followed by anything" in the regex is strange.
>> Wouldn't that also capture a line that looks like, e.g.,
>>
>>     (defined-foo bar)
>>
>> Perhaps we want "define[- \t].*" in the regex?
> 
> Yes, this is what I intended to do, thanks for correcting it.
>