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

make the timeout for formatOnSave configurable #43702

Merged
merged 2 commits into from Mar 2, 2018

Conversation

@buyology
Copy link
Contributor

buyology commented Feb 14, 2018

Fixes #41194

As reported by users of extensions for TypeScript, Python, ruby and go formatOnSave-commands can take longer than the hard coded 750 ms, e.g. for large projects or complex commands.

This removes the hardcoded value and instead makes it user configurable.

@@ -298,6 +298,12 @@ configurationRegistry.registerConfiguration({
'description': nls.localize('formatOnSave', "Format a file on save. A formatter must be available, the file must not be auto-saved, and editor must not be shutting down."),
'overridable': true,
'scope': ConfigurationScope.RESOURCE
},
'editor.formatOnSaveTimeout': {

This comment has been minimized.

@MikhailArkhipov

MikhailArkhipov Feb 15, 2018

Member

Setting is editor-global, it will be hard for end user to figure out correct timeout as it depends on the language and the machine speed.

This comment has been minimized.

@buyology

buyology Feb 15, 2018

Contributor

True. Made it overridable, then at least every extension that identifies this as a potential issue can provide a sane default (and the user the opportunity to configure it for that specific usage).

@buyology

This comment has been minimized.

Copy link
Contributor

buyology commented Feb 20, 2018

Any update on this @jrieken?

@prateek

This comment has been minimized.

Copy link

prateek commented Feb 23, 2018

+1 I was about to put up a PR for the same thing.

@jrieken jrieken added this to the March 2018 milestone Feb 26, 2018

@jrieken jrieken added the formatting label Feb 26, 2018

@jrieken

This comment has been minimized.

Copy link
Member

jrieken commented Feb 26, 2018

Back from vacations... Too late for Feb-endgame but will take a look in March.

@jrieken

jrieken approved these changes Mar 1, 2018

Copy link
Member

jrieken left a comment

Looks good. I will merge as soon as master is open for business again.

@jrieken jrieken merged commit 41f7a57 into Microsoft:master Mar 2, 2018

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment