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

Added search to jslint error #3304

Closed
wants to merge 1 commit into from
Closed

Added search to jslint error #3304

Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions src/extensions/default/JSLint/htmlContent/results-table.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<td class="line" data-character="{{character}}">{{line}}</td>
<td>{{reason}}</td>
<td>{{evidence}}</td>
<td><a class="lint-url" data-reason="{{reason}}" href="#">Search</a></td>
Copy link
Contributor

Choose a reason for hiding this comment

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

You should move the search string to nls/root/string.js with the other JSLint strings and use it here by the key name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. Thanks!

</tr>
{{/reportList}}
</tbody>
Expand Down
9 changes: 9 additions & 0 deletions src/extensions/default/JSLint/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ define(function (require, exports, module) {
Resizer = brackets.getModule("utils/Resizer"),
ExtensionUtils = brackets.getModule("utils/ExtensionUtils"),
StatusBar = brackets.getModule("widgets/StatusBar"),
NativeApp = brackets.getModule("utils/NativeApp"),
JSLintTemplate = require("text!htmlContent/bottom-panel.html"),
ResultsTemplate = require("text!htmlContent/results-table.html");

Expand Down Expand Up @@ -153,6 +154,14 @@ define(function (require, exports, module) {
EditorManager.focusEditor();
});

$lintResults.find(".lint-url")
.on("click", function (e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This works good like this, but if we want to avoid multiple event listeners and use delegation, you can just move it to the click listener in the table, and add an if ($target.hasClass("lint-url")) where $target = $(e.target) which would be best to add as a variable before the first use inside this function since it will be used more than once now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

Copy link
Contributor

Choose a reason for hiding this comment

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

You can also do the following:

$lintResults.on("click", ".lint-url", function (e) { ... })

This is somewhat of a shortcut for Tom's suggestion, as it creates a single event handler that is designated to url handling and safe towards content changes. Additionally, you can keep the url handling separate from the generic click handling, which I think is more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I didn't knew you could do that and I like that idea better.

var reason = $(this).data("reason");
var url = "https://www.google.com/search?q=" + encodeURIComponent(reason);
NativeApp.openURLInDefaultBrowser(url);
return false;
});

$lintResults.show();
if (JSLINT.errors.length === 1) {
StatusBar.updateIndicator(INDICATOR_ID, true, "jslint-errors", Strings.JSLINT_ERROR_INFORMATION);
Expand Down