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

Externalize strings from Extension Manager view item template #3483

Merged
merged 3 commits into from
Apr 19, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 42 additions & 2 deletions src/extensibility/ExtensionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/

/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
/*global define, window, $, brackets */
/*global define, window, $, brackets, semver */
/*unittests: ExtensionManager*/

/**
Expand All @@ -42,6 +42,9 @@ define(function (require, exports, module) {
NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem,
ExtensionLoader = require("utils/ExtensionLoader");

// semver isn't a proper AMD module, so it will just load into the global namespace.
require("extensibility/node/node_modules/semver/semver");

/**
* Extension status constants.
*/
Expand Down Expand Up @@ -85,7 +88,11 @@ define(function (require, exports, module) {
*/
function getRegistry(forceDownload) {
if (!_registry || forceDownload) {
return $.getJSON(brackets.config.extension_registry, {cache: false})
return $.ajax({
url: brackets.config.extension_registry,
dataType: "json",
cache: false
})
.done(function (data) {
_registry = data;
});
Expand Down Expand Up @@ -155,12 +162,45 @@ define(function (require, exports, module) {
return (_extensions[id] && _extensions[id].status) || NOT_INSTALLED;
}

/**
* Returns information about whether the given entry is compatible with the given Brackets API version.
* @param {Object} entry The registry entry to check.
* @param {string} apiVersion The Brackets API version to check against.
* @return {{isCompatible: boolean, requiresNewer}} Result contains an
* "isCompatible" member saying whether it's compatible. If not compatible, then
* "requiresNewer" says whether it requires an older or newer version of Brackets.
*/
function getCompatibilityInfo(entry, apiVersion) {
var requiredVersion = entry.metadata.engines && entry.metadata.engines.brackets,
result = {};
result.isCompatible = !requiredVersion || semver.satisfies(apiVersion, requiredVersion);
if (!result.isCompatible) {
if (requiredVersion.charAt(0) === '<') {
result.requiresNewer = false;
} else if (requiredVersion.charAt(0) === '>') {
result.requiresNewer = true;
} else if (requiredVersion.charAt(0) === "~") {
var compareVersion = requiredVersion.slice(1);
// Need to add .0s to this style of range in order to compare (since valid version
// numbers must have major/minor/patch).
if (compareVersion.match(/^[0-9]+$/)) {
compareVersion += ".0.0";
} else if (compareVersion.match(/^[0-9]+\.[0-9]+$/)) {
compareVersion += ".0";
}
result.requiresNewer = semver.lt(apiVersion, compareVersion);
}
}
return result;
}

// Listen to extension load events
$(ExtensionLoader).on("load", _handleExtensionLoad);

// Public exports
exports.getRegistry = getRegistry;
exports.getStatus = getStatus;
exports.getCompatibilityInfo = getCompatibilityInfo;

exports.NOT_INSTALLED = NOT_INSTALLED;
exports.ENABLED = ENABLED;
Expand Down
21 changes: 14 additions & 7 deletions src/extensibility/ExtensionManagerView.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/

/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
/*global define, window, $, brackets, Mustache, semver */
/*global define, window, $, brackets, Mustache */
/*unittests: ExtensionManager*/

define(function (require, exports, module) {
Expand All @@ -36,12 +36,11 @@ define(function (require, exports, module) {
registry_utils = require("extensibility/registry_utils"),
itemTemplate = require("text!htmlContent/extension-manager-view-item.html");

// semver isn't a proper AMD module, so it will just load into the global namespace.
require("extensibility/node/node_modules/semver/semver");

/**
* @constructor
* Creates a view enabling the user to install and manage extensions.
* Events:
* "render": whenever the view fully renders itself.
*/
function ExtensionManagerView() {
var self = this;
Expand Down Expand Up @@ -81,7 +80,7 @@ define(function (require, exports, module) {
// Show the busy spinner and access the registry.
var $spinner = $("<div class='spinner large spin'/>")
.appendTo(this.$el);
ExtensionManager.getRegistry().done(function (registry) {
ExtensionManager.getRegistry(true).done(function (registry) {
// Display the registry view.
self._render(registry_utils.sortRegistry(registry));
}).fail(function () {
Expand Down Expand Up @@ -116,10 +115,17 @@ define(function (require, exports, module) {
// Create a Mustache context object containing the entry data and our helper functions.
var context = $.extend({}, entry),
status = ExtensionManager.getStatus(entry.metadata.name);

// Normally we would merge the strings into the context we're passing into the template,
// but since we're instantiating the template for every item, it seems wrong to take the hit
// of copying all the strings into the context, so we just make it a subfield.
context.Strings = Strings;
Copy link
Member

Choose a reason for hiding this comment

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

Seems fine


context.isInstalled = (status === ExtensionManager.ENABLED);

var requiredVersion = entry.metadata.engines && entry.metadata.engines.brackets;
context.isCompatible = !requiredVersion || semver.satisfies(brackets.metadata.apiVersion, requiredVersion);
var compatInfo = ExtensionManager.getCompatibilityInfo(entry, brackets.metadata.apiVersion);
context.isCompatible = compatInfo.isCompatible;
context.requiresNewer = compatInfo.requiresNewer;

context.allowInstall = context.isCompatible && !context.isInstalled;

Expand All @@ -146,6 +152,7 @@ define(function (require, exports, module) {
$item.appendTo($table);
});
$table.appendTo(this.$el);
$(this).triggerHandler("render");
};

/**
Expand Down
19 changes: 11 additions & 8 deletions src/htmlContent/extension-manager-view-item.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,30 @@
<span class="ext-name">{{#metadata.title}}{{metadata.title}}{{/metadata.title}}{{^metadata.title}}{{metadata.name}}{{/metadata.title}}</span>
<span class="muted ext-version">{{metadata.version}}</span><br/>
<span class="muted ext-author">
by
{{Strings.EXTENSION_AUTHOR}}:
Copy link
Member

Choose a reason for hiding this comment

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

Would we want to keep our options open by including "by" in the translation?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think it's really necessary...it's probably going to be pretty obvious from context that these are the author/date even if we don't use any labels at all.

{{#metadata.author.name}}
{{metadata.author.name}} /
{{/metadata.author.name}}
<a href="{{ownerLink}}">{{formatUserId}}</a>
</span>
<span class="muted ext-date">on {{lastVersionDate}}</span>
</span><br/>
<span class="muted ext-date">{{Strings.EXTENSION_DATE}}: {{lastVersionDate}}</span>
Copy link
Member

Choose a reason for hiding this comment

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

How are dates getting formatted? Other than the update notifications which use a hard-coded date format (distinct per locale), we haven't done any locale specific dynamic formatting.

Copy link
Author

Choose a reason for hiding this comment

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

I'm just using the UTC date format for now, which is pretty unambiguous (YYYY-MM-DD). We can make this more sophisticated later.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. I'll file a bug.

</td>
<td class="ext-desc">
{{^isCompatible}}
<div class="alert-message warning">This extension is incompatible with this version of Brackets.</div>
<div class="alert-message warning">
{{#requiresNewer}}{{Strings.EXTENSION_INCOMPATIBLE_NEWER}}{{/requiresNewer}}
{{^requiresNewer}}{{Strings.EXTENSION_INCOMPATIBLE_OLDER}}{{/requiresNewer}}
</div>
{{/isCompatible}}
{{#metadata.description}}
{{metadata.description}}
{{/metadata.description}}
{{^metadata.description}}
<span class="muted"><em>No description</em></span>
<span class="muted"><em>{{Strings.EXTENSION_NO_DESCRIPTION}}</em></span>
{{/metadata.description}}
{{#metadata.keywords.length}}
<br/>
<span class="ext-keywords">Keywords:
<span class="ext-keywords">{{Strings.EXTENSION_KEYWORDS}}:
{{#metadata.keywords}}
{{.}}
{{/metadata.keywords}}
Expand All @@ -32,8 +35,8 @@
</td>
<td>
<button class="btn install" data-extension-id="{{metadata.name}}" {{^allowInstall}}disabled{{/allowInstall}}>
{{^isInstalled}}Install{{/isInstalled}}
{{#isInstalled}}Installed{{/isInstalled}}
{{^isInstalled}}{{Strings.INSTALL}}{{/isInstalled}}
{{#isInstalled}}{{Strings.EXTENSION_INSTALLED}}{{/isInstalled}}
</button>
</td>
</tr>
12 changes: 10 additions & 2 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ define({
"INVALID_VERSION_NUMBER" : "The package version number ({0}) is invalid.",
"INVALID_BRACKETS_VERSION" : "The Brackets compatibility string {{0}} is invalid.",
"DISALLOWED_WORDS" : "The words {{1}} are not allowed in the {{0}} field.",
"API_NOT_COMPATIBLE" : "The extension isn't compatible with this version of Brackets. It's installed in your disabled extensions folder.",
"API_NOT_COMPATIBLE" : "The extension isn't compatible with this version of {APP_NAME}. It's installed in your disabled extensions folder.",
"MISSING_MAIN" : "The package has no main.js file.",
"ALREADY_INSTALLED" : "An extension with the same name was already installed. The new extension is installed in your disabled extensions folder.",
"NO_DISABLED_DIRECTORY" : "Cannot save extension to extensions/disabled because the folder does not exist.",
Expand All @@ -337,8 +337,16 @@ define({
"UNKNOWN_ERROR" : "Unknown internal error.",
// For NOT_FOUND_ERR, see generic strings above
"EXTENSION_MANAGER_TITLE" : "Extension Manager",
"EXTENSION_MANAGER_ERROR_LOAD" : "Unable to access the Brackets extension registry. Please try again later.",
"EXTENSION_MANAGER_ERROR_LOAD" : "Unable to access the extension registry. Please try again later.",
"INSTALL_FROM_URL" : "Install from URL\u2026",
"EXTENSION_AUTHOR" : "Author",
"EXTENSION_DATE" : "Date",
"EXTENSION_INCOMPATIBLE_NEWER" : "This extension requires a newer version of {APP_NAME}.",
"EXTENSION_INCOMPATIBLE_OLDER" : "This extension currently only works with older versions of {APP_NAME}.",
"EXTENSION_NO_DESCRIPTION" : "No description",
"EXTENSION_KEYWORDS" : "Keywords",
"EXTENSION_INSTALLED" : "Installed",
"EXTENSION_SEARCH_PLACEHOLDER" : "Search",

// extensions/default/JSLint
"JSLINT_ERRORS" : "JSLint Errors",
Expand Down
41 changes: 36 additions & 5 deletions test/spec/ExtensionManager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,21 +224,52 @@ define(function (require, exports, module) {
expect(spy).toHaveBeenCalledWith(jasmine.any(Object), mockPath + "/mock-legacy-extension", ExtensionManager.ENABLED);
});
});

it("should calculate compatibility info correctly", function () {
function fakeEntry(version) {
return { metadata: { engines: { brackets: version } } };
}

expect(ExtensionManager.getCompatibilityInfo(fakeEntry(null), "1.0.0"))
.toEqual({isCompatible: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry(">0.5.0"), "0.6.0"))
.toEqual({isCompatible: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry(">0.6.0"), "0.6.0"))
.toEqual({isCompatible: false, requiresNewer: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry(">0.7.0"), "0.6.0"))
.toEqual({isCompatible: false, requiresNewer: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("<0.5.0"), "0.4.0"))
.toEqual({isCompatible: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("<0.4.0"), "0.4.0"))
.toEqual({isCompatible: false, requiresNewer: false});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("<0.3.0"), "0.4.0"))
.toEqual({isCompatible: false, requiresNewer: false});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("~1.2"), "1.2.0"))
.toEqual({isCompatible: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("~1.2"), "1.2.1"))
.toEqual({isCompatible: true});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("~1.2"), "1.3.0"))
.toEqual({isCompatible: false, requiresNewer: false});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("~1.2"), "1.3.1"))
.toEqual({isCompatible: false, requiresNewer: false});
expect(ExtensionManager.getCompatibilityInfo(fakeEntry("~1.2"), "1.1.0"))
.toEqual({isCompatible: false, requiresNewer: true});
});
});

describe("ExtensionManagerView", function () {
var testWindow, view, fakeLoadDeferred, installerDeferred, mockInstalledExtensions;

// Sets up a real registry (with mock data).
function setupRegistryWithMockLoad() {
// Prefetch the model so the view is constructed immediately. (mockjax appears to
// add a little asynchronicity even if it's returning data right away.)
runs(function () {
waitsForDone(ExtensionManager.getRegistry());
});
var rendered = false;
runs(function () {
view = new ExtensionManagerView();
$(view).on("render", function () {
rendered = true;
});
});
waitsFor(function () { return rendered; }, "view rendering");
}

// Sets up a mock registry (with no data).
Expand Down