Skip to content

Conversation

@stevearc
Copy link
Owner

This deprecates the lsp_fallback option (gracefully; it should continue to work) and replaces it with the lsp_format enum which can be several values. Three of the values produce behavior that was possible with lsp_fallback, and two of them are new behaviors. The new behaviors fix #316

lsp_format lsp_fallback description
nil or "never" nil or false Do not use LSP formatting
"fallback" true Use LSP formatting if there are no formatters configured for this filetype
"prefer" - Use LSP formatting when it is available, otherwise fall back to the formatters configured for this filetype
"last" "always" Run configured formatters, then run LSP formatting
"first" - Run LSP formatting, then run configured formatters

@stevearc stevearc merged commit 9228b2f into master Jun 17, 2024
@stevearc stevearc deleted the stevearc-lsp-format branch June 17, 2024 14:15
@folke
Copy link
Contributor

folke commented Jun 19, 2024

Hi!

I'm getting multiple reports from users that the lsp fallback doesn't work anymore.
Haven't looked into it myself.

@dpetka2001
Copy link

I don't know if this is an intended change or not, but the previous fallback behavior didn't check if there was a formatters_by_ft defined for the current buffer, so it just used lsp formatting as a last resort.

Now, with this line introduced in the latest update, it seems that it also checks if there's no filetype formatter defined in formatters_by_ft, which is clearly different from before. This leads to confusion to users who were accustomed to previous behavior, that used LSP formatting if no other formatting was used before. Again, I don't know if this is intended behavior or not in the latest update.

@folke
Copy link
Contributor

folke commented Jun 19, 2024

@dpetka2001 nice find!

The LSP should definitely be used as a fallback.
@stevearc I assume this change was not intentional?

@dpetka2001
Copy link

Also, this function M.will_fallback_lsp does not have any references after the last update. So, it's not being used. Previously, it was used here. So, I believe more changes would have to be introduced other than the line that checks for the filetype formatters. Unfortunately, I don't understand the codebase that well to really understand how this should be solved and can't submit a PR. Sorry about that.

@dpetka2001
Copy link

Did my best and submitted a PR that fixes this behavior if it's not intentional. Please feel free to disregard it if you think a better approach is possible, as I'm not that familiar with the codebase myself.

@stevearc
Copy link
Owner Author

This behavior change is not intentional, it was supposed to be a transparent API change with no effect on behavior. I'll take a look at the PRs and make sure a fix gets in soon

@stevearc
Copy link
Owner Author

Should be fixed by bde3bee

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.

feature request: prefer LSP and fallback on formatters

4 participants