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

Add Loading Style option to 'runAction' #433

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Apr 16, 2024

No description provided.

Copy link
Contributor

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly stupid question, but would it make more sense to just consider this a different cradle type? It's not clear that it really is an orthogonal setting, e.g. it makes no sense if you set it and you have a stack or direct cradle. Or maybe you should get an error in that case? But a separate cradle type might work nicely. Then in HLS/hie-bios we can give people a choice of direct/stack/cabal/cabal-multi.

src/HIE/Bios/Cradle.hs Outdated Show resolved Hide resolved
src/HIE/Bios/Cradle.hs Outdated Show resolved Hide resolved
@michaelpj
Copy link
Contributor

Also, if we did my suggestion about making this a cradle type, then exposing that in HLS would naturally fix haskell/haskell-language-server#3832

@fendor fendor force-pushed the enhance/loading-style branch 2 times, most recently from 6859fb1 to 9d51a5a Compare April 22, 2024 09:25
src/HIE/Bios/Cradle.hs Outdated Show resolved Hide resolved
pretty (LogRequestedCradleLoadStyle crdlName ls) =
"Request loading" <+> pretty crdlName <+> "cradle using" <+> case ls of
SingleComponent -> "Single Component Strategy"
LoadWithContext fps -> "Multiple Components Strategy:" <> line <> indent 4 (pretty fps)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The terminology seems a bit inconsistent now. Is it single-component vs multi-component, or single-component vs "with context"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like from the hie-bios perspective the truth is that it's either one file or many files, right? hie-bios doesn't particularly judge whether or not they're in different components or anything. Just that you can ask for flags to load one file, or a bunch of files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is true. But logs are still kind of user facing, so I thought, it might be better to use the terminology we would use in HLS.

Should we uniformly use the hie-bios perspective here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes hie-bios more confusing if we're not consistent. But maybe we're just a bit confused about what hie-bios is saying it will do? Or perhaps we can make the message informative enough to cover both: "Requested to load cradle using all open files (multi-component)" or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good suggestion, updated!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also renamed the LoadStyle constructors to reflect this more accurately.

@fendor fendor force-pushed the enhance/loading-style branch 3 times, most recently from 71b2e17 to 56e19ef Compare April 22, 2024 12:39
Allows users to decide at run-time whether they would like to use
experimental features, such as `cabal`'s `multi-repl` feature that
will be released in 3.12.

The `LoadStyle` can not always be honoured by the respective cradle.
For example, if the ghc version or cabal version isn't recent enough.
@fendor fendor merged commit cdc7fa5 into haskell:master Apr 22, 2024
16 checks passed
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

2 participants