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] Fixes #1256 #1724

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

[CLOSED] Fixes #1256 #1724

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

Comments

@core-ai-bot
Copy link
Member

Issue by DennisKehrig
Thursday Oct 04, 2012 at 13:49 GMT
Originally opened as adobe/brackets#1769


Fixes #1256: Deleting a file should remove entry from inline editor

This one could use a thorough review...


DennisKehrig included the following code: https://github.com/adobe/brackets/pull/1769/commits

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Monday Oct 08, 2012 at 22:20 GMT


Done with initial review.

@core-ai-bot
Copy link
Member Author

Comment by DennisKehrig
Monday Oct 08, 2012 at 23:10 GMT


Thank you for the review! Much appreciated!

Personally I'm most skeptical about forwarding an event as the "cause" of another event. It feels fishy somehow. What are your thoughts on this?

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Monday Oct 08, 2012 at 23:19 GMT


I can't think of other elegant alternatives that will let you distinguish between "deleted" and "out-of-sync". So I'm ok with forwarding the "cause". It is harder to understand it at first. I was almost to question you on how we get "deleted" as the cause, but later I figured out by doing global search.

@core-ai-bot
Copy link
Member Author

Comment by DennisKehrig
Wednesday Oct 10, 2012 at 15:52 GMT


Thanks a lot for your review, much appreciated! I updated the comments as per your suggestions. Unless I overlooked anything, that's it from me for now.

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Wednesday Oct 10, 2012 at 16:08 GMT


Looks good. Merging

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