[plt-dev] proposed: folding in minor changes to stepper for 4.2 release

From: John Clements (clements at brinckerhoff.org)
Date: Thu May 21 02:02:48 EDT 2009

I'd like to fold in a few changes from the trunk onto the release  
branch. I've pasted the diffs below, along with comments on why each  
change is there. First, though, let me explain the change.

The stepper has a new feature, in that you can use a choice box to  
jump to the expression containing the selection.  More specifically,  
you can jump to the first step whose innermost before or after  
expression contains the beginning of the cursor selection in the  
definitions window. This allows you to jump to the evaluation of the  
code you're interested in, and should make the stepper more usable on  
larger programs.

In point of fact, the code that's currently in the release candidate  
has one additional restriction.  Rather than jumping to the first step  
whose innermost before or after expression *contains* the beginning of  
the cursor selection, it jumps to the first step whose innermost  
before or after expression *begins with* the beginning of the cursor  
selection.

So, for instance, in this code:

(check-expect (f 9) 16)

If the cursor were between the ( and the 'f', the new feature might  
not find a matching step at all in the release candidate.  With the  
changes I've committed to the trunk, it will halt  just before the  
call to 'f', because the application (f 9) contains the cursor position.

To summarize: this change makes the new feature less persnickety and  
more useful.

It's still not what I'd like, because putting the cursor before the  
test case doesn't halt at all; this is because currently, there is no  
explicit step associated with the check-expect itself. Doing this  
"right" probably involves interacting with the definitions window to  
determine s-expression shapes within the definitions window, and is  
not something that I would try to add at this stage of the release.

I think that the diff below will demonstrate that this is the kind of  
change that should be feasible; even if you don't read it carefully,  
its existence should suggest that I have.

Thanks for your time,

John



> pcp063697pcs:~/plt/collects/stepper clements$ svn diff -r 14506 .
> Index: view-controller.ss
> ===================================================================
> --- view-controller.ss	(revision 14506)
> +++ view-controller.ss	(working copy)
> @@ -169,8 +169,15 @@
>
>    ;; is this step on the selected expression?
>    (define (selected-exp-step? history-entry)
> -    (member selection-posn (step-posns history-entry)))
> +    (ormap (posn-in-span selection-posn) (step-posns history-entry)))
>
> +  (define ((posn-in-span selection-posn) source-posn-info)
> +    (match source-posn-info
> +      [#f #f]
> +      [(struct model:posn-info (posn span))
> +       (and posn
> +            (<= posn selection-posn)
> +            (< selection-posn (+ posn span)))]))

This is the main change.  Rather than checking the source position  
against a list, it checks to see whether the source position is in one  
of a sequence of ranges.

>
>    ;; build gui object:
>
> @@ -358,7 +365,10 @@
>               (list x:finished-text 'finished-stepping (list))])])
>        (hand-off-and-block step-text step-kind posns)))
>
> -  ;; need to capture the custodian as the thread starts up:
> +  ;; program-expander-prime : wrap the program-expander for a  
> couple of reasons:
> +  ;; 1) we need to capture the custodian as the thread starts up:
> +  ;; ok, it was just one.
> +  ;;
>    (define (program-expander-prime init iter)
>      (program-expander
>       (lambda args

This is just a change to a comment.

> Index: private/model.ss
> ===================================================================
> --- private/model.ss	(revision 14506)
> +++ private/model.ss	(working copy)
> @@ -11,7 +11,7 @@
>  ; held = NO-HELD-STEP :
>  ;  first(x) : held := HELD(x)
>  ;  skipped-first : held := SKIPPED-STEP
> -;  second(x) : trigger(NO-HELD-STEP, x), held := NO-HELD-STEP
> +;  second(x) : trigger(NO-HELD-STEP, x), held := NO-HELD-STEP.

Ditto.

>  ;      this happens when evaluating unannotated code
>  ;  skipped-second : held := NO-HELD-STEP
>  ;      I believe this can also arise in unannotated code
> @@ -72,6 +72,12 @@
>        . -> .
>        void?)])
>
> +
> +(define-struct posn-info (posn span))
> +
> +(provide (struct-out posn-info))
> +
> +

