working set files are not guaranteed to be openable, so check for failure #1564
Conversation
|
Reviewing |
| // File chosen to be switched to could not be opened, and the original file | ||
| // is still in editor. Close it again so code will try to open the next file, | ||
| // or empty the editor if there are no other files. | ||
| closeFullEditor(file); |
There was a problem hiding this comment.
What if we change DocumentCommandHandlers.doOpen() to call DocumentManager.notifyFileDeleted() instead of calling removeFromWorkingSet() as it does today? Then this fail() block would be unneeded (since notifyFileDeleted() calls closeFullEditor()).
That would also get us halfway to fixing #1451 in the bargain...
There was a problem hiding this comment.
That change doesn't work if I remove the fail() block code, but it does work if I leave the fail() block code in.
I'm not sure if I agree with #1451, so I'll add my comment there.
There was a problem hiding this comment.
Ok, I see -- my suggestion isn't sufficient because closeFullEditor() is relying on FILE_OPEN (of the no-longer-existing file) to switch away from the closed file's editor. FILE_OPEN notifying that the next file was deleted doesn't substitute for that, since notifyFileDeleted() only touches the editor if the deleted file was open in the editor. It has no idea about the pending close operation that wants to get rid of some other editor.
I still believe #1451 is a change we should make, but it indeed doesn't help with anything here. Sorry for the confusion.
|
Done reviewing |
|
With that question resolved, I think this is good to merge as-is |
This is for issues #1543 and #1544