Stabilize preferences initialization #6719
Conversation
Missing documentation, test failures. This is primarily for review.
Clean-up all the structures on remove: remove/add is used to setup the tests.
|
@dangoor what about creating in-memory user scope when the system fails (due to parse or fs error)? Brackets should also warn that the user prefs cannot be parsed/loaded. |
|
@busykai You got a JSHint Error -> Travis failure. |
|
thanks, @SAplayer ! i'll fix it with the next set of changes. |
"user" scope is treated differently than the rest of the scopes. if it fails, then "user" scope is installed using memory storage. additionally, when it fails due to ParsingError, the user is warned and the configuration file is loaded so that it could be fixed.
…ions' into fix-6660 Seemingly working merge with current work by @dangoor in adobe#6715. Conflicts: src/preferences/PreferencesBase.js src/preferences/PreferencesManager.js
|
@busykai I'm going to start reviewing this now. All that's left is the test change you referenced in your checklist? |
|
@busykai Good to know, thanks for the update. |
Conflicts: src/preferences/PreferencesBase.js
|
@dangoor ready for review. |
|
@busykai would you mind merging master into your branch? Sorry for the churn. I think the merge is easy (and did it myself), but I wouldn't want to mess it up! |
Conflicts: src/preferences/PreferencesManager.js
|
@dangoor, done! Didn't realize that view status has landed. all tests pass. |
| @@ -65,6 +65,11 @@ define({ | |||
| "ERROR_CREATING_FILE_TITLE" : "Error creating {0}", | |||
| "ERROR_CREATING_FILE" : "An error occurred when trying to create the {0} <span class='dialog-filename'>{1}</span>. {2}", | |||
|
|
|||
| // Application preferences corrupt error strings | |||
| "ERROR_PREFS_CORRUPT_TITLE" : "Preferences file is corrupt", | |||
| "ERROR_PREFS_CORRUPT" : "{APP_NAME} preferences file is not a valid JSON and cannot be parsed. It will now be opened so that you could fix it. You will need to restart or reload {APP_NAME} for the changes to take effect.", | |||
There was a problem hiding this comment.
How about something like:
"ERROR_PREFS_CORRUPT_TITLE" : "Error reading preferences",
"ERROR_PREFS_CORRUPT" : "Your preferences file is not valid JSON. The file will be opened so that you can correct the format. You will need to restart {APP_NAME} for the changes to take effect.",
|
Review complete. The change appears to work well in my bit of testing just now. I think just those handful of minor things to address and this will be set. |
|
Looks good. Merging |
|
Err, except I can't because of a conflict... |
Instead of chaining the adding of the scopes at the caller, scopes are now added in the order the addScope was called. For each scopeOrder, a shadowScopeOrder is maintained which tracks the scope load status. If the scope was specified explicit "before" location and the "before" scope failed to load, it will be added to the first successfully loaded scope after "before". "default" scope is always loaded for the context and it's always resolved.
Deficient $.when is replaced with new Async.waitForAll (which would call always only when all promises are either resolved or rejected).
cc: @dangoor -- this is working, but not mergeable fix for #6660. I'll create task list to complete it later today or tomorrow. Please let me know what you think.
TODO: