Fix bug #7300 (Prefs never saved again after opening ancestor folder of user prefs.json)#7376
Conversation
contains the user prefs JSON files in its subtree) - ensure that FileSystem always knows we've read the JSON file before we try to write to it. We don't want to enable the 'blind' flag since it will mean we virtually always overwrite external changes to the prefs file, and we don't want to start using file watchers to observe external changes on the fly since we haven't yet deeply tested having multiple watch roots active at once. - Rename confusing "filename" vars in PreferencesBase - Fix JSLint errors in ProjectManager from #7026
There was a problem hiding this comment.
minor nit: the preferred nomenclature is pathname
There was a problem hiding this comment.
Is it? I get 2x as many hits for 'filepath' in our codebase than 'pathname.' I'm not sure we've settled on any strict convention. I'm just certain that 'filename' is misleading here :-)
There was a problem hiding this comment.
Or I wonder if it should be fullPath to harmonize with the FileSystem nomenclature?
|
@dangoor Do you have cycles to review today, since this is in prefs code? |
|
@peterflynn Sure, I will review this today. |
|
Review complete. Everything looks fine to me and the change works well. I don't feel likely to be thrown off by any of the nomenclature we discussed, so I wouldn't block this PR on that. If you do want to change the name to pathname or fullPath, feel free to do so and merge without additional review. |
|
Thanks for the review! I'l just go ahead & land it now in the interest of getting on with the remaining sprint tasks... |
Fix bug #7300 (Prefs never saved again after opening ancestor folder of user prefs.json)
3 participants
Fix bug #7300 -- ensure that FileSystem always knows we've read the JSON file before we try to write to it. We don't want to enable the 'blind' flag since it will mean we virtually always overwrite external changes to the prefs file, and we don't want to start using file watchers to observe external changes on the fly since we haven't yet deeply tested having multiple watch roots active at once (see #7375).
Also: