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] Close Others extension improvements. #5049

Open
core-ai-bot opened this issue Aug 30, 2021 · 9 comments
Open

[CLOSED] Close Others extension improvements. #5049

core-ai-bot opened this issue Aug 30, 2021 · 9 comments

Comments

@core-ai-bot
Copy link
Member

Issue by sathyamoorthi
Saturday Oct 12, 2013 at 05:02 GMT
Originally opened as adobe/brackets#5497


PR adobe/brackets#4590 follow up.

1.Small fix added to retain Current Document after closing list of files.
2.Code added to dynamically add / remove context menus based working set file selection.

CC:@JeffryBooher


sathyamoorthi included the following code: https://github.com/adobe/brackets/pull/5497/commits

@core-ai-bot
Copy link
Member Author

Comment by sathyamoorthi
Monday Oct 21, 2013 at 10:58 GMT


@JeffryBooher Hi, I did the changes that you had mentioned. Does it need any other changes?

@core-ai-bot
Copy link
Member Author

Comment by iwehrman
Monday Oct 21, 2013 at 23:36 GMT


Drive-by comment, since it looks like you're cleaning up changes introduced with the Close Others extension: modifying the fullPath of a FileEntry, as in bd4018d55cd3c0efcf42c117d623243997bed696, is not good. In the future, this will be enforced and will cause an exception. I cleaned this up in a separate branch here bd967ffe9506e071f1d888253baff997fa01297c; you might want to incorporate something similar into master.

@core-ai-bot
Copy link
Member Author

Comment by sathyamoorthi
Tuesday Oct 22, 2013 at 06:39 GMT


@JeffryBooher Changed that. Sorry i failed to see it.

@iwehrman I did the change that you have proposed. But just curious, why do you think it would cause error? I think, if newFile had changes other than filePath, we will miss them. Is that a reason?

@core-ai-bot
Copy link
Member Author

Comment by iwehrman
Tuesday Oct 22, 2013 at 18:22 GMT


Yes, also other clients would like to be notified when file paths change, so just rewriting the path is not sufficient. Also some clients might maintain sets of files that are keyed by path, and changing the path of the underlying file without notifying them could cause problems. And, as I said, we're working on changes, which are coming soon, to the filesystem infrastructure such that rewriting the path will cause an explicit error.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Nov 01, 2013 at 01:26 GMT


We had ported some of these changes to the filesystem branch (to fix the code that modifies fullPath), and I think we may have found a bug in that part. To avoid conflicts, we should probably wait until that's sorted out and the filesystem stuff lands before merging this.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Nov 01, 2013 at 01:32 GMT


See fa8c824c for details.

@core-ai-bot
Copy link
Member Author

Comment by sathyamoorthi
Friday Nov 01, 2013 at 02:04 GMT


Sure. Let me know once you merge filesytem branch.

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Tuesday Nov 05, 2013 at 19:19 GMT


@sathyamoorthi you might want to merge off of that file system branch to see how things are progressing and to make your merge as seamless as possible. Closing this pull request until you've had a chance to look at the changes coming in the new File System API and you can re-open when it's ready.

@core-ai-bot
Copy link
Member Author

Comment by sathyamoorthi
Wednesday Nov 06, 2013 at 06:27 GMT


@JeffryBooher i gave simple PR adobe/brackets#5866 to fix #5800. Let me give other improvements in separate PR once they merge filesystem branch into core. So we can have minimum guarantee to solve that issue in next sprint.

What do you think?

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