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

feat: allow to set cwd #179

Merged
merged 1 commit into from
Aug 24, 2022
Merged

feat: allow to set cwd #179

merged 1 commit into from
Aug 24, 2022

Conversation

sileht
Copy link
Contributor

@sileht sileht commented Feb 4, 2022

Fixes #178

@sileht sileht marked this pull request as draft February 4, 2022 23:23
@sileht sileht force-pushed the master branch 2 times, most recently from 9bd6662 to da471d6 Compare February 4, 2022 23:47
@mfussenegger
Copy link
Owner

Is this a use-case that you want to set the cwd per linter? I doubt there is a sensible cwd default other than vim.fn.getcwd, so users who want to change the working directory would then always have to modify all the linters they use?

How about adding an opts arguments to try_lint with a cwd property?
Not sure I like how impactful this is.

@sileht
Copy link
Contributor Author

sileht commented Feb 8, 2022

Is this a use-case that you want to set the cwd per linter?

I have no idea, I guess no.

I doubt there is a sensible cwd default other than vim.fn.getcwd, so users who want to change the working directory would then always have to modify all the linters they use?

In my previous vim experience, when I have used null-ls, ALE, (any older plugins I used)... the default was always the "project root directory" if found or buffer cwd if not. This has always worked out of the box for me.

How about adding an opts arguments to try_lint with a cwd property? Not sure I like how impactful this is.

That should work for me.

@sileht sileht changed the title feat: allow to set linter cwd feat: allow to set cwd Feb 9, 2022
@sileht
Copy link
Contributor Author

sileht commented Feb 9, 2022

I updated the PR to pass the option to try_lint()

@mfussenegger
Copy link
Owner

In my previous vim experience, when I have used null-ls, ALE, (any older plugins I used)... the default was always the "project root directory" if found or buffer cwd if not. This has always worked out of the box for me.

neovim doesn't have the concept of a "project root directory".

There is actually work going on that might result in some kind of project module that would introduce such a concept. That's also the reason why I'm holding off on this for now.

@sileht
Copy link
Contributor Author

sileht commented Feb 10, 2022

In my previous vim experience, when I have used null-ls, ALE, (any older plugins I used)... the default was always the "project root directory" if found or buffer cwd if not. This has always worked out of the box for me.

neovim doesn't have the concept of a "project root directory".

There is actually work going on that might result in some kind of project module that would introduce such a concept. That's also the reason why I'm holding off on this for now.

Exactly, that is why I just introduced the configuration of cwd, I don't think a tool like nvim-lint should implement any "project root directory" lookup. Personally, I used a lspconfig helper to do that.

To give you an idea of my current setup, I have:

autocmd BufWritePost <buffer> lua require("post_write_tools").run_linter()

And run_linter() does something like this:

function M.run_linter()
  local lspconfig = require("lspconfig")
  local rootdir = lspconfig.util.root_pattern(".git")(vim.api.nvim_buf_get_name(0))
  local env = {PATH = rootdir .. "/.tox/pep8/bin/:" .. rootdir .. "/node_modules/.bin/:" .. vim.fn.getenv("PATH"}
  require("lint.linters.flake8").env = env
  require("lint.linters.mypy").env = env
  require("lint.linters.eslint").env = env
  require("lint").try_lint(nil, {cwd = rootdir})
end

@juanramirezc2
Copy link

I work with a monorepo structure using eslint and nested .eslintrc this will be pretty useful.

right now eslint only works if I open vim from one of the subfolders.
When i open vim from the root it doesn't pick the nested config 😞

@juanramirezc2
Copy link

Vscode has a working directories option microsoft/vscode-eslint#1170

@sileht sileht marked this pull request as ready for review June 24, 2022 06:44
@mfussenegger
Copy link
Owner

@sileht would you be up for rebasing this? Sorry that I let this slide for so long. I wanted to wait out a discussion around some project system on neovim core (that it didn't happen), and then got sidetracked with plenty of other stuff.

I think in general adding the opts with the cwd property is good

@sileht sileht force-pushed the master branch 2 times, most recently from fd26a2a to 8ff878d Compare August 24, 2022 16:38
@sileht
Copy link
Contributor Author

sileht commented Aug 24, 2022

@mfussenegger done.

lua/lint/parser.lua Outdated Show resolved Hide resolved
@mfussenegger
Copy link
Owner

Wow that was quick. Left a minor suggestion, otherwise this seems good to go.

@sileht
Copy link
Contributor Author

sileht commented Aug 24, 2022

Wow that was quick. Left a minor suggestion, otherwise this seems good to go.

I use it every day, so it was almost up to date

@mfussenegger mfussenegger merged commit f92c6e1 into mfussenegger:master Aug 24, 2022
@gegoune gegoune mentioned this pull request Aug 8, 2023
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.

Allow to set the CWD on the linter
3 participants