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

Plugin breaks with function in table #8

Open
TabulateJarl8 opened this issue Jul 11, 2024 · 6 comments
Open

Plugin breaks with function in table #8

TabulateJarl8 opened this issue Jul 11, 2024 · 6 comments
Labels
good first issue Good for newcomers

Comments

@TabulateJarl8
Copy link

Hey! I started using your plugin today and it works great, however I believe I've discovered a bug. I'm not sure if this is something you're actually able to fix on your side, but here's an example snipped from my configuration for conform which triggers the error:

local prettier = function(bufnr)
	if require("conform").get_formatter_info("prettier", bufnr).available then
		return { "prettier" }
	else
		return {}
	end
end

require("conform").setup({
	formatters_by_ft = {
		javascript = prettier,
		typescript = prettier,
		vue = prettier,
	},
})

It throws this error on startup:

Error detected while processing BufEnter Autocommands for "*":
Error executing vim.schedule lua callback: ...ed/mason-conform.nvim/lua/mason-conform/auto_install.lua:10: bad argument #1 to 'pairs' (table expected, got function)
stack traceback:
        [C]: in function 'pairs'
        ...ed/mason-conform.nvim/lua/mason-conform/auto_install.lua:10: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>
        [C]: in function 'wait'
        ...oload/plugged/nvim-tree.lua/lua/nvim-tree/git/runner.lua:171: in function '_wait'
        ...oload/plugged/nvim-tree.lua/lua/nvim-tree/git/runner.lua:221: in function 'run'
        ...utoload/plugged/nvim-tree.lua/lua/nvim-tree/git/init.lua:243: in function 'load_project_status'
        ...ad/plugged/nvim-tree.lua/lua/nvim-tree/explorer/init.lua:49: in function '_load'
        ...ad/plugged/nvim-tree.lua/lua/nvim-tree/explorer/init.lua:41: in function 'new'
        ...im/autoload/plugged/nvim-tree.lua/lua/nvim-tree/core.lua:20: in function 'init'
        .../nvim-tree.lua/lua/nvim-tree/actions/root/change-dir.lua:88: in function 'f'
        .../nvim-tree.lua/lua/nvim-tree/actions/root/change-dir.lua:77: in function 'force_dirchange'
        ...ig/nvim/autoload/plugged/nvim-tree.lua/lua/nvim-tree.lua:117: in function <...ig/nvim/autoload/plugged/nvim-tree.lua/lua/nvim-tree.lua:105>

If i replace the function in the table with just {"prettier"} the issue goes away. Also, if I call the function like prettier() instead of just putting the function object in the table, I believe the issue also goes away. Now that I'm using this plugin, I don't really need to worry about needing this specific function, but it may come up in the future for others so I figured I'd document it here at least. Thanks for your work, and let me know if you need any more info

@zapling
Copy link
Owner

zapling commented Jul 11, 2024

Thanks for the report 😄

I'm not sure if this is something you're actually able to fix on your side

I did not know that you could provide a function to conform in this manner.

Now that I'm using this plugin, I don't really need to worry about needing this specific function

Yea that is my take on this issue as well. Configuring a function like this with conform does not really make sense in the context of this plugin, at least not to me.

I'd say this is very low on my priority list right now.

@marty-oehme
Copy link

I have stumbled upon this as well when selecting one of multiple formatters, followed by running more formatters.

With the release of 7.0.0, conform.nvim switched to a new configuration format which deprecates the nested table configuration style for running one formatter out of a pool and subsequently another formatter:

require("conform").setup({
	formatters_by_ft = {
		markdown = { { "prettier", "prettierd" }, "cbfmt" },
	},
})

This produces a warning with the new version.
The manual instead recommends the following code recipe:

---@param bufnr integer
---@param ... string
---@return string
local function first(bufnr, ...)
	local conform = require("conform")
	for i = 1, select("#", ...) do
		local formatter = select(i, ...)
		if conform.get_formatter_info(formatter, bufnr).available then
			return formatter
		end
	end
	return select(1, ...)
end

require("conform").setup({
	formatters_by_ft = {
		markdown = function(bufnr)
			return { first(bufnr, "prettierd", "prettier"), "cbfmt" }
		end,
	},
})

This seems to pose the same issue again.

I agree that it's not a priority issue but with the documentation recommending this way of configuration I think more reports are going to appear which stumble upon the same error.

@zapling
Copy link
Owner

zapling commented Aug 13, 2024

With the release of 7.0.0, conform.nvim switched to a new configuration format

Thanks for the heads up, we should definitely support this going forward then.

@lukoshkin
Copy link

One way to fix this is to make amends to both the plugins conform.nvim and mason-conform.nvim

  • In conform.nvim:

    ---@param bufnr integer | nil
    ---@param ... string
    ---@return string | table
    local function first(bufnr, ...)
      if bufnr == nil then
        return { ... }
      end
    
      for i = 1, select("#", ...) do
        local formatter = select(i, ...)
        if conform.get_formatter_info(formatter, bufnr).available then
          return formatter
        end
      end
      return select(1, ...)
    end

    Suppose you have somewhere in settings:

        json = function(bufnr)
          return { first(bufnr, "prettierd", "prettier"), "fixjson" }
        end,
  • In mason-conform.nvim in lua/mason-conform/auto_install.lua, adjust auto_install function:

    local function auto_install()
      local config = require("mason-conform").config
      local formatters_by_ft = require("conform").formatters_by_ft
    
      local formatters_to_install = {}
      for _, formatters in pairs(formatters_by_ft) do
    +    if type(formatters) == "function" then
    +      formatters = formatters(nil)
    +    end
        for _, formatter in pairs(formatters) do
          -- Support case where the user has defined multiple formatters
          -- for said filetype. E.g javascript = { { "prettierd", "prettier" } }
          if type(formatter) == "table" then
            for _, f in pairs(formatter) do
              formatters_to_install[f] = 1
            end
          else
            formatters_to_install[formatter] = 1
          end
        end
      end
    
      -- Filter out formatters that the user wants to ignore
      for _, formatter_to_ignore in pairs(config.ignore_install) do
        formatters_to_install[formatter_to_ignore] = nil
      end
    
      for conformFormatter, _ in pairs(formatters_to_install) do
        local package = mapping.conform_to_package[conformFormatter]
        if package ~= nil then
          require("mason-conform.install").try_install(package)
        end
      end
    end

@lukoshkin
Copy link

@zapling, maybe we can allow passing functions in the formatters_by_ft section? At least in a way, it is demonstrated above, and document it in the README for the rest of folks?

@zapling
Copy link
Owner

zapling commented Oct 13, 2024

The recipe from mason-conform to Run the first available formatter followed by more formatters posses a problem for this plugin.

We need to get all possible formatters at run-time, in order for us to download them via mason. Adding a function to the return list (in this case first(bufnr)) makes it so we do not get all the formatters, and are therefor unable to download them all.

We could support a function, but we probably do not have a bufnr that we can pass in, so the users first(bufnr) function needs to handle that, maybe returning the full list if nil?

Is that a solution that could work?

@zapling zapling added the good first issue Good for newcomers label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants