Skip to content

Remove the ghcide executable #2979

@michaelpj

Description

@michaelpj
Collaborator

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?

Activity

pepeiborra

pepeiborra commented on Jun 21, 2022

@pepeiborra
Collaborator

+1

Previous discussions:

#813
haskell/ghcide#939

fendor

fendor commented on Oct 5, 2022

@fendor
Collaborator

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

soulomoon commented on Mar 21, 2024

@soulomoon
Collaborator

+1

soulomoon

soulomoon commented on Mar 21, 2024

@soulomoon
Collaborator

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

fendor commented on Mar 22, 2024

@fendor
Collaborator

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:

hls-deps drawio(1)

I hope there are no mistakes, but it can help identifying why it is non-trivial to refactor 🙃

wz1000

wz1000 commented on Mar 22, 2024

@wz1000
Collaborator

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

michaelpj commented on Mar 22, 2024

@michaelpj
CollaboratorAuthor

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

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, the Typecheck 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 in ghcide that could become hls-core or something.

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.

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

wz1000 commented on Mar 22, 2024

@wz1000
Collaborator

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.

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

michaelpj commented on Mar 22, 2024

@michaelpj
CollaboratorAuthor

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

soulomoon commented on Mar 23, 2024

@soulomoon
Collaborator

Nice graph @fendor, it is even messier if we consider the hls-bench and ghcide-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 versus ghcide-bench.
  • exe:ghcide versus exe:hls.
  • ghcide-test-utils versus hls-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 off ghcide as in @michaelpj's vision , making things more modularized. As for @wz1000 's concerns, I agree on the minimal HLS executable.

Since the refactor is non-trivial as @fendor point out in the graph. It is not realistic do everything in one go. But we can start attacking it by weakening the ghcide 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.

  1. Plugin that take away PluginMethodHandler and its test from ghcide, as what is already suggested. To prove the concept, something like this add core plugin soulomoon/haskell-language-server#3
  2. The dependency "Some plugins -> ghcide-test-utils" can shifit to Some plugins -> hls-test-utils.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @pepeiborra@michaelpj@wz1000@fendor@Ailrun

        Issue actions

          Remove the ghcide executable · Issue #2979 · haskell/haskell-language-server