Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Add default configuration for Haskell LSP #1918

Merged
merged 1 commit into from
Mar 27, 2018

Conversation

Tehnix
Copy link
Contributor

@Tehnix Tehnix commented Mar 24, 2018

This adds a default configuration for the Haskell LSP, HIE.

@akinsho
Copy link
Member

akinsho commented Mar 25, 2018

@Tehnix, thanks for the PR 👍, Although i'm wondering whether this would go better in the documentation wiki re LSPs we have tended to put information re how to integrate new LSPs here, the reasoning behind this as @bryphe has pointed out is that the more lsps Oni ships with the bulkier the application, I think having it in the wiki like for golang, c++ and c-sharp would be really great though with some installation instructions?

@Tehnix
Copy link
Contributor Author

Tehnix commented Mar 25, 2018

That would also make sense, yeah :) I'll add it to the wiki page instead, and close this PR then.

@Tehnix Tehnix closed this Mar 25, 2018
@Tehnix Tehnix reopened this Mar 25, 2018
@codecov
Copy link

codecov bot commented Mar 25, 2018

Codecov Report

Merging #1918 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1918   +/-   ##
=======================================
  Coverage   35.17%   35.17%           
=======================================
  Files         262      262           
  Lines       10347    10347           
  Branches     1322     1322           
=======================================
  Hits         3640     3640           
  Misses       6522     6522           
  Partials      185      185
Impacted Files Coverage Δ
...src/Services/Configuration/DefaultConfiguration.ts 81.25% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39cdafc...69d65f6. Read the comment docs.

@Tehnix
Copy link
Contributor Author

Tehnix commented Mar 25, 2018

Perhaps a bit too quick on that one. I saw that pyls the python LSP is configured in the DefaultConfiguration, but still requires one to install it yourself.

Perhaps a mix of adding the configuration here, and then install instructions in the wiki entry?

Let me know what you think.

@CrossR
Copy link
Member

CrossR commented Mar 25, 2018

We've got pyls and clangd defined in the code already, so I don't see any issues with adding another?

If we were bundling the executables that would be a bit different, but I think just including the config pre-entered seems fine to me.

@akinsho
Copy link
Member

akinsho commented Mar 25, 2018

tbh atm its a few lines and having it means a haskell user who has the lsp downloaded will have everything work automagically which is great.

Not sure what the long term plan is though, aka going forward will we have defaults for most user languages in which case if it isn't clear that were setting specific defaults then it could raise issues if our defaults clash with what a user ends up setting e.g. a user sets 2/4 of the lsp config opts since they want to use a (different) lsp for a language than our default then don't realise we've set another two defaults to something that's incompatible with their choice 🤷‍♂️.

I'm happy with this merged into the default config or as documentation will defer to whatever the prevailing preference is.

@akinsho akinsho merged commit 26d015c into onivim:master Mar 27, 2018
@bryphe
Copy link
Member

bryphe commented Mar 27, 2018

💯 , thanks for putting this together @Tehnix , and thanks for merging it in @Akin909 !

I didn't realize there is a haskell language server... I just read a book on haskell, I'll have to try this out 👍 Very cool!

Not sure what the long term plan is though, aka going forward will we have defaults for most user languages in which case if it isn't clear that were setting specific defaults then it could raise issues if our defaults clash with what a user ends up setting

Long-term, once our plugin story is more mature, it'd be cool if we had language-specific plugins. It'd be great if they could bundle the language servers as well (like VSCode's plugins). It's a nice experience if you just add one plugin with minimal configuration and you're all set 👍

Shorter-term, I think this approach is great - especially for cases like this where there is one primary language server. One thing I'd like to add to make this a bit nicer is to have some sort of message / URL that we can point a user to if the language server fails to start (for example, if the user doesn't have it installed). It'd be cool if we could put up a notification like "I see you're opening a Haskell file - Oni works great with the haskell language server. Click here to learn more.". Potentially we could add configuration settings like language.<language-identifier>.languageServer.helpText and language.<language-identifier>.languageServer.helpUrl. Then, if a language server fails to launch, we look at the config and see if those are available, and if so, pop up a notification.

They don't necessarily have to be mutually exclusive strategies either, I think we can grow into the plugin based approach once the infrastructure is there 👍

@Tehnix
Copy link
Contributor Author

Tehnix commented Jun 5, 2018

I'm fairly certain HIE won't work without a project file, i.e. a .cabal file (and possibly a stack.yaml). Are you able to get HIE working with other editors (e.g. VSCode) in the same project?

@akinsho
Copy link
Member

akinsho commented Jun 5, 2018

@Tehnix thanks for fielding that I dont know HIE at all, @Preconf what I can say real quick re the csproj error you're seeing is its a hang over from the initial implementation and the error message is actually from the function which tries to identify the project root I have a PR which should fix how that function works so itll be less confusing/will work better

@Tehnix
Copy link
Contributor Author

Tehnix commented Jun 5, 2018

@Akin909 No worries, it's a pretty HIE specific question :)

@Preconf Heh, I feel a bit bad that we don't have any "setup" or "getting started" in the HIE repository. For the best experience, I recommend the following:

  • Use stack in general for managing projects
  • Build haskell-ide-engine with make build-copy-compiler-tool
  • HIE should only work in projects, you can maybe do stack new HelloWorld simple
  • Set your Oni command to "language.haskell.languageServer.command": "stack exec -- hie"
  • Set your oni root files to "language.haskell.languageServer.rootFiles": [".git", "stack.yaml", "*.cabal"]

Should get you going in the right direction :) The reason for me recommending make build-copy-compiler-tool, is that the GHC version you built HIE with needs to match the GHC version in your project, else you'll get some strange errors (if it even works). You can check with hie --compiler.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants