Ormolu range format support #1602
Conversation
| Range (Position sl 0) $ Position el $ T.length $ last txt | ||
| -- Pragmas will not be picked up in a non standard location, | ||
| -- or when range starts on a Pragma | ||
| extPragmas = takeWhile ("{-#" `T.isPrefixOf`) |
There was a problem hiding this comment.
Given we are working in the context of a module that is likely to already be loaded by GHC, in principle we should be able to query these directly.
There was a problem hiding this comment.
And if we put this behaviour into a utility, we can either get the pragmas from the module DynFlags, or do this parsing as a fallback. And the GHC API already has a function to do that.
There was a problem hiding this comment.
I didn't see that I could show extensions from the EnumSet in DynFlags, I mistakenly thought I was going to have to test if each extension was set, to construct the list.
Avi-D-coder
force-pushed
the
ormolu-range
branch
from
345c12e
to
4084859
Compare
| (IdeError | ||
| PluginError | ||
| (T.pack | ||
| "You must format a whole block of code. Ormolu does not support arbitrary ranges." |
There was a problem hiding this comment.
What user experience does this result in? A pop-up window? This is likely to be a fairly common experience, so should be non-threatening, just a notice.
Another possible approach is to find the enclosing block that can be formatted, and process that. What does the brittany plugin do in this case? I wonder if that logic should be captured in a library, for use in all RangeFormatting requests, so we have a standard behaviour across hie.
There was a problem hiding this comment.
I use nvim and coc so I don't know how it looks to most users. Brittany will often produce an error, in similar situations. The point of this line is just to provide a more helpful error than what we would usually get unable to parse.
There was a problem hiding this comment.
I am happy with the message, and its contents. I just want to make sure the end user experience makes sense. But it can be tweaked easily enough, so its not a blocker.
|
If there are no more objections I'm going to merge this. |
There was a problem hiding this comment.
Too complicated for me to review, I trust that @Avi-D-coder tested it. So, no explicit review from me, but I am happy with merging! Thank you for this nice feature addition!
|
There is one more edge case, reintroduced in the switch to |
|
I've been using this version today, no issues so far, so I'm going to merge. |
This is a bit of a hack but it works pretty great.
We format lines of equal or increasing indention, not precise ranges, and there are a few documented edge cases.