-
Notifications
You must be signed in to change notification settings - Fork 177
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: add pre and post formatter hooks #623
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on board with the general idea; left a bit of feedback
vim.api.nvim_exec_autocmds("User", { | ||
pattern = "ConformPreFormatter", | ||
data = { formatter = formatter }, | ||
}) | ||
log.info("Run %s on %s", formatter.name, vim.api.nvim_buf_get_name(bufnr)) | ||
log.trace("Input lines: %s", input_lines) | ||
callback = util.wrap_callback(callback, function(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try to provide the guarantee that if ConformPreFormatter
is called, then ConformPostFormatter
will also be called. Right now there are some error paths that won't hit the second autocmd. I believe if we move it into this util.wrap_callback
function it will always get called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved this into the callback and changed the event data to return the error (code and message)
lua/conform/runner.lua
Outdated
@@ -298,6 +298,10 @@ local last_run_errored = {} | |||
---@param callback fun(err?: conform.Error, output?: string[]) | |||
---@return integer? job_id | |||
local function run_formatter(bufnr, formatter, config, ctx, input_lines, opts, callback) | |||
vim.api.nvim_exec_autocmds("User", { | |||
pattern = "ConformPreFormatter", | |||
data = { formatter = formatter }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole formatter contains a lot of fields that probably aren't very useful. Could we start with just passing the name and then add other data if it's needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I've left the structure of data.formatter.name
so it can be easily extended in the future
To follow naming patterns of Vim/nvim I suggest changing event names to See |
Also changes the event formatter data to only contain the name of the formatter
Thanks for the feedback! I have actioned that and also gone with the rename suggested by @gegoune |
LGTM thanks for the PR! |
This is a feature I've added to my local copy of
conform.nvim
which I wondered if you would be interested in adding! I would've created and issue instead but I thought I'd just do a PR instead as I had already made the changes locally.What?
Auto command triggers for
ConformPreFormatter
andConformPostFormatter
which fire before and after each formatter that is called. This allows hooking into the format process for each formatter.Why?
I've added this as the formatters I have to run for work can sometimes take a while and there doesn't seem to be any feedback as to which formatter is currently running.
Adding these hooks enables me to have the following auto commands:
which results in the following:
Screen.Recording.2025-01-14.at.13.02.18.mov