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

Multiple linters won't spawn properly #136

Closed
UnderCooled opened this issue Jan 13, 2024 · 8 comments
Closed

Multiple linters won't spawn properly #136

UnderCooled opened this issue Jan 13, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@UnderCooled
Copy link

I think you should put coroutine in for loop.

My modification in lint.lua:

-- NOTE: From for loop in coroutine to coroutine in for loop
  for i, lint in ipairs(linters) do
    lint = vim.deepcopy(lint)
    lint.args = lint.args or {}
    lint.args[#lint.args + 1] = fname
    lint.lines = prev_lines
    lint.cmd = vim.fn.exepath(lint.cmd)
    -- print("create co for " .. lint.cmd)
    local co = coroutine.create(function()
      local results
      -- print("lint " .. lint.cmd)
      local data = spawn(lint)
      -- print('got data:\n' .. data)
      if #data > 0 then
        results = lint.parse(data, buf)
      end
      -- print('got results')
      vim.schedule(function()
        if not api.nvim_buf_is_valid(buf) or not results or #results == 0 then
          return
        end
        -- FIXME: without changed namespace id, the first linter diag message will be override by the second.
        vd.set(ns + i, buf, results)
      end)
    end)
    coroutine.resume(co)
  end

"Working" example:

image

@xiaoshihou514 xiaoshihou514 changed the title Multiple linters won't get spawn properly. Multiple linters won't spawn properly when using exepath Jan 13, 2024
@xiaoshihou514
Copy link
Collaborator

I am a bit confused, what is this issue about? Could you state the problem first.

@xiaoshihou514 xiaoshihou514 changed the title Multiple linters won't spawn properly when using exepath Multiple linters won't spawn properly Jan 13, 2024
@UnderCooled
Copy link
Author

I add two linters to '*', the second one won't spawn, I'm using those print to find out why the second linter will never get called in the forloop. I don't know why guard.nvim wrap forloop in a big coroutine, shouldn't we create coroutine for uv.spawn, so they all run async? So I moved forloop out of coroutine, wrapped uv.spawn in coroutine, now the second linter get call and return results.

I checked "format.lua", it's in same situation, forloop in coroutine, maybe that's why there is #113 exist.

@xiaoshihou514
Copy link
Collaborator

shouldn't we create coroutine for uv.spawn, so they all run async?

uv.spawn is true async on its own (asnyc exterior process spawning provided by libuv), whereas coroutine is not really "async". Check this out:

print("1")
coroutine.resume(coroutine.create(function()
    os.execute("sleep 5")
    print("2")
end))
print("3")

On executing this would always print 1,2,3 in order, as opposed to 1,3,2

@xiaoshihou514
Copy link
Collaborator

The second linter not spawning is due to, I believe, the broken fix I introduced in #134 , I apologize for it :(

@UnderCooled
Copy link
Author

UnderCooled commented Jan 13, 2024

不好意思,请允许我再用中文问一下。不是专业学编程的,异步只接触过早期 nodejs 那些。

也就是 forloop 包在协程里面,相当与是异步顺序执行,然后我这样改就是异步并行?

如果是这样,那么 format.lua 中的顺序异步执行,有道理,不过 lint 的话,并行然后放在不同命名空间上是不是也行?毕竟两个 linter 之间可能没有顺序问题。

@xiaoshihou514
Copy link
Collaborator

  • The namespace has to stay the same, cause we depend on that to clear diagnostics
  • And yes, linters don't have to have an execution order, not sure if it's the problem tho

@xiaoshihou514 xiaoshihou514 added the bug Something isn't working label Jan 13, 2024
@UnderCooled
Copy link
Author

A diagnostic is a Lua table with the following keys. Required keys are
indicated with (+):

bufnr: Buffer number
lnum(+): The starting line of the diagnostic
end_lnum: The final line of the diagnostic
col(+): The starting column of the diagnostic
end_col: The final column of the diagnostic
severity: The severity of the diagnostic |vim.diagnostic.severity|
message(+): The diagnostic text
source: The source of the diagnostic
code: The diagnostic code
user_data: Arbitrary data plugins or users can add

So we can get rid of "namespace" property in diag_fmt ?

  for _, lint in ipairs(linters) do
    lint = vim.deepcopy(lint)
    lint.args = lint.args or {}
    lint.args[#lint.args + 1] = fname
    lint.lines = prev_lines
    -- print(lint.cmd)
   -- NOTE: make namespace for each linter
    local lns = api.nvim_create_namespace('Guard' .. lint.cmd)
   -- NOTE: reset the specific namespace
    vd.reset(lns, buf)
    lint.cmd = vim.fn.exepath(lint.cmd)
    -- print("create co for " .. lint.cmd)
    local co = coroutine.create(function()
      local results
      -- print("lint " .. lint.cmd)
      local data = spawn(lint)
      -- print('got data:\n' .. data)
      if #data > 0 then
        results = lint.parse(data, buf)
      end
      -- print('got results')
      vim.schedule(function()
        if not api.nvim_buf_is_valid(buf) or not results or #results == 0 then
          return
        end
        -- NOTE: add diags to the specific namespace
        vd.set(lns, buf, results)
      end)
    end)
    coroutine.resume(co)
  end

Checkout -- NOTE: comments above.

@xiaoshihou514
Copy link
Collaborator

xiaoshihou514 commented Jan 13, 2024

Fixed :)
Screenshot from 2024-01-13 11-53-42
Screenshot from 2024-01-13 11-54-05
Screenshot from 2024-01-13 11-54-23

should hopefully work as intended now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants