Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

[WIP] Implement the whitespace.suppressor service #161

Closed
wants to merge 2 commits into from

Conversation

segevfiner
Copy link

@segevfiner segevfiner commented Jul 20, 2017

No specs for now. Before I invest more time into this, I prefer to get some feedback if this is even the right way to go. Maybe even from someone that maintains the editorconfig package which is where this issue comes from.

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This service allows packages to suppress the automatic transformations of the whitespace package. This is useful for packages that implement or want to control such functionality themselves. A package that implements this service will be called before whitespace modifies the whitespace in a file automatically and can decide to suppress the transformation. An example of such a service is below.

Alternate Designs

Having the user manually disable the whitespace package or change it's settings to work with packages that conflict with it is not convenient. And disabling whitespace entirely also removes the access to it's commands. Having other packages disable/enable the entire whitespace package automatically is quite an hack and probably won't work that well in practice, and will still remove the access to the whitespace package commands.

Their might be a different/better API for such a service, or maybe something different than a service that is appropriate.

For example wouldn't it feel nicer if all such whitespace, and similar settings, were something that is set per buffer (Defaulting to a language scope setting) and a package like editorconfig only had to modify such settings to enable/disable or disable and implement itself. I think some settings that are managed by core Atom and used by editorconfig are like this. I'm not familiar enough with the code base to say whether this is even a plausible idea. Just a drive by contributor here. And that's probably a much larger change anyhow...

Benefits

A package like editorconfig can implement the service and suppress the operation of the whitespace package automatically when it's managing the relevant transformation and leave it active otherwise, so the whitespace package settings take effect when unset. For example:

const provideWhitespaceSuppressor = () => {
	return {
		shouldSuppressRemoveTrailingWhitespace(editor) {
			const settings = editor.getBuffer().editorconfig.settings;
			return settings.trim_trailing_whitespace !== 'unset';
		},

		shouldSuppressEnsureSingleTrailingNewline(editor) {
			const settings = editor.getBuffer().editorconfig.settings;
			return settings.insert_final_newline !== 'unset';
		}
	}
}

And in package.json:

"providedServices": {
  "whitespace.suppressor": {
    "versions": {
      "1.0.0": "provideWhitespaceSuppressor"
    }
  }
},
(That's code I added locally to the editorconfig package to test this, it seemed to work but I didn't thoroughly test it)

Possible Drawbacks

Not exactly sure on the names I choose here. It's a yet another semi-internal API for solving a specific conflict with packages that want to manage this themselves and I'm not entirely sure about the design of this.

Applicable Issues

Fixes #160

This service allows packages to suppress the automatic transformations
of the whitespace package. This is useful for packages that implement or
want to control such functionality themselves.

Fixes atom#160
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a service allowing packages to suppress removal of whitespace
2 participants