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

Rework "Configuration" and "Manually testing HLS" documentations #3772

Conversation

sir4ur0n
Copy link
Collaborator

While working on HLS in the last couple days for #3771 I found it difficult to understand how to:

  • configure HLS
  • manually test HLS when modifying the code

This PR tries to improve the current documentation.

In my opinion this makes the documentation clearer:

  • explicit separation between "implicit vs generated vs manual" hie.yaml configuration
  • remove parts of the "Using HLS on HLS code" section that were not specific to the HLS code at all (it could be applied to any code base) and reuse the "Configuration" documentation
  • explicit separation of the 4 steps to use a custom HLS on any code base. Some steps depend on the build tool (Stack vs Cabal) while others depend on the editor, so I think this separation is more intuitive and exhaustive

Please feel free to make any additional suggestion

@sir4ur0n sir4ur0n force-pushed the rework-configuration-and-manually-testing-hls-documentation branch from 398812c to ab517d3 Compare August 23, 2023 12:42
Copy link
Collaborator

@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.

Thanks, this is great! sorry that the hie-bios configuration reality is a bit of a mess...


*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.
Copy link
Collaborator

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...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 739e58f

### 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.
Copy link
Collaborator

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.

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 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
Copy link
Collaborator

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...)

Copy link
Collaborator Author

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

@@ -196,6 +210,12 @@ dependencies:
- someDep
```

### Common problems
Copy link
Collaborator

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?

Copy link
Collaborator Author

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:
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@michaelpj michaelpj merged commit e516937 into haskell:master Aug 24, 2023
19 checks passed
Comment on lines +133 to +138
Run:
```shell
$ cabal build exe:haskell-language-server && cabal list-bin exe:haskell-language-server
[..]
<some long path>/haskell-language-server
```
Copy link
Collaborator

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.

Copy link
Collaborator Author

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 😅

  1. cabal install will install in a global location outside of the project, so it makes it harder to keep things isolated
  2. 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
  3. With the cabal build approach, you can "undo" all the steps in the documentation and land back on your original setup. With the cabal 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

Copy link
Collaborator

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.

Copy link
Collaborator

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?

@fendor fendor mentioned this pull request Aug 25, 2023
19 tasks
@sir4ur0n sir4ur0n deleted the rework-configuration-and-manually-testing-hls-documentation branch August 27, 2023 09:31
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.

3 participants