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

Expose initSessionWithMessage #129

Merged
merged 1 commit into from
Jan 22, 2020

Conversation

Avi-D-coder
Copy link
Contributor

Needed by haskell/haskell-ide-engine#1589

Github and vim automatically added the newline so I gave up.

@fendor fendor merged commit c8b9877 into haskell:master Jan 22, 2020
@fendor
Copy link
Collaborator

fendor commented Jan 22, 2020

Thanks!

@fendor
Copy link
Collaborator

fendor commented Jan 23, 2020

Actually, now I am not entirely sold on the idea of just exposing initSessionWithMessage. Maybe we could change the type of initializeFlagsWithCradleWithMessage to also return the ComponentOptions if there are any? Otherwise the function initializeFlagsWithCradleWithMessage is rather useless, esepcially for Haskell IDE Engine, and I think we are the only users of that function.

Would you mind brainstorming a better API for initializeFlagsWithCradleWithMessage?

What do you think about:

initializeFlagsWithCradleWithMessage ::
  GhcMonad m
  => Maybe G.Messager
  -> FilePath -- ^ The file we are loading the 'Cradle' because of
  -> Cradle   -- ^ The cradle we want to load
  -> m (CradleLoadResult (ComponentOptions, m G.SuccessFlag)) -- ^ Whether we actually loaded the cradle or not.

Disadvantage: looks dirty
Advantage: Does not force users of the lib to re-implement initializeFlagsWithCradleWithMessage with the newly exposed initSessionWithMessage.

@Avi-D-coder
Copy link
Contributor Author

@fendor If we are truly the only user of this function we can change the return type and willing to make a breaking change, this would also get ride of the unreachable case (haskell/haskell-ide-engine#1589 (comment)). I was working under the erroneous assumption that a breaking change was not an option.

Ghcide does not use initializeFlagsWithCradleWithMessage, are there any other known users of hie-bios?

Avi-D-coder added a commit to Avi-D-coder/hie-bios that referenced this pull request Jan 23, 2020
@fendor
Copy link
Collaborator

fendor commented Jan 23, 2020

Ghcide uses nothing of the HIE.Bios.Ghc namespace and I am not aware of other big users. Moreover, next release will have breaking changes regarding the Cradle type anyways

@ndmitchell
Copy link
Contributor

Breaking changes are not a big deal to ghcide - we expect them at this stage in the maturity cycle.

Avi-D-coder added a commit to Avi-D-coder/hie-bios that referenced this pull request Jan 23, 2020
fendor pushed a commit that referenced this pull request Jan 23, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants