search

sindresorhus / atom-editorconfig

Helps developers maintain consistent coding styles between different editors
https://atom.io/packages/editorconfig
MIT License
812 stars 80 forks source link

[WIP] Implement the whitespace.suppressor service #199

Closed segevfiner closed 5 years ago

segevfiner commented 6 years ago

Accompanying PR for https://github.com/atom/whitespace/pull/161. No specs for now, I'm only opening this to garner attention for the whitespace package PR. This won't do anything without it.

This allows editorconfig to suppress the automatic transformations of the core Atom whitespace package, removing the need to disable the whitespace package and allowing it to work when the relevent editorconfig properties are unset.

TODO

Related

https://github.com/sindresorhus/atom-editorconfig/issues/105 https://github.com/sindresorhus/atom-editorconfig/issues/159

To make the maintainer's life easier, I...

florianb commented 6 years ago

It seems this won't be necessary anymore. Thank you for your efforts.

segevfiner commented 6 years ago

@florianb Can you link me into why this won't be necessary? Thanks. 😃

florianb commented 6 years ago

Ooops sorry - i misread that your Service was already implemented in the whitespace package and i thought i gonna go and just use it in the pending rewrite.

Sorry for the confusion.. 😩

the-j0k3r commented 5 years ago

This is a great idea, but none in Atom even reviewed that other PR. I imagine you need both to be merged in order to work?

Alhadis commented 5 years ago

I imagine you need both to be merged in order to work?

I'd say so, judging from the OP:

No specs for now, I'm only opening this to garner attention for the whitespace package PR. This won't do anything without it.

Now, if I were to guess, I'd say the service proposed by atom/whitespace#161 is a little too specific to warrant inclusion as a core service. The Atom devs would probably prefer a more general-purpose service, such as something to override all editor settings, rather than suppressing only a couple.

Alhadis commented 5 years ago

Sadly, I'm gonna close this, as I feel it's neither the right solution, nor something the Atom team will have the time or interest in reviewing.

Good news, though. I'm considering adding a package setting (off by default) that'll enable users to opt into more "aggressive" solutions for suppressing nuisances like the whitespace package. Similar options might be added for other problematic packages which are sufficiently popular enough to be worth the trouble.

Thanks for putting in the effort, though. =) Patience is appreciated.