Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Exposing removeFromWorkingSet() API seems error prone #1451

Closed
peterflynn opened this issue Aug 26, 2012 · 4 comments
Closed

Exposing removeFromWorkingSet() API seems error prone #1451

peterflynn opened this issue Aug 26, 2012 · 4 comments

Comments

@peterflynn
Copy link
Member

A few sprints ago we changed removeFromWorkingSet() from a private method to a public one, to support #1197 (see 59e40a9).

It seems easy to misread this as the API to call when you want the same effect as the user clicking the "X" button in the working set. However, it's actually a low-level model API that doesn't handle prompting for unsaved changes, etc. (The FILE_CLOSE command would be the right thing to call).

I propose we change removeFromWorkingSet() back to private, and have the DocumentCommandHandlers error case call notifyFileDeleted() instead (which is more semantically correct anyway, and is already a public API).

@ghost ghost assigned peterflynn Aug 26, 2012
@gruehle
Copy link
Member

gruehle commented Aug 28, 2012

Reviewed.

@redmunds
Copy link
Contributor

redmunds commented Sep 6, 2012

One of the problems fixed with #1197 was that all of the API calls used were too high-level, and switching projects (closing down multiple files in 1 project and opening multiple files in another project) did a UI refresh for each file as it was opened or closed.

Yes, we need APIs for every UI interaction, but we also need APIs to control the state of the app which are separate from the UI. Maybe a naming convention will make it easy to tell the difference between them.

@peterflynn
Copy link
Member Author

Could maybe be tackled in a batch cleanup along with #513 & #5706

@JeffryBooher
Copy link
Contributor

@peterflynn can we close this? The API has been deprecated and removed with SplitView

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants