[CLOSED] Editor options set on hidden editors when switching files

[core-ai-bot]

Issue by peterflynn
Saturday Jan 18, 2014 at 01:58 GMT
Originally opened as adobe/brackets#6578

---

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

[core-ai-bot]

Comment by dangoor
Saturday Jan 18, 2014 at 02:39 GMT

---

Good points! Thanks for the suggestions

[core-ai-bot]

Comment by TomMalbran
Saturday Jan 18, 2014 at 04:40 GMT

---

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.

[core-ai-bot]

Comment by peterflynn
Monday Jan 20, 2014 at 22:42 GMT

---

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

[core-ai-bot]

Comment by peterflynn
Monday Jan 20, 2014 at 22:45 GMT

---

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

[core-ai-bot]

Comment by dangoor
Thursday Jan 23, 2014 at 16:26 GMT

---

Bumping to medium priority.

[core-ai-bot]

Comment by dangoor
Friday Jan 24, 2014 at 03:41 GMT

---

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.

[core-ai-bot]

Comment by dangoor
Friday Jan 31, 2014 at 14:56 GMT

---

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.

[core-ai-bot]

Comment by dangoor
Friday Jan 31, 2014 at 15:56 GMT

---

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.

[core-ai-bot]

Comment by redmunds
Friday Feb 14, 2014 at 21:30 GMT

---

FBNC back to@peterflynn