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 support for purescript language server #2572

Merged
merged 3 commits into from
Jun 17, 2019
Merged

Add support for purescript language server #2572

merged 3 commits into from
Jun 17, 2019

Conversation

drewolson
Copy link
Contributor

Adds support for the purescript language server. Based off of this old, now stale PR.

This seems to work for me locally, though I'm a bit fuzzy on if this is the correct way to implement config for the language server protocol.

Thanks!

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Add tests for the code here. See :help ale-dev and look at tests for other linters.

@@ -338,6 +338,8 @@ Notes:
* `languageserver`
* `puppet`
* `puppet-lint`
* Purescript
* `purels`
Copy link
Member

Choose a reason for hiding this comment

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

Name this and the heading purescript-language-server. Update the table of contents in ale.txt for the options for linters. Update the Markdown list of supported tools.

\ 'command': function('ale_linters#purescript#purels#GetCommand'),
\ 'project_root': function('ale_linters#purescript#purels#FindProjectRoot'),
\ 'lsp_config': {b -> ale#Var(b, 'purescript_purels_config')},
\ 'language': 'purescript',
Copy link
Member

Choose a reason for hiding this comment

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

You can remove the language key, it's redundant.


call ale#Set('purescript_purels_executable', 'purescript-language-server')
call ale#Set('purescript_purels_use_global', get(g:, 'ale_use_global_executables', 0))
call ale#Set('purescript_purels_config', {})
Copy link
Member

Choose a reason for hiding this comment

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

Name the file and the variables ls instead of purels. That will more closely match how other files are named.

endfunction

call ale#linter#Define('purescript', {
\ 'name': 'purels',
Copy link
Member

Choose a reason for hiding this comment

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

Name the linter here purescript-language-server.

@drewolson
Copy link
Contributor Author

drewolson commented Jun 10, 2019

Thanks for the feedback, I’ll work on updating the PR in the next few days.

@drewolson
Copy link
Contributor Author

@w0rp I think I've address all of your naming concerns, but travis is failing with errors about my help files being incorrect. I don't know much about correctly formatting vim help files. They appear aligned to my eye, but perhaps I'm missing something.

I'll look into adding tests tomorrow.

w0rp
w0rp previously approved these changes Jun 11, 2019
@drewolson
Copy link
Contributor Author

@w0rp apologies, a force push negated your approval.

I've added tests and gotten the build passing. I think this should be good to merge now.

Thanks for the feedback and help.

@drewolson
Copy link
Contributor Author

Please let me know if there's anything else to be done here. Thanks!

@w0rp w0rp merged commit 1c71da5 into dense-analysis:master Jun 17, 2019
@w0rp
Copy link
Member

w0rp commented Jun 17, 2019

Cheers! 🍻

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.

2 participants