racket / drracket

DrRacket, IDE for Racket
http://www.racket-lang.org/
Other
452 stars 94 forks source link

syncheck:add-definition-target does not use syncheck:find-source-object #485

Open greghendershott opened 3 years ago

greghendershott commented 3 years ago

IIUC syncheck-annotations<%> methods are supposed to follow the protocol of calling syncheck:find-source-object -- giving it the opportunity to skip the annotation, or return a value it desires. For example it could choose to return the original syntax object, or, it could return just syntax-source. Anyway, it gets to choose.

This should return #f if the source of this syntax object is uninteresting for annotations (if, for example, the only interesting annotations are those in the original file and this is a syntax object introduced by a macro and thus has a source location from some other file).

Otherwise, it should return some (non-#f) value that will then be passed to one of the other methods below as a source-obj argument.

However syncheck:add-definition-target seems to ignore this and hard wire syntax-source, which seems wrong. For example, I'd like it to be the full syntax object.

Should it be changed:


;; add-definition-target : syntax[(sequence of identifiers)] (listof symbol) -> void
(define (add-definition-target stx mods phase-level)
  (when mods
    (define defs-text (current-annotations))
    (for ([id (in-list (syntax->list stx))])
-     (define source (syntax-source id))
+     (define source (find-source-editor id))
      (define ib (identifier-binding id phase-level))
      (when (and (list? ib)
                 source
                 defs-text
                 (syntax-position id)
                 (syntax-span id))
        (let* ([pos-left (- (syntax-position id) 1)]
               [pos-right (+ pos-left (syntax-span id))])
          (send defs-text syncheck:add-definition-target
                source
                pos-left
                pos-right
                (list-ref ib 1)
                mods))))))
rfindler commented 3 years ago

I think you're right.