I'm now passing ranges instead of numbers.  This is the new structure  
that holds them.

>  ; go starts a stepper instance
>  ; see provide stmt for contract
>  (define (go program-expander receive-result render-settings
> @@ -94,7 +100,7 @@
>    ;; the "held" variables are used to store the "before" step.
>    (define held-exp-list the-no-sexp)
>
> -  (define-struct held (exps was-app? source-pos))
> +  (define-struct held (exps was-app? source-info))

pos turns into info

>
>    (define held-finished-list null)
>
> @@ -215,7 +221,9 @@
>                                   mark-list returned-value-list  
> render-settings)
>                                  #f))
>                            (r:step-was-app? mark-list)
> -                          (syntax-position (mark-source (car mark- 
> list))))))]
> +                          (make-posn-info
> +                           (syntax-position (mark-source (car mark- 
> list)))
> +                           (syntax-span (mark-source (car mark- 
> list)))))))]

passing a posn-info rather than just a number

>
>                  [(result-exp-break result-value-break)
>                   (let ([reconstruct
> @@ -248,7 +256,7 @@
>                        (append (reconstruct-all-completed)  
> (reconstruct))
>                        'normal
>                        #f #f))]
> -                   [(struct held (held-exps held-step-was-app? held- 
> source-pos))
> +                   [(struct held (held-exps held-step-was-app? held- 
> posn-info))

pos turns into info

>                      (let*-values
>                          ([(step-kind)
>                            (if (and held-step-was-app?
> @@ -267,8 +275,11 @@
>
>                        (send-result
>                         (make-before-after-result
> -                        left-exps right-exps step-kind held-source- 
> pos
> -                        (syntax-position (mark-source (car mark- 
> list))))))]))]
> +                        left-exps right-exps step-kind
> +                        held-posn-info
> +                        (make-posn-info
> +                         (syntax-position (mark-source (car mark- 
> list)))
> +                         (syntax-span (mark-source (car mark- 
> list)))))))]))]

passing a posn-info rather than just a number

>
>                  [(double-break)
>                   ;; a double-break occurs at the beginning of a let's
> @@ -284,13 +295,16 @@
>                                          (maybe-lift (car  
> reconstruct-result) #f))]
>                          [right-side (map (lambda (exp) (unwind exp  
> render-settings))
>                                           (maybe-lift (cadr  
> reconstruct-result) #t))])
> -                   ;; add highlighting code as for other cases...
> -                   (receive-result
> -                    (make-before-after-result
> -                     (append new-finished-list left-side)
> -                     (append new-finished-list right-side)
> -                     'normal
> -                     #f #f)))]
> +                   (let ([posn-info (make-posn-info
> +                                     (syntax-position (mark-source  
> (car mark-list)))
> +                                     (syntax-span (mark-source (car  
> mark-list))))])
> +                     (receive-result
> +                      (make-before-after-result
> +                       (append new-finished-list left-side)
> +                       (append new-finished-list right-side)
> +                       'normal
> +                       posn-info
> +                       posn-info))))]

define a new posn-info, pass it twice instead of #f

>
>                  [(expr-finished-break)
>                   (unless (not mark-list)
> @@ -323,13 +337,13 @@
>      (match held-exp-list
>        [(struct no-sexp ())
>          (receive-result (make-error-result message))]
> -      [(struct held (exps dc source-pos))
> +      [(struct held (exps dc posn-info))

pos turns into info

>         (begin
>           (receive-result
>            (make-before-error-result (append held-finished-list exps)
>                                      message
>                                      #f
> -                                    source-pos))
> +                                    posn-info))

pos turns into info

>           (set! held-exp-list the-no-sexp))]))
>
>    (program-expander
> Index: private/annotate.ss
> ===================================================================
> --- private/annotate.ss	(revision 14506)
> +++ private/annotate.ss	(working copy)
> @@ -35,18 +35,6 @@
>     . -> .
>     syntax?)]                       ; results
>
> - [annotate/not-top-level           ;; SAME CONTRACT AS ANNOTATE!
> -  (syntax?                         ; syntax to annotate
> -   (((or/c continuation-mark-set? false/c)
> -     break-kind?)
> -    (list?)
> -    . opt->* .
> -    (any/c))                       ; procedure for runtime break
> -   boolean?                        ; show-lambdas-as-lambdas?
> -   (union any/c (symbols 'testing)); language-level
> -   . -> .
> -   syntax?)]                       ; results
> -

dead code removal

>
>   #;[top-level-rewrite (-> syntax? syntax?)])
>
>  ;  ;;                                              ;;;;                          ;
> @@ -272,7 +260,7 @@
>
>
>
> -(define ((annotate/master input-is-top-level?) main-exp break show- 
> lambdas-as-lambdas? language-level)
> +(define (annotate main-exp break show-lambdas-as-lambdas? language- 
> level)

dead code removal

>
>    #;(define _ (>>> main-exp #;(syntax->datum main-exp)))
>
> @@ -1135,12 +1123,13 @@
>                    (#%plain-lambda () . rest3)))
>           exp]
>          [else
> +         ;; I think we can re-enable this error now. I don't want  
> to do it right before a release, though. 2009-05-20

Comment change only.

>           #;
>           (error `annotate/top-level "unexpected top-level  
> expression: ~a\n"
>                  (syntax->datum exp))
>           (annotate/module-top-level exp)])))
>
> -  (define/contract annotate/top-level/acl2
> +  #;(define/contract annotate/top-level/acl2

dead code commenting-out

>      (syntax? . -> . syntax?)
>      (lambda (exp)
>        (syntax-case exp (begin define-values #%plain-app)
> @@ -1222,18 +1211,13 @@
>                #;(error `annotate/module-top-level "unexpected  
> module-top-level expression to annotate: ~a\n" (syntax->datum  
> exp))])]))
>
>    ; body of local
> -  (if input-is-top-level?
> -      (let* ([annotated-exp (cond
> -                              [(and (not (eq? language-level  
> 'testing))
> -                                    (string=? (language-level->name  
> language-level) "ACL2 Beginner (beta 8)"))
> -                               (annotate/top-level/acl2 main-exp)]
> -                              [else
> -                               (annotate/top-level main-exp)])])
> -        annotated-exp)
> -      (let*-2vals ([(annotated dont-care)
> -                    (annotate/inner (top-level-rewrite main-exp)  
> 'all #f #f)])
> -                  annotated)))
> +  (let* ([annotated-exp (cond
> +                          ;; support for ACL2 is commented out.
> +                          #;[(and (not (eq? language-level 'testing))
> +                                (string=? (language-level->name  
> language-level) "ACL2 Beginner (beta 8)"))
> +                           (annotate/top-level/acl2 main-exp)]
> +                          [else
> +                           (annotate/top-level main-exp)])])
> +    annotated-exp))
>
> -;; !@#$ defs have to appear after annotate/master.
> -(define annotate (annotate/master #t))
> -(define annotate/not-top-level (annotate/master #f))
> +

dead code removal.

> pcp063697pcs:~/plt/collects/stepper clements$ svn info .
> Path: .
> URL: http://svn.plt-scheme.org/plt/trunk/collects/stepper
> Repository Root: http://svn.plt-scheme.org/plt
> Repository UUID: dd0f82c0-e3f7-0310-82dd-f13d63558e96
> Revision: 14894
> Node Kind: directory
> Schedule: normal
> Last Changed Author: clements
> Last Changed Rev: 14893
> Last Changed Date: 2009-05-20 17:04:17 -0700 (Wed, 20 May 2009)
>
> pcp063697pcs:~/plt/collects/stepper clements$ svn status .
> pcp063697pcs:~/plt/collects/stepper clements$
>




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.racket-lang.org/dev/archive/attachments/20090520/8f882964/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2484 bytes
Desc: not available
URL: <http://lists.racket-lang.org/dev/archive/attachments/20090520/8f882964/attachment.p7s>

Posted on the dev mailing list.