Editor options set on hidden editors when switching files

[peterflynn]

1. Open a bunch of files in the Brackets source
2. Put a breakpoint in _setEditorOption() in the Editor module
3. Open codemirror.js

Result:
setOption() gets called on every Editor instance that exists, setting them all to use codemirror.js's tab settings. When to switch away from codemirror.js, setOption() gets called on all of them again to restore the setting (and also, mirroring this, the now-hidden codemirror.js is set to the 'wrong' setting).

This seems inefficient... and you could maybe even imagine some prefs where temporarily applying a different setting isn't 100% non-destructive.

Expected:
Only the visible editor (the editor to whose path the setting actually applies) gets the change event.
Or better yet, when the editor is new'ed up, it's constructed with the right path-appropriate settings to begin with and no change events are processed. (Though then we'd need a way to distinguish ignored change events due to path context from change events due to the user changing a setting, which we still couldn't ignore).

Low priority @dangoor as food for thought...

[dangoor]

Good points! Thanks for the suggestions

[TomMalbran]

I think the _instances.forEach is there because of the inline editors, since you need to change both the visible Editor and all the Inline Editors that point to the currently viewed file. But still better than doing it for all the editors.

[peterflynn]

@TomMalbran It's also needed because changing the preference via the statusbar affordance changes it globally, so any already-instantiated editors -- whether they're full-size or inline -- need to get updated. The new preferences system sends out events both when an actual setting has changed, and when the global "context" has changed causing different prefs to get swapped in/out. For Editors, it seems like those two cases need to get handled differently.

Btw, I just realized that split-views will make this a much more pressing bug, because then one of the editors that shouldn't get updated will still be in view. In certain cases, this could make the indentation jump around as the editor gains/loses focus...

[peterflynn]

Hmm, I actually just realized my "split-views" scenario with indent levels visibly jumping around can happen already in the case of inline editors. Nominating for Sprint 37 for that reason. Fyi @dangoor

[dangoor]

Bumping to medium priority.

[dangoor]

Making a note of a suggestion that Peter had for clearing up some cases that are a little off with the new preferences model:

As a quick strawman, I think these changes would address all of the issues above:
· When retrieving a preference, pass an argument indicating the context/scope. Values could be CURRENT_PROJECT, CURRENT_FILE, or a concrete specific file (e.g. an Editor is always bound to one specific file, and that context doesn’t change when it’s not the current file/Editor).
· If CURRENT_PROJECT is specified, the .brackets.json in the root of the project is used even if the currently open file is external to the project.

My thinking is to automatically set a "project" Scope that is the same object as the "path" scope. Requesting CURRENT_PROJECT values would change the scopeOrder to session, project, user, default.

[dangoor]

Something to note is that get() is synchronous. If you pass a file context to get and the preferences around that file have not yet been loaded, you wouldn't get the full information on the first call. It would make sense to also add context for events so that you can listen to preference changes for a specific file.

[dangoor]

One addition to the note above: in most common practice this will not be an issue. Preferences for the current project will generally be in hand already, but I thought it was worth mentioning. The impact would specifically be for files external to the project that do have a preference file in the file tree above them somewhere.

[redmunds]

FBNC back to @peterflynn