haskell
Remove exactprint dependencies from ghcide by introducing hls-refactor-plugin.
All code actions have been moved to hls-refactor-plugin
Mostly straightforward, only slight complication was that completion auto imports depends on exactprint, but I didn't want to remove all completion logic from ghcide just for this.
Instead, I added logic to dynamically lookup the plugin that provides the extend import command, so that auto imports work as expected when you have hls-refactor-plugin enabled.
@pepeiborra you might want to look the changes to showAstData in ghcide/src/Development/IDE/GHC/Dump.hs and see if you are comfortable with those. We could add a flag to use exactprint for that function.
Also the async tests now use code lenses instead of code actions, I believe this still preserves the expected test behavior, is that correct @pepeiborra?
@pepeiborra I've also replaced code actions in the benchmarks with code lenses. This is unfortunate, but we should fix this in the future by benchmarking the entire IDE with plugins included instead of just ghcide.
I don't love the idea of dynamically looking up another plugin just in order to get access to some helper code. I think that could do with some clear documentation and a big warning saying not to use it unless you really really have to. I'm not sure what the "right" solution is: probably moving the completions logic to a plugin and pulling out a common dependency?
I don't love the idea of dynamically looking up another plugin just in order to get access to some helper code. I think that could do with some clear documentation and a big warning saying not to use it unless you really really have to. I'm not sure what the "right" solution is: probably moving the completions logic to a plugin and pulling out a common dependency?
It isn't really a function call, it decides whether to respond with an LSP Command depending on whether or not a plugin that provides the correct command is enabled.
If we are serious about composing plugins together, this is behaviour we probably want to support, so that plugins can pass off tasks to each other in such cases.
Also there's still an inverse dependency, in that extendImportCommandId is still defined in ghcide. So this won't work as a way of composing plugins together unless you have a central registry of the possible command ids to look for.
I'm not sure what the alternative is though, strings or something :/
All the plugins that use the GetAnnotatedParsedSource rule now have to link it in via pluginRules to ensure they have access to it even if hls-refactor-plugin is disabled.
But this would mean hls-completions cannot be built until hls-refactor has been ported, so we would probably want to put the command id inside a new package hls-refactor-command-ids. Which can be done right now. @michaelpj what do you think?
Well, I think we should think about what a world where we do this more often looks like. I think we'd have to have a package where all the command IDs for all plugins are defined. That wouldn't be so crazy: that's the "interface definition" for a plugin in this case, and having a separate interface definition which calling modules depend on isn't that unusual. I think I'd rather have one such package than one per plugin, though...
It's still quite ugly in that it means that we end up with a bit of logical coupling between all our plugins in this location. I don't have any better ideas, though. I would be happiest if we could just avoid doing it :sweat_smile: So I would prefer hls-completions -> hls-refactor, or better hls-completions/hls-refactor -> hls-plugin-utils or whatever. But that's significantly more work.
so we would probably want to put the command id inside a new package hls-refactor-command-ids. Which can be done right now. @michaelpj what do you think?
I will do this in another MR. I think this one is ready to merge.