> On 30-Mar-2021, at 12:34, Atharva Raykar <raykar.ath@xxxxxxxxx> wrote: > > > >> On 29-Mar-2021, at 15:48, Phillip Wood <phillip.wood123@xxxxxxxxx> wrote: >> >> Hi Atharva >> >> On 28/03/2021 13:23, Atharva Raykar wrote: >>> On 28-Mar-2021, at 05:16, Johannes Sixt <j6t@xxxxxxxx> wrote: >>> [...] >>>>> 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)' > > The reason I went for this over the top level forms, is because > I felt it was useful to see the nearest definition for internal > functions that often have a lot of the actual business logic of > the program (at least a lot of SICP seems to follow this pattern). > The disadvantage is as you said, it might also catch trivial inner > functions and the developer might lose context. Never mind this message, I had misunderstood the problem you were trying to demonstrate. I wholeheartedly agree with what you are trying to say, and the indentation heuristic discussed does look interesting. I shall have a glance at the RFC you linked in the other reply. > The disadvantage is as you said, it might also catch trivial inner > functions and the developer might lose context. Feel free to disregard me misquoting you here. You did not say that (: > Another problem is it may match more trivial bindings, like: > > (define (some-func things) > ... > (define items '(eggs > ham > peanut-butter)) > ...) > > What I have noticed *anecdotally* is that this is not common enough > to be too much of a problem, and local define bindings seem to be more > favoured in Racket than other Schemes, that use 'let' more often. > >> 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. > > The other issue with only matching top level defines is that a > lot of scheme programs are library definitions, something like > > (library > (foo bar) > (export ...) > (define ...) > (define ...) > ;; and a bunch of other definitions... > ) > > Only matching top level defines will completely ignore matching all > the definitions in these files. That said, I still stand by the fact that only catching top level defines will lead to a lot of definitions being ignored. Maybe the occasional mismatch may be worth the gain in the number of function contexts being detected?
