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

Regression: Editing user settings is broken #31246

Closed
vvs opened this issue Jul 22, 2017 · 13 comments
Closed

Regression: Editing user settings is broken #31246

vvs opened this issue Jul 22, 2017 · 13 comments
Assignees
Labels
settings-editor VS Code settings editor issues
Milestone

Comments

@vvs
Copy link

vvs commented Jul 22, 2017

  • VSCode Version: Code - Insiders 1.15.0-insider (81e812c, 2017-07-21T13:15:10.775Z)
  • OS Version: Windows_NT x64 10.0.15063
  • Extensions:
Extension Author (truncated) Version
Bookmarks ale 0.15.2
project-manager ale 0.18.1
vscode-eslint dba 1.2.11
gitlens eam 4.3.2
EditorConfig Edi 0.9.4
beautify Hoo 1.1.1
atom-keybindings ms- 2.9.1
stylelint shi 0.28.0

When EditorConfig extension is installed, editing the user settings is broken. The regression was introduced in e6770cc, according to git bisect.

Steps to Reproduce:

  1. Install EditorConfig extension
  2. Open the settings Ctrl+,
  3. Try to type Enter on some line, the new line is inserted, but the cursor remains on the initial line. No Undo as well!
  4. Try to type anything, the letters are inserted, but the cursor remains on the same place, which totally breaks the editing. Typing 1, then 2 and then 3 will end up with 321

Reproduces without extensions: No

@vscodebot vscodebot bot added the insiders label Jul 22, 2017
@vscodebot vscodebot bot added the editor label Jul 22, 2017
@sandy081 sandy081 assigned sandy081 and unassigned rebornix Jul 24, 2017
@sandy081 sandy081 added the settings-editor VS Code settings editor issues label Jul 24, 2017
@sandy081 sandy081 added this to the July 2017 milestone Jul 24, 2017
@sandy081
Copy link
Member

@vvs Thanks for reporting this.

Fixed it.

sandy081 added a commit that referenced this issue Jul 24, 2017
@vvs
Copy link
Author

vvs commented Jul 24, 2017

@sandy081 Is this quick auto-save really needed for the settings?

You see, even with the additional fix, there are cases when text gets grabled in the settings. For example, if I quickly type 123456, chances are the end result will be something different, like 132456 or 134562.

Further more, it is impossible to indent to the new line to start typing some new setting, since this immediate auto-save would strip out the starting spaces and the cursor will be forced to the start of the line. Then I type TAB, it gets indented to the proper position, and immediately gets back to start of the line.

All in all, pretty inconvenient experience now with the settings.

@sandy081
Copy link
Member

@vvs There were couple of reasons for implementing Auto save of settings.

IMO Auto saving the settings always, improves the user experience. Do you think, the current delay time of 200ms is too short? I tried several cases to reproduce the issue with this delay and I could not reproduce.

To be safe, I will use the default value of files.autoSaveDelay configuration which is 1s.

sandy081 added a commit that referenced this issue Jul 24, 2017
@vvs
Copy link
Author

vvs commented Jul 24, 2017

@sandy081 Thank you for the explanation, so I now have better understanding of why the settings auto-saving is desirable in some contexts.

Still, for me, in my simple single-workspace world the auto-saving is kinda bad.

There are multiple scenarios when auto-saving is less than ideal:

  • I start editing the settings, typed "" somewhere in the middle, this makes the file invalid, but it is saved and immediately my theme settings are gone, my editor switches from my theme to the default one. Looks bad and quite unexpected, when entire editor "blinks" from white to black, when fonts change, etc.
  • I rearrange my settings and would like to complete the change before I save everything, but auto-save saves the intermediate results with conflicting settings
  • When I start a new indented line, chances are the spaces will be removed and the cursor would get back to the start position (even with 1 sec delay that happens often)
  • When I edit my settings, they are constantly being saved, in all intermediate states of editing, and VS Code tries to apply some of these incorrect, invalid, intermediate settings with no apparent reason. Sometimes things get quite confusing so it is just better to restart the VS Code to make sure I am in the consistent state with the proper configuration settings.

@vvs
Copy link
Author

vvs commented Jul 24, 2017

Here's an example of rather bad and unexpected behavior:
vs_code-settings

@sandy081
Copy link
Member

@vvs Is not it the same when you have auto save enabled? Is your argument is to disable auto save completely for settings?

If you see above gif, one of the settings has become invalid due to the edit that was done and Editor gets updated according to the settings those are valid at present.

Current settings experience is always based on editing, so there can be an intermediate state always. Hence, I think auto save is not the prime reason for the issues you mentioned.

IMO instant feedback to the user is good. I agree there are pros and cons with the new behaviour. If the advantages are good enough then the above corner cases can be overseen.

@sandy081
Copy link
Member

@Microsoft/vscode Opinions on always auto saving Settings with the delay of 1s - same as files.autoSaveDelay value

@egamma
Copy link
Member

egamma commented Jul 24, 2017

@sandy081 can you check whether there is a parse error before you automatically save the file?

@vvs
Copy link
Author

vvs commented Jul 24, 2017

@sandy081,

Is not it the same when you have auto save enabled?

I am not really using auto-save feature, and for the reasons we discuss here as well: it is inconvenient when a file I am currently editing is being changed under myself at random times (and removing trailing spaces, including the line I am currently started, and possibly reformatting the rest).

Back to the settings. I can see where saving the settings without user requesting it would be nice:

  • When user changes some enum-like settings via UI "Edit" button (for example, 'true' to 'false')
  • When user adds settings from the left pane to the right pane
  • When user closes the settings file or closes the VS Code altogether

IMO instant feedback to the user is good.

There are a lot of cases when there will be no instant feedback anyways, since some of the settings would require user to reload/restart the editor or to switch to other file to see the change.

@Tyriar
Copy link
Member

Tyriar commented Jul 24, 2017

Opinions on always auto saving Settings with the delay of 1s - same as files.autoSaveDelay value

Does this affect people who have auto save disabled? If so then 👎 , it doesn't auto save anywhere else so it's inconsistent and could lead to confusion.

@sandy081 sandy081 reopened this Jul 24, 2017
sandy081 added a commit that referenced this issue Jul 25, 2017
- Revert auto saving settings
- Revert virtual editor for workspace settings in MR workspace
@sandy081
Copy link
Member

After discussing with the team, reverted auto saving, to not to regress the behaviour for users not using auto save.

Side affect of this is to revert virtual editor for workspace settings in MR workspace and show the workspace configuration file completely.

@bpasero
Copy link
Member

bpasero commented Aug 11, 2017

@sandy081 makes sense to me, we should maybe stop trying to fix the settings editor and do the real thing with a UI editing experience instead...

I like how the settings are presented today with your decorations, so I think we are fine for now:

image

Maybe we could immediately put the cursor into the settings section to make clear where the editing happens.

Did you have a chance to play with the readonly regions in the editor? Even though it is not a 100% perfect fix for the problem, maybe it would avoid some frustrations when editing this file.

@sandy081
Copy link
Member

@bpasero I am not aware of the readonly regions. Will look into them.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
settings-editor VS Code settings editor issues
Projects
None yet
Development

No branches or pull requests

6 participants