Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

housekeeping between ghcide and hls #948

Closed
alanz opened this issue Mar 10, 2020 · 10 comments
Closed

housekeeping between ghcide and hls #948

alanz opened this issue Mar 10, 2020 · 10 comments

Comments

@alanz
Copy link
Collaborator

alanz commented Mar 10, 2020

ghcide is primarily a library, which is then used in both ghcide (the exe) and as a building block for hls.

This is working well, except for one minor niggle.

Over time, the amount of functionality in the ghcide/exe directory is growing. This is not shared between the applications, and requires a manual sync when it does change.

Can we try to reduce ghcide/exe/ to contain just Main.hs, which does the minimum to configure and run the specific incarnation it is being used for?

@ndmitchell
Copy link
Collaborator

I think the thing that bloated up recently is the cradle initialisation parts. That's tricky, because there is a lot of logic there, but we don't yet really know the right abstractions. We always wanted to keep the core library free of hie-bios (it makes good hygiene sense, and is also helpful for DAML), and that still feels the right line to draw, but there is increasingly more stuff left out of it.

Maybe we should wait for the multi-component stuff to land, as I imagine that is going to add more to the exe part, but also potentially show where the abstraction points can be cleanly cut to move more stuff into the library?

Or maybe once hls is in full swing, we can ditch the exe beyond basic testing, and use hls as the development host for ghcide?

@alanz
Copy link
Collaborator Author

alanz commented Mar 10, 2020

I am happy to wait a while, for things to stabilise. The main things is to start a discussion and decide on a course for the long haul.

If that takes too long, an interim solution could be to expose a second library in ghcide with the hie-bios initialisation stuff. And we can migrate that to hls if we choose to at an appropriate time.

I know @fendor and @jneira are talking about breaking out some commonality from haskell-ide-engine too, which is shared between it and haskell-language-server. I suspect the overall transition period is going to be longer than we would like, so should probably be managed clearly.

@alanz
Copy link
Collaborator Author

alanz commented May 3, 2020

I think it is time to re-visit this discussion.

Ping @cocreature @ndmitchell @fendor @jneira @mpickering and anyone else with an opinion

@ndmitchell
Copy link
Collaborator

I'm of the opinion that recent changes from @mpickering and @pepeiborra are still waiting to land, and we won't know the final state until they have. However, I think the direction they are going is more complexity, which needs to get shifted somewhere. I really like that idea that hie-bios doesn't intertwine with GHC API, and ghcide doesn't intertwine with hie-bios, then hls brings them both together - it seems a beautiful API separation. But it's going to require the complexity in loadSession to take be separated into a few higher-order functions if it's going to be feasible to maintain.

@pepeiborra
Copy link
Collaborator

Now that all our changes have landed, and the Main module in the ghcide executable stanza has grown quite sizable, perhaps it could be extracted into a ghcide-hiebios-exe library?
Given that Cabal supports packages with multiple libraries now, maybe withing the same package even.

@alanz
Copy link
Collaborator Author

alanz commented Jun 28, 2020

I agree that it should be extracted, and s sub library in the same package makes sense.

@jneira
Copy link
Member

jneira commented Jun 28, 2020

That would be the optimal configuration, only want to remember that hie-bios stack based hie.yaml does not work with private libraries for now: #114

@ndmitchell
Copy link
Collaborator

My concern is that sublibraries aren't very well supported in Haskell. And you really have to keep it in this repo, since otherwise you can't test ghcide. But it would be nice if this code belonged to HLS, not Ghcide. It seems to be stuck between three alternatives, none of them great.

@jneira
Copy link
Member

jneira commented Jul 3, 2020

I would bet for creating subpackages within the repo:

  • hls-plugin-api with the common modules to support plugins
  • hls-plugins with all the existing plugins, they could be splitted afterwards, it will depende on the previous one.
  • hlint-hls-plugin, given it needs a special configuration for ghc-lib (see Integrate HLint plugin ghcide#104). It will depend on hls-plugin-api
  • ghcide-hie-bios-exe (or simply ghcide-bios?) in the ghcide repo (if maintainers agree). It will depend on ghcide and hie-bios
  • hls-bios (??) with the specific hls changes over the previous one
    • given we are going to depend on implicit-hie that in turn could depend on Cabal to being able to cover more cases
    • not sure abouth this one, maybe it could be in the hls executable or fused with the previous one if everybody agree
    • it will need that ghcide-bios has the proper extension points to add the changes (with higher order functions as @ndmitchell suggested?)

lukel97 referenced this issue in lukel97/ghcide Jul 14, 2020
This way haskell-language-server can also reuse this logic.
Part of the work towards #478
lukel97 referenced this issue in lukel97/ghcide Jul 14, 2020
This way haskell-language-server can also reuse this logic.
Part of the work towards #478
lukel97 referenced this issue in lukel97/ghcide Jul 14, 2020
This way haskell-language-server can also reuse this logic.
Part of the work towards #478
lukel97 referenced this issue in lukel97/ghcide Jul 14, 2020
This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards #478
lukel97 referenced this issue in lukel97/ghcide Jul 14, 2020
This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards #478
lukel97 referenced this issue in lukel97/ghcide Jul 20, 2020
This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards #478
cocreature referenced this issue in haskell/ghcide Jul 27, 2020
* Split out the session loading logic into a sublibrary

This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards #478

* Move Development.IDE.Session into ghcide itself

Sublibraries do not seem to play well. Hide this behind the ghc-lib flag
so that the Haskell specific hie-bios stuff can be disabled

Note that we need to put the template-haskell part of this module into a
separate module because of an access exception when compiling with
Stack, GHC 8.10.1 and Windows.
@jneira
Copy link
Member

jneira commented Sep 15, 2020

I think this is almost done cause plugins had been reestructured in hls and the Main.hs exes for both ghcide and hls had been slimmed down. Only left make hls use ghcide master (tracked with haskell/ghcide#173)
Could we close this one @alanz?

@alanz alanz closed this as completed Sep 15, 2020
pepeiborra referenced this issue in pepeiborra/ide Dec 29, 2020
* Split out the session loading logic into a sublibrary

This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards haskell/ghcide#478

* Move Development.IDE.Session into ghcide itself

Sublibraries do not seem to play well. Hide this behind the ghc-lib flag
so that the Haskell specific hie-bios stuff can be disabled

Note that we need to put the template-haskell part of this module into a
separate module because of an access exception when compiling with
Stack, GHC 8.10.1 and Windows.
pepeiborra referenced this issue in pepeiborra/ide Dec 29, 2020
* Split out the session loading logic into a sublibrary

This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards haskell/ghcide#478

* Move Development.IDE.Session into ghcide itself

Sublibraries do not seem to play well. Hide this behind the ghc-lib flag
so that the Haskell specific hie-bios stuff can be disabled

Note that we need to put the template-haskell part of this module into a
separate module because of an access exception when compiling with
Stack, GHC 8.10.1 and Windows.
pepeiborra referenced this issue in pepeiborra/ide Dec 29, 2020
* Split out the session loading logic into a sublibrary

This way haskell-language-server can also reuse this logic.
Note that this sublibrary is public so it requires cabal-version: 3.0
Part of the work towards haskell/ghcide#478

* Move Development.IDE.Session into ghcide itself

Sublibraries do not seem to play well. Hide this behind the ghc-lib flag
so that the Haskell specific hie-bios stuff can be disabled

Note that we need to put the template-haskell part of this module into a
separate module because of an access exception when compiling with
Stack, GHC 8.10.1 and Windows.
@pepeiborra pepeiborra transferred this issue from haskell/ghcide Jan 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants