Preferences are being erroneously saved in the path layer

[redmunds]

When I change a Theme, it is only applied to the current project. But, after I shutdown and restart Brackets, then it is applied to all projects.

[redmunds]

I played around with this some more, and the theme is not necessarily applied to all projects. I can't find any pattern. How is it supposed to work?

Maybe problem is with Dialog changing theme, but not changing preference when I click Cancel button? See #8390.

[MiguelCastillo]

@redmunds So how is works is that as soon as a theme is selected it is applied and the theme is saved as a preference. I am not sure how a theme could just be applied to only the current project...

Please let me know if you come up with some steps to reproduce the issue.

[redmunds]

I am getting intermittent results on Win7 at the moment. Someone on IRC earlier today was seeing same issue.

I see the following exception in console the first time I switch projects after changing the Theme, so maybe it's related to this:

Recursive tests with the same name are not supported. Timer name: readAsText:   C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/extensions/default/ThorDarkTheme/main.less ErrorNotification.js:117
window.console.error ErrorNotification.js:117
_markStart PerfUtils.js:116
markStart PerfUtils.js:150
readAsText FileUtils.js:56
(anonymous function) ThemeManager.js:245
Nt lodash.js:21
loadCurrentThemes ThemeManager.js:243
refresh ThemeManager.js:278
(anonymous function) ThemeManager.js:398
o.event.dispatch jquery-2.1.0.min.js:3
r.handle jquery-2.1.0.min.js:3
o.event.trigger jquery-2.1.0.min.js:3
(anonymous function) jquery-2.1.0.min.js:3
o.extend.each jquery-2.1.0.min.js:2
o.fn.o.each jquery-2.1.0.min.js:2
o.fn.extend.trigger jquery-2.1.0.min.js:3
(anonymous function) PreferencesBase.js:1095
(anonymous function) PreferencesBase.js:1102
o.event.dispatch jquery-2.1.0.min.js:3
r.handle jquery-2.1.0.min.js:3
o.event.trigger jquery-2.1.0.min.js:3
(anonymous function) jquery-2.1.0.min.js:3
o.extend.each jquery-2.1.0.min.js:2
o.fn.o.each jquery-2.1.0.min.js:2
o.fn.extend.trigger jquery-2.1.0.min.js:3
_.extend._triggerChange PreferencesBase.js:1708
_.extend._tryAddToScopeOrder PreferencesBase.js:1211
(anonymous function) PreferencesBase.js:1291
(anonymous function) jquery-2.1.0.min.js:2
j jquery-2.1.0.min.js:2
k.add jquery-2.1.0.min.js:2
(anonymous function) jquery-2.1.0.min.js:2
o.extend.each jquery-2.1.0.min.js:2
(anonymous function) jquery-2.1.0.min.js:2
o.extend.Deferred jquery-2.1.0.min.js:2
d.then jquery-2.1.0.min.js:2
_.extend._addToScopeOrder PreferencesBase.js:1289
_.extend.addToScopeOrder PreferencesBase.js:1322
_toggleProjectScope PreferencesManager.js:271
_setCurrentEditingFile PreferencesManager.js:301
(anonymous function) ProjectManager.js:1206
(anonymous function) FileSystemEntry.js:292
(anonymous function) AppshellFileSystem.js:267
(anonymous function) AppshellFileSystem.js:241
(anonymous function)

Note that this error means that you have unbalanced calls to PerfUtils methods markStart() and addMeasurement()/finalizeMeasurement().

[MiguelCastillo]

@redmunds You know what, I have seen that before... But it has never stopped themes from working. I will get on IRC to see what's up.

[redmunds]

@MiguelCastillo I removed my extensions and I can no longer reproduce. Closing.

[redmunds]

Re-opening. I found another piece of info that helps to explain why this is intermittent. I just looked into one of my project preference files (i.e. .brackets.json) and found this:

{
    "defaultExtension": "js",
    "path": {
        "*.htm": {
            "spaceUnits": 2
        },
        "*.html": {
            "spaceUnits": 2,
            "useTabChar": false,
            "brackets-themes.themes": [
                "thor-light-theme"
            ]
        },
        "*.xml": {
            "spaceUnits": 2
        },
        "*.php": {
            "spaceUnits": 2
        },
        "*.blade.php": {
            "spaceUnits": 4
        },
        "*.txt": {
            "spaceUnits": 2,
            "wordWrap": true
        },
        "*.md": {
            "closeBrackets": false
        }
    },
    "wordWrap": false,
    "spaceUnits": 4,
    "useTabChar": false,
    "smartIndent": true,
    "softTabs": true,
    "closeTags": {
        "whenOpening": true,
        "whenClosing": false
    }
}

"brackets-themes.themes" should not be set at the project scope, and also not at the path level (file scope). Currently, this means that I have a different Theme for HTML files than for all other files!

The reason this happens is that when you set a preference, by default it is updated at the current preference scope. Theme preferences need to be forced to be set at the user scope.

[redmunds]

@MiguelCastillo To be able to reproduce this, create a .brackets.json file with the contents above and put in the root a a project. Then open an html file and change the Theme. Then check Theme for non-html files in that project, and files in other projects.

[MiguelCastillo]

@redmunds Oh man, I am glad you pinned it down. I would have been spinning my wheels for a bit. So, the trick is to make sure the settings are saved as a user scope. So, I don't do anything special here. I just do prefs = PreferencesManager.getExtensionPrefs("brackets-themes"); . How would that be changed to user scope settings?

[redmunds]

The scope is specified on the set() method. I think this should work:

    PreferencesManager.set("brackets-themes", theme, { location: { scope: "user" } });

[peterflynn]

@redmunds This is how all preferences work, by design (CC @dangoor to confirm) -- changing a pref's value sets it at whatever level the value is currently coming from. Any particular reason we'd want to make themes behave differently from the rest?

It seems like there's a circular problem here:

1. You didn't expect the theme to get set at the project level. (You don't remember ever hand-editing the JSON file to do this, I assume).
2. The reason it was set at the project level is that the previous theme value was also set at the project level.
3. Why was the previous value set at the project level? You didn't expect that one to be there either... see step 1 and repeat. The 'infinite loop' doesn't stop until you reach a case where the last value wasn't set at the project level, but changing its value adds a setting there. And that's not explained by the standard prefs scoping behavior.

I'm also not 100% sure if the proposed fix would work. If that sets it at the user level, but your last theme remains set at the more specific project level, won't the last theme continue to display since the project level has higher precedence? You might need to set it to null first with no scope (to clear it at whatever level it was last set at) and then set it to the right value with user scope explicitly specified. (Plus some magic to avoid flicker in between?). But that still feels like a hack that doesn't necessarily get at the root bug -- how we wound up with an unexpected project-specific setting in the first place...

[redmunds]

Yes, I manually set up a project prefs file. Yes, I set up a paths prefs for *.html specific preferences. But, I never manually set a "brackets-themes" preference, so we need to figure out how that was set.

Maybe the Themes code is sharing a location for many preferences? There are several preferences in the Themes dialog -- just because I may have set one of them at the project level, that doesn't mean I want the Current Theme also set at the project level.

[MiguelCastillo]

@redmunds Let me ask you this. Were you using my themes extension before? Because I was using that preference key "brackets-themes". I changed the themes extension so that brackets would use that key. I wanted to allow the settings people had from the extension to carry forward, but this may have been a bad idea.

[redmunds]

I tried your extension a long time ago when it first came out, but haven't used it to change Themes recently. I may have randomly installed it for menu testing because I know it creates a new top-level menu, so it's possible there may have been a setting lying around.

[MiguelCastillo]

@redmunds @peterflynn Guys I am not sure what the next steps are on this one. I haven't seen this issue before.

[redmunds]

I agree that forcing scope to "user" is not the right solution, but something is not right about how the prefs in the Themes dialog are getting set. This also affects the font-size preference. I can reproduce this every time:

1. Create a .brackets.json file in a project root folder & give it the following contents:

   {
      "defaultExtension": "js",
      "path": {
          "*.html": {
              "spaceUnits": 2,
          },
      },
      "spaceUnits": 4,
      "useTabChar": false
   }

2. Use Debug > Open Preferences File to verify that "brackets-themes.fontSize" is not set in user (or any other) prefs file.
3. Open a .html file
4. Press Ctrl/Cmd-+ (or Themes dialog) to increase font-size

Results "brackets-themes.fontSize" pref is set in project prefs for only .html files.

Expected "brackets-themes.fontSize" pref is set in user prefs since it is not yet set in any preference file. This is how it works in Release 0.41.

cc @dangoor

[MiguelCastillo]

@redmunds Help me understand this. Themes preferences are not doing anything special. Is there something that themes should be doing or is this more of an issue in the preferences manager?

[redmunds]

Looks like the code is still using the var prefs = PreferencesManager.getExtensionPrefs() and prefs.set() methods even though it's now core code.

Instead you should use PreferencesManager.definePreference() to define the pref, then PreferencesManager.get() and PreferencesManager.set() to get/set values so PreferencesManager can figure out all the scope stuff for you.

[MiguelCastillo]

I just randomly ran into while testing another issues... I cannot reproduce it with your steps so I cannot verify that the changes you suggest fix the issue, even though they just seem correct.

[MiguelCastillo]

@redmunds I take it back... I randomly ran into the issue because I had just installed the themes extension another issue. And things started to act weird.

[dangoor]

Using getExtensionPrefs is legit, even for core code. All it does is ensure that all of the prefs you use have a consistent prefix.

@redmunds Based on this comment in which you show the theme settings appearing in the *.html path part of a project prefs, I don't think this is a problem with themes. The themes code is just calling an unqualified prefs.set which means that it should set the pref in the same place from which the pref value is coming from (or "user" level, if the pref is coming from defaults).

[dangoor]

I can reproduce this problem. I created a .brackets.json file in the Getting Started project with a *.html path and settings wound up in there (including brackets-git settings). This is some sort of preferences manager bug.

[MiguelCastillo]

@dangoor awesome you can reproduce... bad we got a bug :) So, is there something immediate I can do to address this issue in themes?

[dangoor]

@MiguelCastillo From what I can see, Themes is off the hook. This is a preferences system problem.

[MiguelCastillo]

@dangoor sounds good