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

Options for pylintrc files? #509

Open
mesa123123 opened this issue Jan 17, 2024 · 4 comments
Open

Options for pylintrc files? #509

mesa123123 opened this issue Jan 17, 2024 · 4 comments

Comments

@mesa123123
Copy link

mesa123123 commented Jan 17, 2024

Hey man loving the plugin!

I'm just wondering if pylint can have a rcfile tacked onto its args capabilities?
image
Currently getting this error when I'm implementing my code:
image
Am I doing something wrong or is this just somehting the parser can't do yet?
I think I could add some kind of logic in for this?

Ok tried something different and just have:
image
So no matter what I'm passing { "-f", "json" } to the args pylint
Same error popped up

Tried this too:
image
and pylint didn't load at all?
image
There should be at least a function docstring not found error right?

Next Tried This
image
and pylint didn't seem to load this time either

And lastly this
image
And pylint worked perfectly...
image

However, this seemed to work
image
But then I'm unsure of how to make sure that it always looks for the pylintrc in the project directory?

Because as soon as I went into a child module and ran from there pylint seems to crash again:
image

This is the pylinrc contents btw:
image

@maxspahn
Copy link

Hi @mesa123123 ,

I had the same issue but with the help of chatgpt 😎, I came up with that solution:

function file_exists_in_parent(filename)
    local function search_in_directory(dir)
        local file_path = dir .. "/" .. filename
        local file = io.open(file_path, "r") -- Try to open the file in read mode
        if file then
            file:close() -- Close the file handle
            return true, file_path -- File exists, return its path
        else
            return false -- File doesn't exist in this directory
        end
    end

    local current_dir = os.getenv("PWD") or os.getenv("CD") or "." -- Get the current directory

    repeat
        local found, file_path = search_in_directory(current_dir)
        if found then
            return true, file_path -- File exists in the current directory or its ancestor
        else
            local parent_dir = current_dir:gsub("[^/\\]+$", ""):sub(0, -2)
            if parent_dir and parent_dir ~= current_dir then
                current_dir = parent_dir -- Move to the parent directory
            else
                break -- Reached the root directory
            end
        end
    until false

    return false -- File not found in the parent directory or its ancestors
end

local file_name = ".pylintrc"
local found, file_path = file_exists_in_parent(file_name)
if found then
  print("Found pylintrc", file_path:sub(0, -1))
  local pylint = require('lint').linters.pylint
  pylint.args = {
    '-f', 'json',
    '--rcfile', file_path,
  }
else
  print("No pylintrc file found. Using default config.")
end

That works except for it does not reload the pylintrc file, if you change directory while being in vim using nerdtree for example.

@kasi-x
Copy link

kasi-x commented Mar 13, 2024

Thank you for your approach, @maxspahn. It addresses some problems, but I think a more comprehensive solution is needed.

I see two main issues with the current code.
First, while your code focuses on pylintrc, configurations are also made in files like pyproject.toml, setup.cfg, and tox.ini. It would be better to accommodate these files as well.

https://github.com/jose-elias-alvarez/null-ls.nvim/blob/0010ea927ab7c09ef0ce9bf28c2b573fc302f5a7/lua/null-ls/builtins/diagnostics/pylint.lua#L67-L77

The second issue is whether we should standardize root-search code.
I've been looking into null-ls's code as I attempted to make some adjustments. null-ls has standardized its directory search functions, but this approach comes with drawbacks, such as increased complexity and more challenging maintenance.

https://github.com/jose-elias-alvarez/null-ls.nvim/blob/0010ea927ab7c09ef0ce9bf28c2b573fc302f5a7/lua/null-ls/utils/init.lua#L215-L236

For nvim-lint, if the directory search problem was unique to pylint, perhaps there wouldn't be a need to place such functions in utils. However, the issue of directory searching extends beyond pylint. For example, codespell also struggles with reading configuration files.

If we're going to reuse code for directory searches, adopting a standardized approach could be beneficial. I'm not a maintainer of nvim-lint and am still trying to understand the best direction for its design. I'm looking for input from others on this matter.

@mfussenegger
Copy link
Owner

I'm not going to merge any complex root detection logic.
The only thing I'd accept is a simple vim.fs.find, nothing more than that.

Most linters pick up config files in the current working directory per default, and don't require any custom logic at all.
I thought that's also the case for pylint, which means you have to make sure that nvim has the right working directory set, or you call try_lint with the cwd option set.

Root dir and projection management heuristics are out of scope of nvim-lint.

@maxspahn
Copy link

Thanks at @kasi-x

First, while your code focuses on pylintrc, configurations are also made in files like pyproject.toml, setup.cfg, and tox.ini. It would be better to accommodate these files as well.

Totally agree, but passing a list of potential config files, rather than a single file as i do it should not be too complicated.

If we're going to reuse code for directory searches, adopting a standardized approach could be beneficial. I'm not a maintainer of nvim-lint and am still trying to understand the best direction for its design. I'm looking for input from others on this matter.

I agree that a standardized approach would be helpful for configuration file search, and to be fully honest, i suspected there were tools for that already, that I was just not aware of. Therefore, this rather clumsy approach.

@mfussenegger

Root dir and projection management heuristics are out of scope of nvim-lint.

Makes sense. For some people, this root-finding heuristic that you add on top of nvim-lint may just be nice to come across in the issues.

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

No branches or pull requests

4 participants