Skip to content
This repository was archived by the owner on Aug 12, 2023. It is now read-only.

fix(builtins): set correct golangci_lint cwd#1206

Open
marcelbeumer wants to merge 1 commit intojose-elias-alvarez:mainfrom
marcelbeumer:fix-golangci-lint
Open

fix(builtins): set correct golangci_lint cwd#1206
marcelbeumer wants to merge 1 commit intojose-elias-alvarez:mainfrom
marcelbeumer:fix-golangci-lint

Conversation

@marcelbeumer
Copy link
Copy Markdown
Contributor

@marcelbeumer marcelbeumer commented Oct 23, 2022

I discovered this issue working on https://github.com/marcelbeumer/go-playground/tree/main/gochat (with cwd being gochat).

golintci_lint was running from the $ROOT, which, if I understand correctly, is set the the dir containing .git, by default. I think it should run in a root appropriate for the file being saved (in this case the dir gochat containing go.mod).

I imagine it's a standard pattern to "find the correct root" for the current buffer, so if there's a better way to do this (using null-ls helpers/utils for example) let me know!

if type(issues) == "table" then
for _, d in ipairs(issues) do
if d.Pos.Filename == params.bufname then
local fname = params.cwd .. "/" .. d.Pos.Filename
Copy link
Copy Markdown
Contributor Author

@marcelbeumer marcelbeumer Oct 23, 2022

Choose a reason for hiding this comment

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

doing this here instead of using --path-prefix so it's easier for consumers to modify the args with .with({ args (or is there a way to retrieve (resolved) .cwd from .with(?)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Was this not working, or did the other change make this one necessary? Either way I think this is fine, though we'll want to use the join_path utility (example) to guarantee consistency.

to_stdin = true,
from_stderr = false,
ignore_stderr = true,
cwd = function(params)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's not a huge deal, since this linter runs (relatively) infrequently, but if we don't expect this result to change, I think we want to cache this to avoid repeated lookups. You may also want to consider using the root_pattern utility. This builtin has an example of both.

if type(issues) == "table" then
for _, d in ipairs(issues) do
if d.Pos.Filename == params.bufname then
local fname = params.cwd .. "/" .. d.Pos.Filename
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Was this not working, or did the other change make this one necessary? Either way I think this is fine, though we'll want to use the join_path utility (example) to guarantee consistency.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants