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

Get rid of "format-all-ensure-formatter" #133

Open
mzuther opened this issue Apr 2, 2021 · 3 comments
Open

Get rid of "format-all-ensure-formatter" #133

mzuther opened this issue Apr 2, 2021 · 3 comments

Comments

@mzuther
Copy link

mzuther commented Apr 2, 2021

Hi!

Is there really a good use case for format-all-ensure-formatter? To me, it seems like like a superfluous option that only does what a normal user would expect anyway. But maybe I'm just being blind here.

Still, I think that you should

  • get rid of format-all-ensure-formatter (and call it behind the scenes), or
  • call it by default and let power users disable it by setting an option such as format-all-do-not-ensure-formatter

Thanks for your package – now that I call format-all-ensure-formatter from a hook it works great! It seems that I skipped that paragraph when reading the manual... 😄

Martin

@lassik
Copy link
Owner

lassik commented Apr 3, 2021

The point of not using a formatter automatically is that many projects do not use code formatters (or they use a formatter for some languages, but not for others). If you have a default formatter, and use it to format code belonging to those projects, the formatting will likely be incorrect according to the style guide of that project.

I agree that it's confusing that you have to remember to call format-all-ensure-formatter. I can't think of an ideal way to do this. Would be nice to hear ideas from more people.

@lassik
Copy link
Owner

lassik commented Apr 3, 2021

Also, setting format-all-formatters buffer-locally to nil is supposed to disable formatting for that buffer. These are quite logical semantics, but nil is currently the default value, and as you say, that may not be a logical default setting.

@mzuther
Copy link
Author

mzuther commented Apr 6, 2021

Thanks for your fast response!

You are right, the semantics make sense and are pretty logical. But maybe you should add your explanation to the README as it currently isn't very clear on that point. 😄

Also, I find that adding a hook to a package in order to run that package slightly counter-intuitive. My idea is to remove the format-all-ensure-formatter hook and exchange it for a customizable variable format-all-enabled (declared as safe) that is set to non-nil by default. This could easily be customized using local variables, and should run just as fast as the current implementation.

Just my two cents. In case you want to brain-storm, go ahead. 😄

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

2 participants