search

adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.26k stars 7.64k forks source link

ProjectManager.getAllFiles() doesn't usually honor includeWorkingSet=false (the supposed default) #8930

Open peterflynn opened 10 years ago

peterflynn commented 10 years ago

Invoking ProjectManager.getAllFiles() (or more explicitly, ProjectManager.getAllFiles(null, false)) often yields an array that includes files outside the project that are open in the working set. This happens any time someone has called ProjectManager.getAllFiles(true) earlier.

Most callers supposedly don't want working-set entries that are outside the project -- only Find in Files & Quick Open pass true. But since this bug has existed seemingly since Sprint 36 (a7a551d4f72092ac69b4aa2003a629072f2d0b03), those callers have been getting project-external items in the result list anyway. So presumably this doesn't cause any major ill effects for most callers.

This happens because getAllFiles() reuses a cached result, and when includeWorkingSet is true it modifies the cache in-place to include the extra files. It should make a copy instead to prevent tainting the cache used by future callers.

dangoor commented 10 years ago

Assigning to myself because I may have already fixed this in the project manager work...

peterflynn commented 9 years ago

Fwiw, looks like it's not fixed yet, since ProjectModel.getAllFiles() directly modifies the result array that _getAllFilesCache() hands back, affecting the value all future calls will see.

dangoor commented 9 years ago

Yeah, this is still on my list. Should be a trivial fix (just add a _.clone).