Remove JSON transport #1105
Remove JSON transport #1105
Conversation
Simplify PluginCommands
Name was already being set by the LSP.command construction site, which makes more sense
Change IdePlugins from a newtype to a type
|
What's the motivation for this so I can understand the internal architecture better? |
|
@mpickering there is an option for hie binary for producing non LSP-compliant JSON on stdio. AFAIK it was implemented long time ago when LSP wasn't a thing (at least in hie) and since that time nobody cared about it because LSP is the main protocol of hie for 99% of use cases. |
|
Eventually, plugins may be loaded dynamically and would be specified by the user. If this PR removes plugin names then we need to look at namespacing and preventing conflicts when registering LSP command IDs. |
|
Adding a name to a plugin is not that hard so you should probably just keep the field. |
|
@alanz @mpickering I just realised: The commands were already namespaced with |
lukel97
force-pushed
the
remove-json-stdio
branch
from
2701d3d to
06769da
Compare
| , liquidDescriptor | ||
| , packageDescriptor | ||
| , pragmasDescriptor | ||
| , floskellDescriptor | ||
| ] |
There was a problem hiding this comment.
The reason I put in the plugin id string when the plugin list is assembled is so that people can easily change the list of installed plugins, and only need to worry about possible name clashes in one place, namely here. By delegating it to the individual plugins we lose this capability.
Unless there is some other mechanism to disambiguate them?
There was a problem hiding this comment.
I think this might need a larger discussion about how loading plugins will eventually look like.
I'm also thinking it might make more sense to remove pluginId as a field from PluginDescriptor, and have HIE generate a unique id at launch instead?
There was a problem hiding this comment.
Scrap that thought: It's used by liquid/ghc-mod and floskell/brittany to get specific plugins
| , pluginSymbolProvider = Nothing | ||
| , pluginFormattingProvider = Nothing | ||
| } | ||
|
|
There was a problem hiding this comment.
Where has this moved to? why?
There was a problem hiding this comment.
The Hoogle plugin didn't have any providers, and the lookup and info commands were just used as a part of haddock when it provides documentation (which are now just called directly instead of via the reactor). If the plugin doesn't provide any providers, it becomes a no-op to HIE, so I thought it would make sense to remove it as a plugin and make it more of a utility module for Hoogle related functions. i.e. if the user disabled the Hoogle plugin, nothing would happen: haddock still calls the methods here regardless.
|
@bubba I think I broke this with the recent commits, it needs a refresh |
|
To avoid confusion, perhaps you should close the PR? |
This PR removes the JSON transport in an attempt to simplify the internal architecture.
I have removed a lot of stuff aggressively and I expect I've removed too much: Please let me know if I should put anything back in!
Changes
PluginIdthemselves. Will help with plugins becoming standalone (and maybe eventually with dynamic loading)GhcMod.hsthat was exposed as a command but not exposed to LSP no longer define commands.Haskell.Ide.Engine.Pluginhave been moved up a levelPluginIdandCommandIdarenewtype'dTodo