Skip to content
This repository was archived by the owner on Aug 12, 2023. It is now read-only.

Meta-question: are you open to more pull reqs like https://github.com/jose-elias-alvarez/null-ls.nvim/pull/1313? #1337

Open
1 task done
andrewferrier opened this issue Jan 9, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@andrewferrier
Copy link
Contributor

andrewferrier commented Jan 9, 2023

Issues

  • I have checked existing issues and there are no existing ones with the same request.

Feature description

I note that in #1313 you added appropriate CLI options for denols for textwidth and shiftwidth. IMHO for these and other settings this is exactly the correct approach to take. With plugins like vim-sleuth - which I use - and the forthcoming built-in editorconfig support in NeoVim 0.9 - I think it's reasonable to assume that users should have correctly set textwidth, shiftwidth, expandtab, etc. most of the time (even if they don't, the proposal I'm going on to make would not make the situation any worse, I think).

In my personal null-ls configuration, I make fairly heavy use of this, setting appropriate CLI args for black, latexindent, prettier and others (please feel free to take a look, I think the code is fairly readable).

Would you be open to more pull requests to incorporate these kind of 'default options' into the default setup for null-ls formatters? If so, would you want one per formatter?

Help

Yes

Implementation help

No response

@andrewferrier andrewferrier added the enhancement New feature or request label Jan 9, 2023
@jose-elias-alvarez
Copy link
Owner

In short: yes, I think this is a good approach. As #1313 showed, though, there are definitely a few potential issues we have to look out for:

  1. We have to make sure that default values don't cause issues. I had to revert the Deno PR for now because the CLI didn't accept 0 as a value - see deno fmt is broken #1322.
  2. A user's Neovim options may not match the options defined in a project config file, leading to discrepancies. I agree that vim-sleuth basically makes this a non-issue, and I also hope that the addition of editorconfig support (which I wasn't aware of but is great to see!) will lead to more users setting these values correctly.
  3. We have to verify how each formatter handles the combination of CLI options + a config file. Well-behaved formatters should merge them, but I believe there are some that won't read config files if any options are set via the command line.

That means if we're going to do this, we'll have to proceed carefully.

@andrewferrier
Copy link
Contributor Author

andrewferrier commented Jan 9, 2023

@jose-elias-alvarez fair enough, and all good points. If and when I open pull reqs for this, I'll do it on a formatter-by-formatter basis so they can be discussed individually.

FYI, here's the editorconfig docs: https://github.com/neovim/neovim/blob/master/runtime/doc/editorconfig.txt

@andrewferrier
Copy link
Contributor Author

andrewferrier commented Jan 9, 2023

So I've been doing a bit more investigating/thinking. It seems that quite at least two of the formatters I've looked at so far (shfmt, prettier...) support a model like this where settings are evaluated in priority like this ("winner" at the top).

  1. The setting you pass in on the CLI, if set.
  2. The setting in .editorconfig (i.e. the formatter itself interrogates editorconfig).

What this means is that if a user does have an editorconfig file, but they are using NeoVim pre-0.9 without an editorconfig plugin, the situation may get worse with the new logic I'm talking about using here, because the values will instead come from the buffer settings, which may be wrong unless something like vim-sleuth has been used (which also interrogates editorconfig!).

However, with the default behavior for NeoVim 0.9 onwards w.r.t editorconfig (as long as the user doesn't turn it off), this would not be a problem as ultimately the settings will still come from editorconfig, but via NeoVim.

What this new behaviour does mean is that for people who don't use editorconfig, but are still using sleuth, or are editing a standalone file somewhere, running :set sw=3 and then reformatting via null-ls, will do the right thing.

For me, that's the right balance, but I wanted to explore this with you before moving forward.

@andrewferrier
Copy link
Contributor Author

@gpanders you might be tangentially interested in this since it's related to the new editorconfig support.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants