Eugleo / magic-racket

The best coding experience for Racket in VS Code
https://marketplace.visualstudio.com/items?itemName=evzen-wybitul.magic-racket
GNU General Public License v3.0
204 stars 28 forks source link

Add formatting functionality #9

Closed Nexuist closed 3 years ago

Nexuist commented 4 years ago

Hello,

First of all, thank you for this package. I use it daily for my business.

One crucial addition I would like is to add formatting, i.e. cmd + F or formatting on save to make sure each file maintains the same level of indentation and long segments are broken down uniformly.

As far as I can tell, VSCode has some sort of support for this type of plugin, but whenever I try to format I get:

There is no formatter for 'racket' files installed.

Would it be possible to add this to this package?

Let me know how I can be of assistance.

Eugleo commented 4 years ago

Hey, great to hear that! Formatting support is on my todo list — VSCode indeed does support such plugins, but first I'll need to write a command-line formatter for racket source and then link it in VSCode. That will take some time and I'm not yet sure how exactly I want to progress.

Nexuist commented 4 years ago

Awesome, great to hear! Could the Racket language server help here?

Eugleo commented 4 years ago

I don't think so, at least for the time being — in the latest LSP implementation, racket-langserver, code formatting is listed as "WIP" (for quite some time now). I think I'll have to implement it myself.

I'm actually thinking about the design of such formatter; rather than formatter, I'd maybe only do a "reindenter" of sorts, which would only indent each line to the correct amount, but wouldn't touch anything else (similar to how DrRacket does it). What do you think?

Nexuist commented 4 years ago

I did some very basic searching and found this: https://github.com/russellw/racket-format

Seems to fit the bill for what you'd want. I like the idea of having it only perform indenting. The only other thing I would like to see it do is break named parameters into separate lines:

