Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLOSED] Exposing removeFromWorkingSet() API seems error prone #1426

Open
core-ai-bot opened this issue Aug 29, 2021 · 4 comments
Open

[CLOSED] Exposing removeFromWorkingSet() API seems error prone #1426

core-ai-bot opened this issue Aug 29, 2021 · 4 comments

Comments

@core-ai-bot
Copy link
Member

Issue by peterflynn
Sunday Aug 26, 2012 at 21:15 GMT
Originally opened as adobe/brackets#1451


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).

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Tuesday Aug 28, 2012 at 17:38 GMT


Reviewed.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Sep 06, 2012 at 04:02 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Mar 18, 2014 at 01:29 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Thursday Feb 12, 2015 at 22:32 GMT


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

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

No branches or pull requests

1 participant