Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Handle undefined theme the right way #10243

Merged
merged 1 commit into from
Dec 20, 2014
Merged

Handle undefined theme the right way #10243

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/view/ThemeManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,8 @@ define(function (require, exports, module) {
var cm = editor._codeMirror;
ThemeView.updateThemes(cm);

// currentTheme can be undefined if you reloaded without extensions
if (currentTheme && currentTheme.addModeClass !== undefined) {
cm.setOption("addModeClass", currentTheme.addModeClass);
}
// currentTheme can be undefined, so watch out
cm.setOption("addModeClass", !!(currentTheme && currentTheme.addModeClass));
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcelgerber good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually already fixed in #10236, but this PR fixes an edgy case where currentTheme got undefined after changing from a addModeClass-theme to the default theme.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I meant that the issue you are fixing with your changes below is a good find :) It is an edge case that only really happens when you uninstall a theme.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if currentTheme should never be undefined. Since we have two default themes, I wouldn't necessarily expect this behavior. I assume that I'm missing an important detail, but it's not fully clear to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current theme is undefined when a theme is uninstalled. At that point the current logic will use the default theme (light-theme). So if the theme is installed again, that theme will then continue to be the current theme.

}

Expand Down