Persist working set #108
Persist working set #108
Conversation
Refactor Documents to allow lazy initialization of editors. Restore DocumentManager working set from storage.
Change to Array.forEach. Add missing showInEditor() call to EditorManager._createEditor. Bug fixes for lazy editor creation.
ghost
assigned 
|
|
||
| prefs.files.forEach(function(value, index) { | ||
| // check if the file still exists | ||
| projectRoot.getFile(value.file, {} |
There was a problem hiding this comment.
getFile() looks like it creates the file if the file doesn't exist. That doesn't seem like what we want -- if the user has a file open in Brackets, quits, deletes the file through some other means, then re-opens Brackets, we'd re-create a zero-length file where the old one used to be...
There was a problem hiding this comment.
getFile() will only create a missing file if {create:true} is passed in the Flags arg. Line 438 in NativeFileSystem.
I tried that use case just now and it correctly skips the file. Are you seeing otherwise?
There was a problem hiding this comment.
Oops, misread the docs there -- sorry.
| /** | ||
| * @private | ||
| */ | ||
| function _doShow(document) { | ||
| // Show new editor | ||
| _currentEditorsDocument = document; | ||
| _currentEditor = document._editor; |
There was a problem hiding this comment.
One concern here is that this lazy-load/create approach introduces a new level of asynchronicity between the user selecting a file in the UI and the editor area actually updating to show that file. Depending on how long it takes to load a file, there may be some interval where the file tree / working set UI selection is out of sync with the editor, but the UI remains fully interactive.
Could bad things happen when the editor area and the rest of the UI (and DocumentManager) disagree about what the "current document" is? Should we try to block the UI until the editor switch is complete?
This may be an obscure edge case now, so maybe we can file a bug and punt... but it will definitely be a real concern once we have a browser-based incarnation.
There was a problem hiding this comment.
Yes, the UI should block until the editor switch is complete. Haven't done this before in HTML-land. Will investigate.
There was a problem hiding this comment.
I'll file an architecture bug for this.
Guard against null Document._editor Refactor readAsText out of EditorManager into DocumentManager
Fix up Document API docs when editor is not assigned. Refactor error dialog UI from FileCommandHandlers to EditorManager. Handle edge case when creating an editor for a file that no longer exists.
Conflicts: src/EditorManager.js
|
|
||
| if (rootPath === null || rootPath === undefined) { | ||
| // Load the last known project into the tree | ||
| rootPath = prefs.projectPath; | ||
| triggerInitEvent = true; |
There was a problem hiding this comment.
Could we call this flag something like 'isFirstProjectOpen'? It took me a minute of thinking through this method before I understood in which cases we fire initializeComplete and in which cases we don't.
I think a name like that would also help make clear the implicit assumption, that rootPath==null means it's the first (default) project open during launch, and rootPath!=null means it's not... If that assumption ever gets broken later, I think this will be easier to debug with a better name.
Refactor error dialog helper functions to EditorUtils. Add asserts to Document when editor is null Update docs
Conflicts: src/DocumentManager.js
| handleFileOpen(fullPath).done(function(doc) { | ||
| DocumentManager.addToWorkingSet(doc); | ||
| // jstree dblclick handling seems to steal focus from editor, so set focus again | ||
| EditorManager.focusEditor(); |
There was a problem hiding this comment.
I think it's best to leave these two lines back in ProjectManager where they were before. They were added specifically to address an issue related to double-clicking the file tree, which in the future might not be the only way in which handleFileAddToWorkingSet() gets called.
(As an aside, these lines don't really actually fix the bug, but that's a separate issue that we'll wind up addressing later).
|
Awesome, I think it's good to go. |
Persist working set & fix issue #121
@peterflynn
Refactor EditorManager and DocumentManager to allow lazy init of editors.
Persist working set files and the current active editor.
Add "initializationComplete" event to ProjectManager. Hook DocumentManager to this new event to initialize the working set.