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

Ignoring EditorConfig on stdin is probably a bad idea #450

Closed
rassie opened this issue Nov 20, 2019 · 3 comments
Closed

Ignoring EditorConfig on stdin is probably a bad idea #450

rassie opened this issue Nov 20, 2019 · 3 comments

Comments

@rassie
Copy link

rassie commented Nov 20, 2019

It's a common use-case for editors (e.g. Emacs) to use stdin, stdout and stderr as an interface to linters and formatters. Usually, the formatter is executed in the project directory so that even though there is no path information, an .editorconfig can be found. Would be great if stdin+editorconfig combination would be supported.

@mvdan
Copy link
Owner

mvdan commented Nov 20, 2019

Interesting, I hadn't thought about this scenario at all. Thanks for bringing it up.

If one wants to use shfmt on standard input with no EditorConfig, they can always rely on a formatting flag to turn off the feature, like -i=0. I mainly did this to reduce the amount of backwards incompatible changes in v3, but I think this change is probably worth it, and would make the EditorConfig logic a bit simpler.

@lassik
Copy link

lassik commented Jun 4, 2020

  • Supporting editorconfig with stdin is an excellent idea.

  • However, it should not be done by default. When people call a program to filter some data from stdin to stdout, they are not generally aware which directory they are running that program from. It can lead to surprising problems if the working directory matters in this case.

  • If cmd/shfmt: EditorConfig support in stdin doesn't support filenames #577 is implemented, stdin formatting should use .editorconfig if and only if the stdin filename flag is given.

@mvdan
Copy link
Owner

mvdan commented Jun 4, 2020

@lassik you're very right that I didn't consider filenames in my patch above. I'll write a longer reply to your new issue, since this one is closed.

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