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

Recommended way to override parts of a formatter #9

Closed
WhoIsSethDaniel opened this issue Aug 29, 2023 · 10 comments
Closed

Recommended way to override parts of a formatter #9

WhoIsSethDaniel opened this issue Aug 29, 2023 · 10 comments

Comments

@WhoIsSethDaniel
Copy link
Contributor

I've been fiddling with conform.nvim and I wonder if there is a recommended way to do the following:
Let's say there are 2 directories a and b. In a is a configuration file for prettier. In b there is no configuration for prettier. When I am editing a buffer that is within a I would like prettier to use the configuration file in a and change directory to there -- while also using the current definition for prettier for all other fields (plus I'd probably just assume use the same arguments as are in the defaults, but just prepending --config=). If I'm in b (or really anywhere else) I would like the defaults. I have been unable to come up with a satisfactory way to do this. What is recommended?

Have you considered opening up Discussions for this project? This question seems more appropriate to there.

@stevearc
Copy link
Owner

use the configuration file in a and change directory to there

I'm a little confused by this because prettier should already run the command from the root directory that contains a config file

cwd = util.root_file({
-- https://prettier.io/docs/en/configuration.html
".prettierrc",
".prettierrc.json",
".prettierrc.yml",
".prettierrc.yaml",
".prettierrc.json5",
".prettierrc.js",
".prettierrc.cjs",
".prettierrc.toml",
"prettier.config.js",
"prettier.config.cjs",
"package.json",
}),

And prettier should automatically pick up that config file with no commandline arguments required.

Are you not seeing this behavior, in which case there is a bug I need to fix, or is your situation somehow different (e.g. nonstandard config file name)? Happy to provide some sample code on the best way to configure the formatter in different directories, but knowing more about the exact scenario will help.

I've tried opening up Discussions in a few of my other plugins, but I found that some people would report bugs there, bypassing the bug report issue template and forcing me to do the whole "what's your system, what are your logs, do you have a repro". I'd rather just use issues, and I'm happy to take questions like this in an issue!

@WhoIsSethDaniel
Copy link
Contributor Author

In this particular case it is a non-standard config file. I don't want to get bogged down in the particulars of a specific formatter though. I have similar, but not necessarily the same, problems with other formatters.

An easy one was shfmt. For me I always need it to have the same arguments. Those arguments are completely different from the default arguments provided by conform. So I just do this:

local shfmt = require 'conform.formatters.shfmt'
shfmt.args = function()
  return { '-i=4', '-ci', '-s', '-bn' }
end

Similar to nvim-lint.

I'm sorry to hear about the discussions. I like that feature but understand your issues with it.

Also, thank you for oil.nvim. I love it and use it every day.

@stevearc
Copy link
Owner

I think probably the thing you're missing (because it's not properly documented) is that the functions have a context parameter that you can use. So for example, to set the arguments based on whether your custom config file is present:

require("conform.formatters.prettier").args = function(ctx)
  local args = { "--stdin-filepath", "$FILENAME" }
  local found = vim.fs.find(".custom-config.json", { upward = true, path = ctx.dirname })[1]
  if found then
    vim.list_extend(args, { "--config", found })
  end
  return args
end

For changing the working directory, you can do that by modifying the cwd property

require("conform.formatters.prettier").cwd = require("conform.util").root_file({
  ".custom-config.json",
  -- These are the builtins
  ".prettierrc",
  ".prettierrc.json",
  ".prettierrc.yml",
  ".prettierrc.yaml",
  ".prettierrc.json5",
  ".prettierrc.js",
  ".prettierrc.cjs",
  ".prettierrc.toml",
  "prettier.config.js",
  "prettier.config.cjs",
  "package.json",
})

There is another approach as well: I just made a change so that the buffer number will be passed in to the config function, if your config is defined as a function. This allows something like this:

require("conform").formatters.prettier = function(bufnr)
  local bufname = vim.api.nvim_buf_get_name(bufnr)
  local config = vim.fs.find(".custom-config.json", { upward = true, path = bufname })[1]
  local default = require("conform.formatters.prettier")
  if config then
    return vim.tbl_deep_extend("force", default, {
      args = { "--stdin-filepath", "$FILENAME", "--config", config },
      cwd = function()
        return vim.fs.dirname(config)
      end,
    })
  end
end

Glad to hear you like oil.nvim!

@WhoIsSethDaniel
Copy link
Contributor Author

WhoIsSethDaniel commented Aug 30, 2023

Is there a way to do it where I don't have to copy the current args from the formatter defaults? That seems error prone since those could change. I've tried mutating args but I always end up changing the value of the default ... so the next time what I prepended is doubled, and then tripled, so on.

Your examples look a lot like my attempts...except I don't want to have to copy the args. Maybe that can't be done. Or shouldn't be done.

@stevearc
Copy link
Owner

stevearc commented Aug 30, 2023

I would argue that it's actually not as error prone as you would think. The only times when I would expect the args to update is if a new version of a CLI tool comes out with backwards-incompatible changes, or with some new feature that needs to be enabled. But in that scenario, you may not even want those changes because you may be using an older version of that tool. Fundamentally, if you know what formatter you're using I think you should be completely comfortable specifying the args yourself. That's just my opinion, though.

If you do want to re-use the built-in args, you can do that:

local prettier = require("conform.formatters.prettier")
local default_args = prettier.args
prettier.args = function(ctx)
  local found = vim.fs.find(".custom-config.json", { upward = true, path = ctx.dirname })[1]
  if found then
    return vim.list_extend({ "--config", found }, default_args)
  end
  return default_args
end

@WhoIsSethDaniel
Copy link
Contributor Author

Fair enough. Thank you for your help.

I'm still working through some things, but I have noticed that formatters can fail and there is no notification of this fact (that I can tell). Is this a bug?

@stevearc
Copy link
Owner

This is because some formatters are very noisy. For example, if you have a syntax error stylua will fail and provide a lot of output. I am undecided on this. I could make it notify on error unless you pass in quiet = true, but my concern is that for many users I've just created a chore where they get annoyed by the noise, have to look up the docs, and set quiet = true. I could make quiet = true the default, but there are other useful notifications that I really think shouldn't be silenced by default.

My hope with the current configuration was that if a formatter isn't working you'll check the logs. That might be too onerous. Would it help if I put the last few lines of the logfile in the :ConformInfo panel?

@WhoIsSethDaniel
Copy link
Contributor Author

The problem I run into is that I don't often notice whether the formatter is working or not until quite some time after it has stopped working. I like the ConformInfo panel (I've used it a lot testing some new formatters), but it doesn't tell me that the formatter is failing at the time it starts to fail. I have to know that it is failing to look there. Or just happen on it by chance. What about a generic error/warning message: "Formatter f is failing. Check logs for more information."?

@stevearc
Copy link
Owner

What do you think of #16 as a solution?

@WhoIsSethDaniel
Copy link
Contributor Author

What do you think of #16 as a solution?

That's great. I like it a lot.

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

2 participants