Vscode: Auto save in new settings results in multiple validation errors when typing text

Created on 12 Sep 2018  路  10Comments  路  Source: microsoft/vscode

Steps to Reproduce:

  1. Open new settings editor, search for "emmet extension"
  2. Emmet: Extensions Path is a setting that takes path to a directory. Start typing a path

Since we auto-save in 100ms, the config change event is fired for almost every other character typed, which results in validation checks from the emmet extension resulting in a series of error notifications.

There might be other extensions in the marketplace that do such validations after listening the config change event.

feature-request settings-editor verification-needed verified

Most helpful comment

I don't think it has to be one or the other. We can save on focus out as well as with a bit longer delay (without losing focus) so if the user is quickly moving through settings or is just a fast typist and moves away from the settings view, it just saves immediately. Otherwise, it saves after some delay.

All 10 comments

Suggestions:

  • Increase the delay for auto-save to 1000ms. This is the default delay for file auto save. Or re-use the value from the auto save delay setting. This will not solve the problem, but will make it less annoying as the validations will be spaced more apart
  • Or Allow settings to opt-in to a much more delayed save than 100ms
  • Or Dont auto-save settings by default. Have a save button and an option to turn on auto save.

cc @Microsoft/vscode

I will add another suggestion:

  • save only on focus out, e.g. when the settings editor (or any of its input fields) lose focus then save settings

or we make it dependent on the setting changed. A 1 out of n selection can save right after the value changed. A field were the user types should save after a longer delay or when the focus gets lost.

_Atomic_ setting changes should react as fast as possible (1 out of n selection, checkboxes, etc). Text input changes (and similar) should probably work on focus out, like @jrieken suggests.

I think that increasing the debounce timeout for text boxes would be good, but I'm worried that only committing the change on blur could confuse users. At least for some settings, like if you change window.title, or a font family/size, or anything else that affects the appearance of the editor... letter spacing, line height, tab size, etc... and you don't see it take effect. Then you start hunting around for a "save" button. Not everyone will try clicking out of the text box at this point.

I don't think it has to be one or the other. We can save on focus out as well as with a bit longer delay (without losing focus) so if the user is quickly moving through settings or is just a fast typist and moves away from the settings view, it just saves immediately. Otherwise, it saves after some delay.

My extension has N properties, and an action triggered by onDidChangeConfiguration make sense only when the user has finished changing more than one property.

In my opinion the optimal way is to give the user full control through an explicit Save action, otherwise an implicit save on focus out (no properties in focus) should work as expected.

My extension has N properties, and an action triggered by onDidChangeConfiguration make sense only when the user has finished changing more than one property.

You can't guarantee that even in the JSON editor, some users will probably have autosave turned on, or just hit cmd+s at different times like I do by habit. I think it's better to handle that on the extension side, rather than have a setting editing "session".

Being the devil's advocate here.. (actually extension authors' advocate)...

You can't guarantee that even in the JSON editor, some users will probably have autosave turned on, or just hit cmd+s at different times like I do by habit.

While that is true for such users, we are now moving away from a bulk update -> one event fired most of the times to one setting update -> one event fired all the time in the new settings editor. Which means much more traffic... It is slightly concerning

I think it's better to handle that on the extension side

How do you suggest extensions handle this? Batch all the config changes, and act on the batch on regular intervals? Why have multiple extensions do that when the product can do that once?

How do you suggest extensions handle this? Batch all the config changes, and act on the batch on regular intervals?

Yes. I've forked this into #59391 and we can keep discussing there.

Was this page helpful?
0 / 5 - 0 ratings