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

use EditorConfig as a config file in shfmt #393

Closed
jokeyrhyme opened this issue Jul 15, 2019 · 12 comments
Closed

use EditorConfig as a config file in shfmt #393

jokeyrhyme opened this issue Jul 15, 2019 · 12 comments
Milestone

Comments

@jokeyrhyme
Copy link

Howdie, love this project, great work!

One thing I've wondered about is how EditorConfig and shfmt interact. Currently, I have several projects where I need to take special care to invoke shfmt in a way that does not contradict my EditorConfig settings. e.g. EditorConfig's indent_style and indent_size options versions shfmt's -i N flag

It would be terrific for me, and maybe even a partial solution for #358 and #234 if shfmt, when no -i N flag is provided, would look for .editorconfig files, and act appropriately:

  • if indent_style=tab, then use the default shfmt behaviour
  • if indent_style=space, then use shfmt -i 2
  • if indent_style=space and indent_size=N, then use shfmt -i N

Let me know if this is the sort of thing you'd accept a PR for :)

Cheers!

@mvdan
Copy link
Owner

mvdan commented Jul 15, 2019

Thanks for raising the issue. This sounds like a good idea in principle, because it doesn't add a new config file just for shfmt, and could mean better integration with existing standards and tools.

Some questions:

  • Does this ignore options which don't show up in EditorConfig?
  • I assume flags would take precedence over EditorConfig?
  • Is there a precedent in tools using EditorConfig? As far as I can tell, it's mainly for interactive editors.

My only remaining itch is the complexity. Right now, the tool behaves in a predictable way, irrespective of the current directory, and has very few external dependencies. This makes both aspects slightly worse, which should be taken into account. You could even call this a breaking change, unless reading the EditorConfig file is behind a flag.

Lucky for you, I'm about to release v3.0, so we can make breaking changes right now. I'm still not sure that we want to, though.

@mvdan
Copy link
Owner

mvdan commented Jul 15, 2019

Another idea that comes to mind is meta-formatters. I assume some of them support shfmt among other formatters, and also support config files like EditorConfig. Have you given that a try?

@jokeyrhyme
Copy link
Author

Does this ignore options which don't show up in EditorConfig?

I was thinking so. Indentation was the main thing on my mind, but EditorConfig does also specify line-endings and trailing-whitespace (end-of-line and end-of-file) settings, which I don't believe are covered by shfmt. It would be super neat for shfmt to honour these settings, too, but these aren't configurable via shfmt flags, so I think it's fair to leave the out of scope, at least initially

I assume flags would take precedence over EditorConfig?

Yeah, I try to keep consistent with "specific beats general", and a flag seems much more intentional and specific than an automatically-discovered .editorconfig file

Is there a precedent in tools using EditorConfig? As far as I can tell, it's mainly for interactive editors.

prettier, which is a popular JavaScript, TypeScript, CSS, Markdown auto-formatter reads some settings from EditorConfig the same way: https://prettier.io/blog/2017/12/05/1.9.0.html#configuration . It does have it's own .prettierrc, but you can skip it completely if you only care about the parts that EditorConfig covers

I don't think rustfmt or gofmt do this, but those are 1st-party language tools with opinionated non-configurable formatting rules, so I think that makes sense there

eslint is a configurable formatter, and does not read EditorConfig files

So, I guess, in terms of prior art, I have one for, and one against

Another idea that comes to mind is meta-formatters.

I'm familiar with gometalinter, but I hadn't hear of "meta-formatters", unless you mean something like https://github.com/w0rp/ale for vim, that hooks into lots of linters and formatters, although it doesn't try to blend them together, it just runs them in sequence with defaults and whatever settings they find

@jokeyrhyme
Copy link
Author

You could even call this a breaking change, unless reading the EditorConfig file is behind a flag.

If we auto-detect EditorConfig by default, then it would be a breaking change for anyone using shfmt's default tabs-indenting, with a .editorconfig that is set to use spaces

It wouldn't affect anyone explicitly controlling indentation with shfmt -i N, because the flag would take precedence over the .editorconfig

@dcsobral
Copy link

I wanted to create a ticket to support modeline(*), so I looked for relevant tickets and, bam. I hadn't heard of EditorConfig, but it's a much better solution than supporting a modeline thingy.

On the topic of tools supporting EditorConfig, and their site mentions three headless tools: ant, gradle and maven. I see tortoisegit also provides support on its merge tool, and Github has support for it when displaying files (in addition to, presumably, its editor).

(*) Modeline from vim saves vim non-default settings as a comment with the file.

@mvdan
Copy link
Owner

mvdan commented Aug 5, 2019

If we auto-detect EditorConfig by default, then it would be a breaking change for anyone using shfmt's default tabs-indenting, with a .editorconfig that is set to use spaces

I think that set of people is small, and 3.0 is around the corner, so I think this kind of backwards incompatible change will be OK.

@dcsobral thanks for the hint about other tools using EditorConfig. However, as far as I can see, those tools need the user to specifically install a plugin and enable the feature, so it doesn't seem like a default. We're different because we have no existing config file of any kind.

Another great thing about EditorConfig is that editors can define and use "domain-specific" configuration entries, and they will be ignored by editors that don't know about them. So we could in theory add support for every shfmt flag to EditorConfig without breaking other tools/editors, which I think is awesome.

Let's do this. 3.0 can begin by simply learning how to indent form the config file.

I only wonder about what section we should use for "shell language". [*.{sh,bash}] would cover files with extension, but not executable scripts with a shebang that don't have a filename extension. I guess we can worry about that later. Another option would be to add special sections that we understand to mean "language", such as [shell], [shell-posix], [shell-bash], and so on. Though I imagine that would go against the spec.

@mvdan mvdan changed the title proposal: detect EditorConfig and use as a config file for shfmt use EditorConfig as a config file in shfmt Aug 5, 2019
@mvdan mvdan added this to the 3.0 milestone Aug 5, 2019
@mvdan
Copy link
Owner

mvdan commented Aug 5, 2019

Ah, I see other tools have had similar discussions:

editorconfig/editorconfig-emacs#75
editorconfig/editorconfig#239

I quite like the [[language-name]] idea. Unlike mine above, it can't really clash with existing useful config sections.

@mvdan
Copy link
Owner

mvdan commented Aug 9, 2019

editorconfig/editorconfig#404 is progressing quickly, so let's wait a week and see what the state is. We'll probably implement this before the language feature is well established, but we might as well see if the proposed syntax is defined in a few days.

@cxw42
Copy link

cxw42 commented Aug 21, 2019

Similar to dlenski/wtf#15 , which is with respect to a different formatter

@mvdan
Copy link
Owner

mvdan commented Sep 28, 2019

Looks like that proposal has stalled a bit, so I'm going ahead with experimental support for now. I won't do anything non-official like [[language]], and instead just see if it works well with simpler patterns like [scripts/] or [*.sh] as a start.

@mvdan
Copy link
Owner

mvdan commented Oct 1, 2019

I started looking at EditorConfig libs, and none of the existing ones fit my needs, so I wrote one. That took less time than trying to adapt one of the existing ones.

That was a pretty big chunk of work done; adding the feature itself should just be a few hours more.

@mvdan
Copy link
Owner

mvdan commented Nov 10, 2019

Thank you all for your patience. The first version of this feature is now ready; please feel free to try it via GO111MODULE=on go get mvdan.cc/sh/v3/cmd/shfmt@master. The README file contains a bit of relevant docs.

I'll release one or two last pre-release tags in the coming weeks, but v3 is pretty much ready. I expect to release the final v3.0.0 before the end of the month, so please help me test the feature and give input while we can still do breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants