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

Health check subcommand doesn't display multiple language servers #8156

Closed
EpocSquadron opened this issue Sep 3, 2023 · 3 comments · Fixed by #7315
Closed

Health check subcommand doesn't display multiple language servers #8156

EpocSquadron opened this issue Sep 3, 2023 · 3 comments · Fixed by #7315
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@EpocSquadron
Copy link
Contributor

Problem Statement

I have PHP configured with multiple language servers as follows:

[[language]]
name="php"
language-servers=["psalm", "intelephense"]

[language-server.psalm]
command="psalm"
args=["--language-server"]

When I run hx --health php only the first configured language sever shows up in the checks:

$ hx --health php
Configured language server: psalm
Binary for language server: /usr/bin/psalm
Configured debug adapter: None
Highlight queries: ✓
Textobject queries: ✓
Indent queries: ✓

Possible Solutions

We should print all of the language servers configured and whether they are picking up the binary. This is more challenging when not filtering to a specific language, as we have the table view to deal with and probably don't want to display a pair of columns for every configured language server. This would be especially painful for things like Svelte, where there can be quite a few language servers defined for eslint, tailwind, svelte, typescript, and possibly others.

It might be best if we instead stacked them within the columns like so:

$ hx --health
Language                  LSP                       DAP                       Highlight                 Textobject                Indent                   
php                       ✓ psalm                   None                      ✓                         ✓                         ✓      
                          ✓ intelephense                   
rust                      ✓ rust-analyzer           ✘ lldb-vscode             ✓                         ✓                         ✓               

Github won't display them for me, but we could use box drawing characters to visually group the language servers for a single language to make the output easier to scan visually.

For output for a single language we could do like so:

$ hx --health php
Configured language servers and binary path: 
    ✓ psalm        /usr/bin/psalm
    ✓ intelephense /home/epocsquadron/.node_modules/bin/intelephense
Configured debug adapter: None
Highlight queries: ✓
Textobject queries: ✓
Indent queries: ✓
@kirawi
Copy link
Member

kirawi commented Sep 3, 2023

#7315

TheRealLorenz added a commit to TheRealLorenz/helix that referenced this issue Sep 3, 2023
Better output (inspired by helix-editor#8156).

Handle the case where no LSPs are configured.
@kirawi kirawi added C-enhancement Category: Improvements A-helix-term Area: Helix term improvements labels Sep 9, 2023
@mabasic
Copy link

mabasic commented Oct 14, 2023

Off topic, I have just tried psalm lsp for php and it is not usable for me. I got it working but whatever I do it does not support. Can't go to definition, no hover documentation, no documents symbols, ... is there anything that is working in psalm lsp?

It has the potential to be a great lsp for php, but seems broken atm.

@kirawi
Copy link
Member

kirawi commented Oct 14, 2023

Can you create a separate discussion thread for that? And in it, include what :log-open says after running hx -v and opening a PHP project.

@the-mikedavis the-mikedavis linked a pull request Oct 16, 2023 that will close this issue
pascalkuthe pushed a commit that referenced this issue Oct 16, 2023
* Add support for showing all LSPs in --health <lang>

* Add support for showing all LSPs in --health languages

* Use available/configured in --health languages

* Apply @AlexanderBrevig suggestion in --health

* Update `--health <language>`

Better output (inspired by #8156).

Handle the case where no LSPs are configured.

* Display all LSPs in `--health languages` instead of x/x

Displays all LSPs as a list in the table generated wih `--health languages`

* Make check_binary accept Optional references to str

Avoids some calls to .clone()

* Apply @the-mikedavis suggestions

* Avoid useless collecting and cloning

* Use for loop instead of .try_for_each()
danillos pushed a commit to danillos/helix that referenced this issue Nov 21, 2023
* Add support for showing all LSPs in --health <lang>

* Add support for showing all LSPs in --health languages

* Use available/configured in --health languages

* Apply @AlexanderBrevig suggestion in --health

* Update `--health <language>`

Better output (inspired by helix-editor#8156).

Handle the case where no LSPs are configured.

* Display all LSPs in `--health languages` instead of x/x

Displays all LSPs as a list in the table generated wih `--health languages`

* Make check_binary accept Optional references to str

Avoids some calls to .clone()

* Apply @the-mikedavis suggestions

* Avoid useless collecting and cloning

* Use for loop instead of .try_for_each()
dgkf pushed a commit to dgkf/helix that referenced this issue Jan 30, 2024
* Add support for showing all LSPs in --health <lang>

* Add support for showing all LSPs in --health languages

* Use available/configured in --health languages

* Apply @AlexanderBrevig suggestion in --health

* Update `--health <language>`

Better output (inspired by helix-editor#8156).

Handle the case where no LSPs are configured.

* Display all LSPs in `--health languages` instead of x/x

Displays all LSPs as a list in the table generated wih `--health languages`

* Make check_binary accept Optional references to str

Avoids some calls to .clone()

* Apply @the-mikedavis suggestions

* Avoid useless collecting and cloning

* Use for loop instead of .try_for_each()
mtoohey31 pushed a commit to mtoohey31/helix that referenced this issue Jun 2, 2024
* Add support for showing all LSPs in --health <lang>

* Add support for showing all LSPs in --health languages

* Use available/configured in --health languages

* Apply @AlexanderBrevig suggestion in --health

* Update `--health <language>`

Better output (inspired by helix-editor#8156).

Handle the case where no LSPs are configured.

* Display all LSPs in `--health languages` instead of x/x

Displays all LSPs as a list in the table generated wih `--health languages`

* Make check_binary accept Optional references to str

Avoids some calls to .clone()

* Apply @the-mikedavis suggestions

* Avoid useless collecting and cloning

* Use for loop instead of .try_for_each()
smortime pushed a commit to smortime/helix that referenced this issue Jul 10, 2024
* Add support for showing all LSPs in --health <lang>

* Add support for showing all LSPs in --health languages

* Use available/configured in --health languages

* Apply @AlexanderBrevig suggestion in --health

* Update `--health <language>`

Better output (inspired by helix-editor#8156).

Handle the case where no LSPs are configured.

* Display all LSPs in `--health languages` instead of x/x

Displays all LSPs as a list in the table generated wih `--health languages`

* Make check_binary accept Optional references to str

Avoids some calls to .clone()

* Apply @the-mikedavis suggestions

* Avoid useless collecting and cloning

* Use for loop instead of .try_for_each()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants