racket / drracket

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

Accessing standard s-expression indentation rules #394

Closed cfinegan closed 4 years ago

cfinegan commented 4 years ago

I'm working on a Language Server Protocol implementation for Racket that seeks to use drracket:indentation to facilitate code formatting. I'm having some difficulty getting this to work for #lang racket and #lang racket/base, although it appears to work okay for other languages. Consider the following example:

#lang racket

(define in (open-input-string "#lang scribble/base\n"))
(define get-info (read-language in))
(and get-info (get-info 'drracket:indentation #f))

This code, when run on Racket 7.7, outputs #<procedure:determine-spaces>, which is exactly what I want. When I run the same code with "#lang racket/base\n" instead, it outputs #f. The indentation docs say that "If #f is returned, DrRacket uses the standard s-expression indentation rules." However, I cannot find any way to programmatically access these standard rules. Are they publicly available somewhere so I can get something similar to the #<procedure:determine-spaces>, but for #lang racket and #lang racket/base?

rfindler commented 4 years ago

You have to use this method:

https://docs.racket-lang.org/framework/Racket.html?q=compute-racket-amount-to-indent#%28meth._%28%28%28lib._framework%2Fmain..rkt%29._racket~3atext~3c~25~3e%29._compute-racket-amount-to-indent%29%29

The docs should probably make this explicit.

cfinegan commented 4 years ago

Thank you for the link! That is definitely what I'm looking for, and it works mostly great. However, I'm having some issues where compute-racket-amount-to-indent is giving different results than the actual indenter in DrRacket. Consider the following minimal example, which hopefully demonstrates correct usage of the API:

#lang racket
(require framework)

(define text (new racket:text%))
(define in (current-input-port))
(port-count-lines! in)
(send text insert (port->string in) 0)
(match-define-values (num-lines _ _)
  (port-next-location in))
(for ([line (in-range num-lines)])
  (define line-start (send text paragraph-start-position line))
  (define line-end (send text paragraph-end-position line))
  (define line-text (send text get-text line-start line-end))
  (define spaces (send text compute-racket-amount-to-indent line-start))
  (printf "~a:~a\n" spaces line-text))

It loops over the lines of the input file and prints the suggested number of leading spaces, as well as the line itself. When run on this this example input, its output begins with the following:

0:#lang racket/base
0:(require (for-syntax racket/base)
8:         data/interval-map
9:         framework
9:         json
9:         racket/class
9:         racket/contract/base
9:         racket/list
...

Note that on the 3rd line, it suggests an indentation of 8, instead of 9 like the subsequent lines. There are more of these off-by-one errors scattered throughout the output, where the suggested indentation is 1 less than the actual indentation observed when the document is edited in DrRacket.

sorawee commented 4 years ago

I used compute-racket-amount-to-indent recently, and recall that you must not use (port-count-lines! in). But that also means that you can't compute num-lines the way you did. Instead, use the last-paragraph method.

sorawee commented 4 years ago

Actually, just also recall that you can use (port-count-lines! in), but needs to use insert-port directly instead of using insert with port->string.

rfindler commented 4 years ago

I'm not sure what's going on with the off-by-one issues in the example above but consider using the load-file method to read in the data (and then, if you're worried, call set-filename with #f).

(Also, I think the docs probably need an update based on the original question!)

cfinegan commented 4 years ago

The document text is sent as a string over JSON RPC, so I don't have the option to fiddle with the filesystem. In the real code that my minimal example is based on, in is constructed using open-input-string. In the example, it's simply stdin. In both cases, the same off-by-one issues are observed. I also tried both revisions suggested by @sorawee and didn't see any change in the output.

rfindler commented 4 years ago

With this small variation of your program I see the expected output (lots of 9s):

#lang racket
(require framework)

(define text (new racket:text%))
(define in (open-input-string
            (string-append
             "#lang racket/base\n"
             "(require (for-syntax racket/base)\n"
             "         data/interval-map\n"
             "         framework\n"
             "         json\n"
             "         racket/class\n"
             "         racket/contract/base\n"
             "         racket/list\n")))
(port-count-lines! in)
(send text insert (port->string in) 0)
(match-define-values (num-lines _ _)
  (port-next-location in))
(for ([line (in-range num-lines)])
  (define line-start (send text paragraph-start-position line))
  (define line-end (send text paragraph-end-position line))
  (define line-text (send text get-text line-start line-end))
  (define spaces (send text compute-racket-amount-to-indent line-start))
  (printf "~a:~a\n" spaces line-text))

Can you get the bad behavior with a string constant like that somehow?

sorawee commented 4 years ago

What OS are you using? I feel like there must be hidden characters somewhere (\r?)

cfinegan commented 4 years ago

Yes, I apologize for not giving more config info earlier. This is all using: Ubuntu 16.04 LTS and Racket/DrRacket v7.6.0.9. The files are all UTF-8/LF. The off-by-one behavior is not random, it seems to always happen to the 2nd line of a require statement at the top of the file, and seems to frequently affect nested expressions with keywords inside match expander invocations. Because it seems to behave semi-consistently across files, I don't think this is an issue with encodings or hidden characters.

cfinegan commented 4 years ago

@rfindler when I run your modified code in DrRacket I get the following output:

0:#lang racket/base
0:(require (for-syntax racket/base)
8:         data/interval-map
9:         framework
9:         json
9:         racket/class
9:         racket/contract/base
9:         racket/list
9:

So still the same off-by-one

rfindler commented 4 years ago

I see the same output you see in 7.6, but not in 7.7 or in more recent versions.

If you need to use 7.6, try making the text% object have a display, like this:

#lang racket
(require framework racket/gui/base)
(define f (new frame% [label ""]))
(define text (new racket:text%))
(define ec (new editor-canvas% [parent f] [editor text]))
(define in (open-input-string
            (string-append
             "#lang racket/base\n"
             "(require (for-syntax racket/base)\n"
             "         data/interval-map\n"
             "         framework\n"
             "         json\n"
             "         racket/class\n"
             "         racket/contract/base\n"
             "         racket/list\n")))
(port-count-lines! in)
(send text insert (port->string in) 0)
(match-define-values (num-lines _ _)
  (port-next-location in))
(for ([line (in-range num-lines)])
  (define line-start (send text paragraph-start-position line))
  (define line-end (send text paragraph-end-position line))
  (define line-text (send text get-text line-start line-end))
  (define spaces (send text compute-racket-amount-to-indent line-start))
  (printf "~a:~a\n" spaces line-text))
cfinegan commented 4 years ago

I'm getting the correct behavior on v7.7, thanks for your help!

One last question: I want to require that users of my collection have the correct version of gui-lib installed by requiring ("gui-lib" #:version "1.47"), but I noticed that, assuming that the fix was introduced by this commit, the version number was not incremented when the fix was introduced, so I don't think requiring v1.47 guarantees that users get the correct behavior. If I require ("base" #:version "7.7.0"), is that enough to guarantee that users have an updated version of gui-lib? Or are there upgrade paths to base v7.7 that wouldn't also upgrade gui-lib?

rfindler commented 4 years ago

I don't know if there are any feasible ways to get that version of base and not have a gui-lib without that commit, but there might be. I could bump the gui-lib version number now, but then you would not be able to use the 7.7 release so I guess that's not helpful?

Probably making a graphical context is safer (you don't need to show the frame) if you're worried about your clients.

cfinegan commented 4 years ago

I'm totally fine with forcing users to upgrade. racket-langserver is essentially a headless mode for DrRacket, so I think it makes sense to expect users to be running on a version of Racket with this fix enabled.

My primary concern is people with outdated Racket installs using raco pkg install racket-langserver and then getting subtly wrong behavior with no indication of what the fix is. I'd like the installer to fail for these people and tell them to go upgrade their version of Racket. I'm not really concerned about guaranteeing correctness for people who upgraded their core packages selectively and ended up with some Frankenstein configuration.