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

feat: add pre and post formatter hooks #623

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

itmecho
Copy link
Contributor

@itmecho itmecho commented Jan 14, 2025

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 and ConformPostFormatter 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:

vim.api.nvim_create_autocmd("User", {
  pattern = 'ConformPreFormatter',
  callback = function(event)
    print('running ' .. event.data.formatter.name)
  end
})

vim.api.nvim_create_autocmd("User", {
  pattern = 'ConformPostFormatter',
  callback = function(event)
    print(string.format('finished running %s: ok=%s', event.data.formatter.name, event.data.ok))
  end
})

which results in the following:

Screen.Recording.2025-01-14.at.13.02.18.mov

@github-actions github-actions bot requested a review from stevearc January 14, 2025 13:07
Copy link
Owner

@stevearc stevearc left a 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)
Copy link
Owner

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.

Copy link
Contributor Author

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)

@@ -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 },
Copy link
Owner

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?

Copy link
Contributor Author

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

@gegoune
Copy link

gegoune commented Jan 20, 2025

To follow naming patterns of Vim/nvim I suggest changing event names to ConformFormatPre and ConformFormatPost.

See :h autocmd-events.

Also changes the event formatter data to only contain the name of the formatter
@github-actions github-actions bot requested a review from stevearc January 20, 2025 12:36
@itmecho
Copy link
Contributor Author

itmecho commented Jan 20, 2025

I'm on board with the general idea; left a bit of feedback

Thanks for the feedback! I have actioned that and also gone with the rename suggested by @gegoune

@stevearc
Copy link
Owner

LGTM thanks for the PR!

@stevearc stevearc merged commit bf94626 into stevearc:master Jan 22, 2025
7 checks passed
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

Successfully merging this pull request may close these issues.

3 participants