Skip to content

Optionally use lsp as default formatter #16

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

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

erikzaadi
Copy link
Contributor

@erikzaadi erikzaadi commented Jul 17, 2023

If the setup flag lsp_as_default_formatter is passed, then instead of failing when a filetype is missing a formatter, it'll default to lsp

Partially resolves #8

@@ -0,0 +1,5 @@
local M = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe there need create a new file ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean like a state file? Or no need to create a new file and chuck that into formatters or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha sry i mean no need create a file. actually I don't understand there. for use default lsp .how to register filetype ? I think something need to do in filetype module when fmt_default_lsp is true ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that's the thing, you don't need to explicitly configure per file type with this change (when enabled).
The lsp format just work ™️ for whatever lsp server that's loaded for the current file type.
E.g if I'm editing a Python file and I've installed jedi or anakin, then they'll be used by the native lsp buf format function.
Regarding the config file, just wanted a in memory state for the plugin that can be set in the init file and later accessed by the formatter when invoked, thinking that this might be the first of many.
I'm quite sure I can move the config to something exposed by the formatter for the init to set if that's something you'd prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something should be like . when LspAttach register this filetype and default is lsp if you config some other tools then make lsp to the first in filetype.[this filetype]. but this mean if someone have au BufWritePre ft vim.lsp.buf.format he must remove this in his config .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glepnir not sure I understand the wanted changes..
if you're using nvim-lspconfig or something similar, then the filetypes are already setup automagically.
If someone has the BufWritePre thingy before using guard then it'll be the same issue before and after this pr.
Could you elaborate a bit more of the changes you want for this PR? I'll be glad to apply them!

@erikzaadi erikzaadi force-pushed the add-default-lsp-option branch 2 times, most recently from 41bf9bc to a14d6fe Compare July 20, 2023 07:02
@barrett-ruth
Copy link
Contributor

@erikzaadi is this ready for merge? Having an LSP fallback is one of the last things guard.nvim needs to be to replace the majority of null-ls formatting functionality.

@erikzaadi
Copy link
Contributor Author

@barrett-ruth IMHO Yes, still waiting for @glepnir 's reply to my comment (and approval)

@glepnir
Copy link
Member

glepnir commented Jul 22, 2023

Sorry for the late reply. I'm confused about the implementation here. guard supports multiple tools to format a language. In fact, the filetype module creates a formatting or lint configuration table (queue) for each file type. What we should do when using lsp as default is to register filetype to filetype module in LspAttach or insert lsp into the first like

if opt.lsp_as_default then
  vim.api.nvim_create_autocmd('LspAttach', {
      calback = function(args)
              local client = vim.lsp.get_client_by_id(args.data.client_id)
              if not client:support_methods('textDoucment/format') then
                   return
              end
              local  fthandler = require('guard.filetype')
              if fthandler[vim.bo[args.buf].filetype] and  fthandler[vim.bo[args.buf].filetype].fmt  then
                          table.insert(fthandler[vim.bo[args.buf]], 1, require('guard.tool.formatter').lsp)
              else
                     fthandler(vim.bo[args.buf].filetype):fmt(require('guard.tool.formatter').lsp)
               end
      end
  })
end

typing on github. so the code just example

@erikzaadi erikzaadi force-pushed the add-default-lsp-option branch from a14d6fe to 4d7df6b Compare July 25, 2023 10:20
@erikzaadi
Copy link
Contributor Author

@glepnir Used your reference to add the functionality , verified that it works locally.
Removed the config file, will squash the last commit before merging.

@erikzaadi
Copy link
Contributor Author

@barrett-ruth FYI #16 (comment)

@erikzaadi erikzaadi force-pushed the add-default-lsp-option branch from 3c0bd57 to b4843be Compare July 25, 2023 10:36
(Will squash this commit upon approval)
@erikzaadi erikzaadi force-pushed the add-default-lsp-option branch from b4843be to 01de8b0 Compare July 25, 2023 10:38
@glepnir glepnir merged commit 33afb5a into nvimdev:main Jul 26, 2023
@erikzaadi erikzaadi deleted the add-default-lsp-option branch July 31, 2023 07:54
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 requests: format without saving and default to LSP
3 participants