Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix textDocument/documentSymbol #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions text-document.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@

;; Hover request
;; Returns an object conforming to the Hover interface, to
;; be used as the result of the response message.
;; be used as the result of the response message.
(define (hover id params)
(match params
[(hash-table ['textDocument (DocIdentifier #:uri uri)]
Expand Down Expand Up @@ -239,25 +239,28 @@
(hash-ref open-docs (string->symbol uri)))
(define in (open-input-string (send doc-text get-text)))
(port-count-lines! in)
(define lexer (get-lexer in))
(define lexer (get-lexer in)) ;; lexer: token producer
(define after-define? (box #f))
(define results
(for/fold ([out empty])
([lst (in-port (lexer-wrap lexer) in)])
(match-define (list text type paren? start end) lst)
(cond [(set-member? '(constant string symbol) type)
(define kind (match type
['constant SymbolKind-Constant]
['string SymbolKind-String]
['symbol SymbolKind-Variable]))
(define range
(Range #:start (abs-pos->Pos doc-text start)
#:end (abs-pos->Pos doc-text end)))
(define sym-info
(SymbolInfo #:name text
#:kind kind
#:location (Location #:uri uri
#:range range)))
(cons sym-info out)]
(cond [(eq? 'symbol type)
(cond [(set-member? '("define" "define-type" "define-syntax") text)
(set-box! after-define? #t)
out]
[(unbox after-define?)
(set-box! after-define? #f)
(define range
(Range #:start (abs-pos->Pos doc-text start)
#:end (abs-pos->Pos doc-text end)))
(define sym-info
(SymbolInfo #:name text
#:kind SymbolKind-Function
#:location (Location #:uri uri
#:range range)))
(cons sym-info out)]
[else out])]
[else out])))
(success-response id results)]
[_
Expand All @@ -277,9 +280,15 @@
(match-define-values
(_ _ _ _ _ _ lexer)
(module-lexer in 0 #f))
(if (procedure? lexer) ;; TODO: Is this an issue with module-lexer docs?
lexer
(error 'get-lexer "~v" lexer)))
#|
lexer is one of:
- 'before-lang-line
- lexer, the procedure
loop until it is a procedure
|#
(if (procedure? lexer)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I think is problematic, as for empty files it seems to cycle indefinitely. It's getting too late here, but I'll push a fix tomorrow. We should also properly detect the defining, as @jeapostrophe suggested.

Copy link

@Eugleo Eugleo May 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per module-lexer docs:

When mode is #f (indicating the start of the stream), the lexer checks in for a #lang specification.

If a #lang line is present but the specified language does not exist, the entire in input is consumed and colored as 'error.

If the language exists and the language provides a get-info function, then it is called with 'color-lexer. If the result is not #f, then it should be a lexer function for use with color:text<%>. The result mode is the lexer—paired with #f if the lexer is a procedure arity 3—so that future calls will dispatch to the language-supplied lexer.

If the language is specified but it provides no get-info or 'color-lexer result, then racket-lexer is returned as the mode.

When the file is empty, I'd say module-lexer will just return #f, among other things — calling it again and again won't change a thing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for @jeapostrophe: the default behaviour seems to be to error out if a lexer procedure isn't returned. I'm not familiar with your codebase nor with LSP too much, but it doesn't seem right to give errors even in non-erroneous state (= plain empty file).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty file is definitely an error in Racket, because a #lang or (module ...) is required.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, thanks.

lexer
(get-lexer in)))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Expand Down