Add support for Fourmolu 0.8 #3078
Conversation
| emptyConfig :: FourmoluConfig | ||
| emptyConfig = | ||
| FourmoluConfig | ||
| { cfgFilePrinterOpts = mempty | ||
| , cfgFileFixities = mempty | ||
| } |
There was a problem hiding this comment.
condition will eventually be #if !MIN_VERSION_fourmolu(0,8,1): fourmolu/fourmolu#221
| first (mkError . show) | ||
| <$> try @OrmoluException (makeDiffTextEdit contents <$> ormolu config fp' (T.unpack contents)) | ||
| bimap (mkError . show) (makeDiffTextEdit contents) | ||
| <$> try @OrmoluException (ormolu config fp' (T.unpack contents)) |
There was a problem hiding this comment.
Nice. Surprised I didn't notice that opportunity.
| @@ -0,0 +1,68 @@ | |||
| {-# LANGUAGE CPP #-} | |||
|
|
|||
| module Ide.Plugin.Fourmolu.Shim ( | |||
There was a problem hiding this comment.
Excellent. This is much cleaner.
I don't see a reason to keep really old versions around, especially if it means huge amounts of CPP. But it's likely that some people won't want to be forced in to updating Fourmolu (and thus probably reformatting their codebase) by an HLS update*. Then again, part of the reason for adding the CLI mode was to give users that flexibility. I've wanted to remove some backwards compatibility before, but it wasn't possible then because of constraints from HLS' other dependencies: #2951. * Although in practice, most people are getting HLS from bindists, so they don't have much choice without building from source instead. |
|
@georgefst By "building from source", do you mean fourmolu? Now that we release binaries on the releases page, is there some hls logic that can download fourmolu binaries where possible instead of building fourmolu? (Even better, make that the canonical way; dont bundle fourmolu in hls, just have hls provide a dropdown to install version of fourmolu) Keeping old versions isnt terrible right now, because all the cpp is for the 0.7 upgrade. But I think maybe once 0.7 becomes two years old (or something) itd be nice to drop support |
I meant HLS, but I suppose that statement works for either, once you take the CLI option in to account. We could make downloading the binary the default, but I'm more confident of correctness when using the library. And it would go against the way every other HLS plugin is doing things (although that is not, in itself, a great argument against). |
Also includes some clean up. Will we always be supporting every version of Fourmolu, or will HLS ever remove support for old versions of Fourmolu?
cc @parsonsmatt @georgefst