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.

Sign up for GitHub

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

fix: add pylyzer dependenncy info (erg) (#3433) #3441

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: add pylyzer dependenncy info (erg) (#3433) #3441

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f992df
fix: add pylyzer dependenncy info (erg) (#3433)
fishBone000 Nov 18, 2024
b0f382f
fix: update pylyzer default cfg as per PR comment
fishBone000 Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lua/lspconfig/configs/pylyzer.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,25 @@ return {
checkOnType = false,
},
},
ergPath = vim.fn.getenv('HOME') .. '/.erg',
on_new_config = function(new_config, _)
if new_config.ergPath then
new_config.cmd_env.ERG_PATH = new_config.ergPath
end
end,
Copy link
Member

Choose a reason for hiding this comment

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

This saves the user basically nothing, while also adding a special-case interface to this config.

It would be more useful to just document what you're doing here, instead of adding a special ergPath field.

Copy link
Author

@fishBone000 fishBone000 Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I thought the cmd_env is gonna work? I found it in bqnlsp's default config.

I think it would still be nice if I set the ERG_PATH in default config, so users who aren't familiar with lsp config can use it out of box (they still need to extract tarball to the path ofc):

    on_new_config = function(new_config, _)
      vim.fn.setenv("ERG_PATH", vim.fn.getenv("HOME") .. "/.erg")
    end

Is this OK?

Copy link
Member

Choose a reason for hiding this comment

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

cmd_env is a standard field, no objection to using that. My objection is about the custom ergPath thing.

still be nice if I set the ERG_PATH in default config, so users who aren't familiar with lsp config can use it out of box

Sure, yes. And some sort of brief hint in the docs.

},
docs = {
description = [[
https://github.com/mtshiba/pylyzer

`pylyzer`, a fast static code analyzer & language server for Python.

`pylyzer` requires Erg as dependency, and finds it via `ERG_PATH` environment variable.
By default `ERG_PATH` is set to `~/.erg` when `lspconfig` runs `pylyzer`,
pass `ergPath = "/path/to/erg"` if you want to change it.

To install Erg, simply extract tarball/zip from [Erg releases](https://github.com/erg-lang/erg/releases/latest)
to the the path where you want to install it, e.g. `~/.erg`.
]],
},
}
Loading