Expose the the cradle config output

[madgen]

This restructures the code so that the selected configuration (whether a yaml-based or an implicit) is exposed in the API.

As a result of the rearrangement there is no longer any need to separately expose the implicit, default, and yaml cradle finding functions.

Basically, I want to use hie-bios to find the environment variables used while building a project for that I need access to the configuration it chooses. This is related to https://github.com/digital-asset/ghcide/issues/124.

I'm not entirely comfortable with the expected output, so if someone could test that these changes preserve behaviour, it would be great.

[ndmitchell]

In ghcide we need to know cheaply if two files get the same config. That doesn't seem possible anymore? We previously used the output of findCradle as the key.

[madgen]

@ndmitchell I restored findCradle now.

[madgen]

@fendor I apologise I think I accidentally somehow managed to delete your other review request.

> cradleConfig "exe/Main.hs"
Just (CradleConfig {cradleDependencies = [], cradleType = Cabal {component = Just "lib:hie-bios"}},".")

If I don't have a hie.yaml

Just (CradleConfig {cradleDependencies = [], cradleType = Cabal {component = Nothing}},".")

These are what I expect. Am I missing something?

[fendor]

@fendor I apologise I think I accidentally somehow managed to delete your other review request.

> cradleConfig "exe/Main.hs"
Just (CradleConfig {cradleDependencies = [], cradleType = Cabal {component = Just "lib:hie-bios"}},".")

If I don't have a hie.yaml

Just (CradleConfig {cradleDependencies = [], cradleType = Cabal {component = Nothing}},".")

These are what I expect. Am I missing something?

I deleted it, because I realized I made a mistake!
I expected the path to be absolute, since it is absolute in the Cradle, but it is not in the config and that is fine.

[mpickering]

Why is this deleted?

[madgen]

Their only use was from Main.hs and I thought it would be better to encapsulate Cradle selection to a single function. Do you want me to revert it?

[mpickering]

I don't really understand the API changes in this PR. Do I understand right the goal is to expose the selected CradleConfig?

[madgen]

@mpickering exposing CradleConfig is one goal. Another is to have one function that decides on which cradle to use rather than deciding it in Main.hs, i.e., return implicit if you can't find one logic.

[mpickering]

I'm not sure what is better but would this API work?

• findCradle :: FilePath -> IO (Maybe FilePath, Origin CradleConfig)
• Add a cradle root dir field to CradleConfig
• data Origin a = Explicit a | Implicit a
• loadCradle :: CradleConfig -> IO Cradle

[madgen]

@mpickering yes and I think your suggestion is cleaner.

I can implement that either as an amendment to this PR or another one from scratch?

[mpickering]

Or perhaps

data Origin a = Explicit FilePath a | Implicit a

[madgen]

Or perhaps

data Origin a = Explicit FilePath a | Implicit a

does that mean findCradle would be IO (Origin CradleConfig)?

[mpickering]

I'm also not sure that loadCradle needs to be in IO. I suspect not.

The difference between CradleConfig and Cradle is meant to be that CradleConfig closely corresponds to what you can put in a hie.yaml file.

[mpickering]

Or perhaps

data Origin a = Explicit FilePath a | Implicit a

does that mean findCradle would be IO (Origin CradleConfig)?

Yes exactly.

[madgen]

Yes, it wouldn't because the IO just happens in findCradle with this interface?

[madgen]

Let me know if you want me to implement this with the interface you suggested and whether you want it as a separate PR or as an amendment to this one and I'll get on it.

[mpickering]

@madgen It would be good if you could implement this interface. Either here or a separate PR is fine.

[mpickering]

I was looking into this ticket again today and it seems a bit awkward to implement. You really want to distinguish between the case where there's a hie.yaml file or not. If there is explicitly configuration but if it fails to parse then you want to report that to the user, but if there is no explicit configuration then you want to use the implicit search.

I think the current API makes expressing this quite easy unless I am missing something.

[madgen]

@mpickering Yes, I've found some problems with the suggested API as well. Sorry, I just haven't got around to forming coherent thoughts. I'll take some time to do it tomorrow.