-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Rework "Configuration" and "Manually testing HLS" documentations #3772
Rework "Configuration" and "Manually testing HLS" documentations #3772
Conversation
398812c
to
ab517d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is great! sorry that the hie-bios configuration reality is a bit of a mess...
docs/configuration.md
Outdated
|
||
*So using a explicit `hie.yaml` file will not likely fix your ide setup*. It will do it almost only if you see an error like `Multi Cradle: No prefixes matched` | ||
See [the hie-bios documentation](https://github.com/haskell/hie-bios#implicit-configuration) for more information on implicit configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is quite accurate. I believe we in fact will use implicit-hie
in this instance, not the hie-bios
implicit auto-configuration...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 739e58f
docs/configuration.md
Outdated
### Explicit, manual configuration | ||
Maybe using the generated `hie.yaml` file does not suit you. | ||
E.g., it still does not work, or you want to fine-tune the configuration. | ||
We recommend to start from the `hie.yaml` file generated by `implicit-hie` (see previous section) and modify it to suit your needs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this recommendation is no longer quite right for cabal projects at least. For cabal projects you can often use the "empty" cabal cradle, which often even works better than the one generated by implicit-hie
. However for stack I believe the implicit-hie
one is better.
This situation is a mess, unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reworked and simplified a bit this section in 63ba56a
Basically rather than trying to re-explain the hie-bios documentation, we directly link to the relevant documentation
the most common stack and cabal configurations | ||
**For a full explanation of how to configure it manually, refer to the [hie-bios documentation](https://github.com/mpickering/hie-bios/blob/master/README.md).** | ||
|
||
#### Examples of explicit `hie-yaml` configurations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should include the "empty cabal cradle" which mostly works for everything now:
cradle:
cabal:
(you might ask "why doesn't implicit-hie generate this?" which is a good question...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I added it in 63ba56a
docs/configuration.md
Outdated
@@ -196,6 +210,12 @@ dependencies: | |||
- someDep | |||
``` | |||
|
|||
### Common problems |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should go in the Troubleshooting page, perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, done in 386f87c
|
||
Note: HLS may implicitly detect codebase as a Stack project (see [hie-bios implicit configuration documentation](https://github.com/haskell/hie-bios/blob/master/README.md#implicit-configuration)). To use Cabal, try creating an `hie.yaml` file: | ||
Note: HLS implicitly detects the HLS codebase as a Stack project (since there is a `stack.yaml` file). | ||
If you want HLS to use Cabal, create this `hie.yaml` file at the root of the project: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Run: | ||
```shell | ||
$ cabal build exe:haskell-language-server && cabal list-bin exe:haskell-language-server | ||
[..] | ||
<some long path>/haskell-language-server | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I am a little bit late to the party, but I feel like using the cabal build
binary for testing can be slightly unexpected.
When you switch branches and run cabal build
just to make sure it compiles, or to speed up HLS to load everything, then you suddenly update your test binary.
Such behaviour might be unexpected to non-expert users, perhaps even expert users. I, for one, definitely prefer cabal install
since I have a couple of local HLS installations lying around.
That's why we previously recommended using cabal install
, updating your tested HLS binary is a very conscious decision. Additionally, it allows you to set program-suffixes and other custom locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguably the opposite argument is as inconvenient for others 😅
cabal install
will install in a global location outside of the project, so it makes it harder to keep things isolated- It might replace the stable binary they had installed in the past, and they might get confused about how to get back to the stable binary
- With the
cabal build
approach, you can "undo" all the steps in the documentation and land back on your original setup. With thecabal install
approach, "undoing" the steps may not be enough to get back to the original setup (because you also need to know how to get back the original binary, which you may have installed long ago, and you did not know which version you had).
I guess different people have different priorities 😅 And it's completely ok by the way!
In my opinion the new version of the documentation is more welcoming towards non-experts (ability to undo), and while some experts may get surprised by the scenario you mentioned, I think they would be able to identify and work around.
In any case it might also be beneficial to add this caveat to the documentation, if you suspect many will get confused by it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, the arguments are also convincing. Ideally, we'd have a command cabal build --copy-to-local-bindir
, that copies the binary to dist-newstyle/bin/
or something similar.
What do you think about instructions such as cabal install --installdir dist-newstyle/bin/
? Arguably, we shouldn't touch dist-newstyle
, but I think that's relatively safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I use cabal build
and it's fine, but it seems reasonable to include both as alternatives?
While working on HLS in the last couple days for #3771 I found it difficult to understand how to:
This PR tries to improve the current documentation.
In my opinion this makes the documentation clearer:
hie.yaml
configurationPlease feel free to make any additional suggestion