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

A contribution guide for adding preconfigured formatters #97

Closed
haras-unicorn opened this issue Nov 4, 2021 · 9 comments
Closed

A contribution guide for adding preconfigured formatters #97

haras-unicorn opened this issue Nov 4, 2021 · 9 comments

Comments

@haras-unicorn
Copy link
Collaborator

This is the only thing that is keeping me from switching from neoformat since they have a bunch of preconfigured formatters.
I would much rather contribute to this repo to add preconfigured formatters than to copy and paste a bunch of formatter configurations that are going to be the same for everyone anyway. I bet a lot of them could be ported straight from neoformat and then patched if users notice bugs.
Why don't I just stick with neoformat then? It is because it is written in Vimscript and it is synchronous which makes me wait for around 4-5s on >1000 LoC python files.

@mhartington
Copy link
Owner

So are you asking for us to provide some formatter-config out of the box? I had avoided that as it's hard to guess what everyone uses and having people provide their own was the simplest and most effective solution.

Many of the formatters could be ported from neoformat, which is the point of the config.md file. So people could just quick copy what they need and move on.

@haras-unicorn
Copy link
Collaborator Author

So are you asking for us to provide some formatter-config out of the box?

Yes. I would be willing to do this over the upcoming weekend (hopefully) by porting configs over from neoformat and submitting a pull request if you're interested in that.

I had avoided that as it's hard to guess what everyone uses and having people provide their own was the simplest and most effective solution.

These could be defaults that people could override with their own configs if it doesn't satisfy their needs, but coming from neoformat, I can assure you that 99% of the configs work as intended without any user configuration and not only that, but I'm also hesitant to make my own config because that just means more maintenance for me and everyone else using a formatter where this could be more of a group effort. I consider this a good step towards respecting the DRY principle on a community level.

Many of the formatters could be ported from neoformat, which is the point of the config.md file. So people could just quick copy what they need and move on.

The config.md file could be used as a guideline on how to configure formatters if the user wishes to override defaults.

@mhartington
Copy link
Owner

I do not want to add a bunch of configs out of the box. As I said, providing defaults is hard to do as everyone has opinions on what tool should be used. For instance, when I had used neoformat, I had to go through and disable every formatter for every filetype that prettier supported in order to just use prettier.

If this were to move forward, I think the way to handle this would be

  • create a new directory for all supported filetypes
  • each filetype can export a formatter config for a various tool
  • users can add them as they need.
-- lua/formatter/filetypes/typescript/prettier.lua

local M = {}
M. prettier  =  function()
  return {
    exe = "prettier",
    args = {...},
    stdin = true
  }
end

return M

Then folks could consume if they wanted by just importing that one config.
Opt-in vs having to opt-out.

What do you think of that?

@haras-unicorn
Copy link
Collaborator Author

haras-unicorn commented Feb 9, 2022

For instance, when I had used neoformat, I had to go through and disable every formatter for every filetype that prettier supported in order to just use prettier.

Looking through my config again, I discovered that I had the same issue with prettier. Opt-in with defaults sounds like the way to go then.

Would you be willing to accept a pull request over the weekend with configs ported over from neoformat in a separate folder as you said? I would also like to update the config.md file accordingly and add a basic Vim help file that describes these changes and explains basic setup for the plugin (nothing too verbose - just as a bonus since I don't see that you added it). Finally, I would like to be able to test all this and I see that you added some test files, but I don't know how to use them properly, so could you explain on how you used those to test?

EDIT: I would also like to add a CONTRIBUTING.md file, since my whole goal with this is for defaults to be extensible by other people wanting to contribute if you are okay with that.
Also, a note on the folder structure, each filetype could have just a file associated with it with functions named after their corresponding formatter. Would you like a structure like that or one where each filetype is in a separate folder and there are multiple files for each formatter, in which case I would just return a function from each file to reduce redundancy.

In other words this (1):

-- lua/formatter/filetypes/typescript/prettier.lua

return function()
  return {
    exe = "prettier",
    args = {...},
    stdin = true
  }
end

, or this (2):

-- lua/formatter/filetypes/typescript.lua

local M = {}

M. prettier  =  function()
  return {
    exe = "prettier",
    args = {...},
    stdin = true
  }
end

-- other formatters...

return M

or your original design (3)?

@mhartington
Copy link
Owner

I like option 2, lua/formatter/filetypes/<filetype>.lua. Probably easier to organize this way 😄.

Let's start with some of the more common ones for

  • JS/TS
  • CSS
  • HTML
  • Go
  • Rust
  • Python

I'm not sure which filetypes are the most popular, but this will be at least a good starting point.

@haras-unicorn
Copy link
Collaborator Author

Expect a PR on the weekend with those added in. I'll link this issue on there.

I'll also modify the config.md file accordingly. I don't know if I will have enough time to do the rest of the things I said during this weekend, but I will try, so expect these changes as a minimum.

@mjlbach
Copy link

mjlbach commented Feb 20, 2022

I think formatter.nvim + nvim-lint is a really great combination as an alternative to null-ls, but the reason many users have flocked to null-ls is the bundled (opt-in) configurations so, in my opinion, this is a great step!

As a side note, I would like to promote the combination of lspconfig + formatter.nvim + nvim-lint as sort of the modern alternative to ALE, neomake, etc.

@haras-unicorn
Copy link
Collaborator Author

As a side note, I would like to promote the combination of lspconfig + formatter.nvim + nvim-lint as sort of the modern alternative to ALE, neomake, etc.

Same here. I've been using this for quite some time now and the experience is great. 😄

@haras-unicorn
Copy link
Collaborator Author

Closing this in favor of #137 , #136 , and #135

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

3 participants