-
-
Notifications
You must be signed in to change notification settings - Fork 400
Open
Labels
Description
The HLS executable can already be built without all the plugins, there are flags for that. So it's unclear what purpose the ghcide
executable serves any more. Removing it would let us simplify some code, and perhaps move some of the glue code out of ghcide
(is that desirable?).
Anyone have any strong reasons to keep it?
fendor, Ailrun and soulomoon
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
pepeiborra commentedon Jun 21, 2022
+1
Previous discussions:
#813
haskell/ghcide#939
fendor commentedon Oct 5, 2022
I did take a look, and it is currently non-trivial because the ghcide-testsuite depends on the ghcide executable and instructs it to emit certain LSP messages for the tests.
To remove the ghcide executable, we need to make the testsuite depend on HLS which requires a bit more untangling of the hls-plugin-api/ghcide/HLS dependency graph.
soulomoon commentedon Mar 21, 2024
+1
soulomoon commentedon Mar 21, 2024
Do you guys think this might be a good approach? we start by adding a core plugin, then move most of the ghcide functionality to the core plugin bit by bit((switching the test to use hls test in the proccess). This should solve the dependency problem.
If you guys can agree with me on this approach, I'll add a core plugin and move a bit of ghcide functionality and test over. In time we can get rid of the dependency for the ghcide binary.
fendor commentedon Mar 22, 2024
To be perfectly clear, ghcide would not expose a plugin any more, consequentially there would be barely any tests in ghcide? I am not sure that's enough, I think we should first focus on the dependency graph and look at what dependency edges we wish to sever. I created this:
I hope there are no mistakes, but it can help identifying why it is non-trivial to refactor 🙃
wz1000 commentedon Mar 22, 2024
I find the
ghcide
executable very useful for quickly testing changes without waiting for all of HLS and its dependencies to build, and without manually disabling all the plugin flags.michaelpj commentedon Mar 22, 2024
In the long run I'm not sure even that is enough. If we were starting again today, I doubt we would have a separate package like
ghcide
at all. I imagine we might have:hls-core
: the rule system, the handlers and rules that deal with file system changes, position mapping etc.hls-haskell-plugin
: a "normal" plugin that provides handlers for Haskell stuff, theTypecheck
rules etc.I think what @soulomoon proposes is a way we could get there: create
hls-haskell-plugin
, gradually move the Haskell handlers over there. Most of the tests would go too. Then we'd be left with some random stuff inghcide
that could becomehls-core
or something.I don't think it would be that hard to define a minimal HLS executable that only includes the core Haskell plugins? The objectionable bit about the ghcide executable is that adds a lot of very similar code to the HLS one, and leads to some confusing attempts to abstract the bits that they share.
wz1000 commentedon Mar 22, 2024
it is already quite abstracted? All the ghcide executable does is set up its configuration and call
Development.IDE.Main.defaultMain
, which is shared between ghcide and HLS.michaelpj commentedon Mar 22, 2024
Sure, I'm just saying that it's confusing. And e.g. all the logger setup is duplicated (and not consistent). And they have different "commands" that we map between. It's just a bunch of unnecessary (IMO) stuff.
soulomoon commentedon Mar 23, 2024
Nice graph @fendor, it is even messier if we consider the
hls-bench
andghcide-bench
.We have duplicated "everything"
hls versus ghcide
, result in a clumsy set of duplicated code and concepts which is rather confusing as @michaelpj point out.hls-bench
versusghcide-bench
.exe:ghcide
versusexe:hls
.ghcide-test-utils
versushls-test-utils
.From a higher perspective,
ghcide
package is pretty self contained and having full set of functionalities. It is like a old kingdown on its own. But we would want Depriving a king of power, slowly killing offghcide
as in @michaelpj's vision , making things more modularized. As for @wz1000 's concerns, I agree on theminimal HLS executable
.Since the refactor is
non-trivial
as @fendor point out in the graph. It is not realistic do everything inone go
. But we can start attacking it by weakening theghcide
package, moving things from ghcide Package down to HLS Package or hls-test-util pcakge or up to hls-plugin-api package.Here is the part I think we can do first.
PluginMethodHandler
and itstest
fromghcide
, as what is already suggested. To prove the concept, something like this add core plugin soulomoon/haskell-language-server#3Some plugins -> hls-test-utils
.