Conversation
Makes it a lot faster
Move brittany unit tests into func tests to account for this
@@ -32,3 +32,6 @@ | |||
url = https://github.com/alanz/ghc-mod.git | |||
|
|||
|
|||
[submodule "submodules/floskell"] | |||
path = submodules/floskell | |||
url = https://github.com/bubba/floskell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What did you have to change in your fork?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjust version bounds and tweak it so it builds with earlier and later haskell-src-exts. I've made a PR for the changes so that hopefully we can drop the reliance on the submodule, since it brings us one step further away from #626
@@ -36,6 +37,7 @@ instance Default Config where | |||
, liquidOn = False | |||
, completionSnippetsOn = True | |||
, formatOnImportOn = True | |||
, formattingProvider = "brittany" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be a datatype instead of some text?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually originally had this as a datatype! But when adding the pluginFormattingProvider
field to the plugin descriptor it made more sense to revert it back to just the PluginId
, that way if a new plugin is added the developer doesn't need to edit Config.hs
as well.
I do agree though, if there are any tricks to having the PluginId
s exported in a more type safe manner then I would much prefer that
lf <- asks lspFuncs | ||
mc <- liftIO $ Core.config lf | ||
-- LL: Is this overengineered? Do we need a pluginFormattingProvider | ||
-- or should we just call plugins straight from here based on the providerType? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would personally not have this complication and just call the plugin directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having an entry in the plugin is a good thing.
Eventually we can manage config based on the installed providers too, and it opens the way to dynamically (or statically but from config) loading plugins. Which in my opinion has always been a goal.
LGTM |
This will expose an LSP config setting for changing the formatting provider, and so should also address #1064
Also worth noting: This moves Brittany off of
IdeGhcM
so it doesn't get clogged in the request queue, so requests should be faster.