(make-event #:time 1230 #:day "11/19" #:name "Post an issue" #:details "More information...")

Would become:

(make-event #:time 1230
            #:day "11/19"
            #:name "Post an issue" 
            #:details "More information...")

Other than that, though, I think it would work perfectly doing just indentation. Let me know if that other guy's CLI works for you.

Eugleo commented 4 years ago

Thanks for the heads up! I'll take a look; I'll need to repackage it, though, to make the installation a little bit easier. And while I'm at it, I could also rewrite it altogether to get some practice, and maybe even add some features, like the one you're suggesting.

Another idea I got was that it might be nice to break the lines, but only sometimes, depending on context. It is similar to your keywords example, but more general, e.g.

(make-event #:time 1230 #:day "11/19" #:name "Post an issue" #:details "More information...")

would stay unchanged, but

(make-event #:time 1230 
            #:day "11/19" #:name "Post an issue" #:details "More information...")

would be become

(make-event #:time 1230
            #:day "11/19"
            #:name "Post an issue" 
            #:details "More information...")

as the user signaled he wants the arguments on separate lines. The same would hold for positional arguments and other lists as well:

; Unchanged
(list 1 2 3 4)

; But this gets formatted
(list 1 2    
 3 4)

(list 1 
      2 
      3 
      4)

But I'll start with the simpler version first.

Nexuist commented 4 years ago

Great idea. Hopefully the existing CLI gives you some guidance so you don't have to start from scratch. Let me know how things go, I'm eager to test this!

wine99 commented 4 years ago

Good to know that you have this feature in your TODOs. Magic Racket is awesome but I think this is a quite crucial feature, at least for me, so hopefully this can be done in the near future.

JoeHowarth commented 3 years ago

Also would really love this as a feature!

micahcantor commented 3 years ago

Hey @Eugleo , did you ever make any progress on this, either by integrating DrRacket's formatting into the Racket LSP, or instead by writing a standalone formatter? I would be willing to help or pick up where you left off along either of these lines, but didn't want to barge in if you already made some progress.

mlavrent commented 3 years ago

@Eugleo @micahcantor I'm also looking for this feature and would be happy to work on it as well. Let me know if you'd want to coordinate.

micahcantor commented 3 years ago

@MLavrentyev Great, so this VSCode article explains how to use the Formatting API to create a formatter in VSCode. The example they give is this:

vscode.languages.registerDocumentFormattingEditProvider('foo-lang', {
  provideDocumentFormattingEdits(document: vscode.TextDocument): vscode.TextEdit[] {
    const firstLine = document.lineAt(0);
    if (firstLine.text !== '42') {
      return [vscode.TextEdit.insert(firstLine.range.start, '42\n')];
    }
  }
});

So essentially what we would have to do to add a formatter to Magic Racket is have some cli program that formats the current document and outputs the formatted text. Then we can load that (line by line?) into the vscode document using the API.

As far as standalone formatters go, the only candidate I found was this repo, which isn't actively developed but I tried it and it seems to work. If we used this, we could instruct users to install this package like they do with the racket-langserver if they want formatting capabilities.

Alternatively, we could look to how DrRacket or Emacs racket-mode implement this feature, and try to replicate that here, or really, in the racket-langserver. I know there was an effort to implement formatting in the lsp already and it stalled.

Eugleo commented 3 years ago

@micahcantor I think making our own standalone formatting utility is the way to go (not to bash the existing formatter, but I wouldn't put much hopes into an unmaintained project). My original plan was to implement the formatter according to this paper, which is at the base of JS's prettier. I'll need to re-read the paper again, since it's been quite some time since I've skimmed it, but I think it shouldn't be too hard.

mlavrent commented 3 years ago

Awesome, thanks for all the info, @Eugleo! Finals end Dec. 10 for me, so I'll be focusing my full efforts on this after that. I'll make a pr when it's done.

Eugleo commented 3 years ago

@MLavrentyev I'll spin up another repo, as I plan to work on this after the exam period as well and it might be better to have the formatter be a standalone project. I will keep you updated.

mlavrent commented 3 years ago

Just to throw out there, another thing to consider supporting is respecting different #lang formatters too. This, I think, is usually defined somewhere close to the required info.rkt so we probably want to be auto-formatting based off that. If we're looking to copy DrRacket's formatting, this seems relevant.

For racket specifically, this is probably relevant.

Eugleo commented 3 years ago

Great point. A better support for custom langs on any front will be a big plus. Anyway, the (empty) repo is set up here: https://github.com/Eugleo/bracquet.

Eugleo commented 3 years ago

By the way the design goals have shifted a little bit, as you can probably tell. Originally I though a simple realign would do, keeping the newlines wherever the user has put them. However, I now immensely enjoy prettier (and black, for that matter), and I think its "parse the whole AST and then pretty print it" approach is the way to go.

mlavrent commented 3 years ago

I've just reached out to the people who make DrRacket - here's their answer regarding re-using DrRacket's formatting functionality @Eugleo. Note that this gets us the goal of auto-tabbing (which is a good start), but if we want to auto-newline parameters, etc., there's probably more to be done (though I think this should probably be a setting you can turn on/off since it makes sense to sometimes not put each param on a new line).

(I also don't think we want to handle WXME files, right?)

If you want to properly handle WXME format files, you'll need a little more smarts, but hopefully this gets you started. The indenter isn't fast but most of the time that this script takes on the example below is loading in the code.

$  (echo "#lang racket" ; echo "(define (f x)" ; echo "(if (= x 0)" ; echo "1" ; echo "2))") | racket ~/x.rkt
#lang racket
(define (f x)
  (if (= x 0)
      1
      2))
$  cat ~/x.rkt
#lang racket/gui
(require framework)
(define t (new racket:text%))
(void (send t insert-port (current-input-port)))
(send t tabify-all)
(void (send t save-port (current-output-port) 'text))
$ 
micahcantor commented 3 years ago

I agree about implementing a formatter from scratch in the long run, other options seem to have some downfalls, but integrating an indenter script in the mean time as @MLavrentyev suggested seems viable?

mlavrent commented 3 years ago

I'll make a pr with this bit shortly, then, and we can look into extending it separately.

mlavrent commented 3 years ago

Looking at this further, racket-langserver already supports basic Racket indentation (but not yet for custom #langs). That should be fairly easy enough to integrate into, since this uses that already, right?

In particular, see here and here.

Eugleo commented 3 years ago

Wow, I didn't notice this, good find! In that case, the integration should very well be possible, easy, even. You can take a look at the existing LSP integration and at the VSCode programmatic language features docs where the methods for implementing formatting into the client are documented.

mlavrent commented 3 years ago

I currently see where the client is being started up, but how is it actually making calls to the language server/where is that happening for the other bits that you use racket-langserver for already? @Eugleo

Eugleo commented 3 years ago

Now that I think about it, it might have been a little misleading to tell you to look at the existing implementation. AFAIK the VSCode client handles the messaging itself, you just tell it where to send the messages, and then launch it by

langClient.start();
mlavrent commented 3 years ago

Ah I see ok :). That makes sense, thanks. I'm working on a pr for racket-langserver first to turn on the formatting feature (they already mostly had it, but it was disabled), and then I'll make the pr here.

mlavrent commented 3 years ago

I finally have some free time to devote to this and adding/enabling the formatting in the racket-langserver, so hopefully I'll have something up over the holidays. Merry Christmas & a Happy New Year to all! 🎄

You can track the pr here: https://github.com/jeapostrophe/racket-langserver/pull/24

Nexuist commented 3 years ago

I just read through this thread and I wanted to drop in and say I'm really excited to see what comes of it! Thank you guys for keeping track of it even though it's been over a whole year since I made this. As always, let me know if you need help testing or debugging! I have a few thousand line codebase I'd love to put this through.

mlavrent commented 3 years ago

Now that jeapostrophe/racket-langserver#24 is merged in, I'm able to use the "Format Document" command in vscode to get proper indentation. Let me know if you run into further issues, but I think this can be closed.

Note that the formatter follows the DrRacket rules of just indenting correctly, rather than splitting lines and the like.

micahcantor commented 3 years ago

@MLavrentyev Thanks for your work on the langserver. However, I can't seem to get the formatting to work. When I use "Format Document" it says "There is no formatter installed for 'racket' files". I updated the langserver with raco but do you need to do anything else?

mlavrent commented 3 years ago

@micahcantor you should only need to update racket-langserver. Did you update after Tuesday, December 29th, 2020 11:42:45pm (UTC)?

If so, I would try uninstalling and reinstalling the package with:

raco pkg remove racket-langserver
raco pkg install racket-langserver

If this still doesn't fix it, I'd also try re-installing the magic racket extension. If there are still issues, would you mind dropping your racket version and os? I'll look into it.

micahcantor commented 3 years ago

@MLavrentyev Got it. I had to set Magic Racket as the default formatter for Racket in the settings.json. It may have just been a problem on my end, but for whatever reason the vscode GUI did not recognize that Magic Racket could be used as formatter.

mlavrent commented 3 years ago

Good to hear - this may be something we want to add to the readme then.

Eugleo commented 3 years ago

@MLavrentyev Maybe we have to signalise to VSCode that we provide this functionality? I'd guess either package.json or when we're registering the langserver. I'll look into it.

Nexuist commented 3 years ago

Hey @micahcantor - how are you telling VSCode to use Magic Racket as a formatter? I tried putting the following in settings.json:

"[racket]": {
    "editor.defaultFormatter": "evzen-wybitul.magic-racket"
  },
  "files.associations": {
    "*.html": "html",
    "*.rkt": "racket",
    "*.eta": "html"
  },

... But I still get the "There is no formatter for 'racket' files installed" popup whenever I try using Format Document. Is there something else I have to do?

Nexuist commented 3 years ago

Nevermind - I had to update racket-langserver :P

All works well now! Would definitely be nicer and more expected for it to work without modifying settings, though. I'd say that's the only other thing that should be added, and then I can close this issue with satisfaction. Thanks again to everyone who helped out!

Runi-c commented 3 years ago

racket-langserver supports onTypeFormatting (as of https://github.com/jeapostrophe/racket-langserver/pull/37) in addition to range and document formatting, so that should be pretty much everything discussed in this issue.

If you now enable "Format On Type" in VSC, S-Expressions will be auto-indented on closing and you'll get correct indentation when you insert a newline as well.

wjx commented 3 years ago

racket-langserver supports onTypeFormatting (as of jeapostrophe/racket-langserver#37) in addition to range and document formatting, so that should be pretty much everything discussed in this issue.

If you now enable "Format On Type" in VSC, S-Expressions will be auto-indented on closing and you'll get correct indentation when you insert a newline as well.

There's a delay between entering newline and indentation correction, whereas DrRacket gives correct indentation immediately.

Runi-c commented 3 years ago

The delay is more or less unavoidable when VSC and the langserver disagree on indentation - which is often, as Racket indentation rules can't be easily expressed to VSC.

There would need to be a way for the extension to synchronously give the correct indentation in order for it to be instant, and I'm not sure if such a way exists.

Eugleo commented 3 years ago

@wjx You can open a new issue if you're unhappy with the current state of formatting. As this original issue is resolved, now, thanks to @Runi-c, I'm closing it.