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

Unable to write to settings should offer an action to open settings #16580

Closed
isidorn opened this issue Dec 6, 2016 · 10 comments
Closed

Unable to write to settings should offer an action to open settings #16580

isidorn opened this issue Dec 6, 2016 · 10 comments
Assignees
Labels
config VS Code configuration, set up issues feature-request Request for new features or functionality verified Verification succeeded
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Dec 6, 2016

  1. Have an error in your user settings
  2. View > hide activity bar
  3. Notice an error that we are unable to write to settings and that the user should fix them

This error should have an action to open settings as most users have a hard time finding settings.

screen shot 2016-12-06 at 10 31 13

@bpasero
Copy link
Member

bpasero commented Dec 8, 2016

Also: we should try to be more aggressive in writing to the settings file if this happens from the default settings editor. A trailing comma for example should not block us from doing the write. Since you see the result of the operation and you can undo, we can be more relaxed and validate less imho.

@BobbyBabes
Copy link

@bpasero Thanks for the hint to the trailing comma.
Just upgraded to 1.8.0. It was a bit confusing why I couldn't change settings through the pencil icon, but could with a manual change followed by Ctrl+S.
Now the "... correct errors/warnings in the file ..." part of the error message makes more sense.

@sandy081
Copy link
Member

sandy081 commented Dec 20, 2016

Enhanced config editing service to be aggressive when writing to buffer.

  • If writing directly to disk, it looks for all validations
  • If writing to buffer, all errors are ignored while updating and saved if not dirty.

So when using actions inside Settings editor, its writes the configuration always. Since user see the editor this makes sense

@sandy081 sandy081 added the verification-needed Verification of issue is requested label Dec 20, 2016
sandy081 added a commit that referenced this issue Dec 20, 2016
sandy081 added a commit that referenced this issue Dec 20, 2016
@sandy081 sandy081 modified the milestones: February 2017, January 2017 Jan 23, 2017
@sandy081
Copy link
Member

Config editing service was improved to be resilient with trailing commas. But all other errors are still valid and considered.

@sandy081
Copy link
Member

All callees of configuration editing service are explicitly calling message service to show the message. This can be moved to config editing service to show message as an option.

@sandy081 sandy081 modified the milestones: March 2017, February 2017 Feb 21, 2017
@isidorn isidorn added the verified Verification succeeded label Feb 22, 2017
@isidorn
Copy link
Contributor Author

isidorn commented Feb 22, 2017

Reopening as I still get the same behavior as I described in my first comment

edit: args this item is not even closed and has verification needed 😕

@isidorn isidorn removed verification-needed Verification of issue is requested verified Verification succeeded labels Feb 22, 2017
@sandy081
Copy link
Member

Its fine to have Verification needed tag even if it was not closed. But not sure how did it come verification list? The query should not contain any open items :)

@isidorn
Copy link
Contributor Author

isidorn commented Feb 22, 2017

I have my own cool query which obviously failed in this case :)

@sandy081 sandy081 modified the milestones: April 2017, March 2017 Mar 27, 2017
@sandy081 sandy081 added this to the Backlog milestone Apr 25, 2017
@sandy081 sandy081 removed this from the April 2017 milestone Apr 25, 2017
@gregvanl
Copy link

gregvanl commented May 2, 2017

I think we need to add a button to open settings when this error occurs as mentioned in the initial comment. It comes up often in doc feedback verbatim where users don't know where to go when a settings.json error occurs. Adding a button seems a good first step.

@gregvanl
Copy link

gregvanl commented May 2, 2017

Adding @sandy081 to hopefully get this into the May milestone.

@sandy081 sandy081 modified the milestones: May 2017, Backlog May 12, 2017
sandy081 added a commit that referenced this issue May 12, 2017
sandy081 added a commit that referenced this issue May 12, 2017
marckassay pushed a commit to marckassay/vscode that referenced this issue May 12, 2017
marckassay pushed a commit to marckassay/vscode that referenced this issue May 12, 2017
marckassay pushed a commit to marckassay/vscode that referenced this issue May 12, 2017
sandy081 added a commit that referenced this issue May 14, 2017
marckassay pushed a commit to marckassay/vscode that referenced this issue May 15, 2017
@gregvanl gregvanl added the verified Verification succeeded label May 16, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config VS Code configuration, set up issues feature-request Request for new features or functionality verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants