Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some JSHint fixes #458

Merged
merged 6 commits into from
Dec 2, 2014
Merged

Some JSHint fixes #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ba7c0c0
Some JSHint fixes;
jerone Nov 28, 2014
dfb3dfc
Removed Ambiguous comment;
jerone Nov 29, 2014
207e147
Revert all `user._id === user._id` changes;
jerone Dec 1, 2014
2e9779d
Merge branch 'master' of https://github.com/OpenUserJs/OpenUserJS.org…
jerone Dec 1, 2014
a9668ad
Merge branch 'master' of https://github.com/OpenUserJs/OpenUserJS.org…
jerone Dec 1, 2014
59a68a0
Revert all `===` comparison with db models;
jerone Dec 2, 2014
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: 0 additions & 1 deletion app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ var passport = require('passport');

var app = express();

var statusCodePage = require('./libs/templateHelpers').statusCodePage;
var modifySessions = require('./libs/modifySessions');

var settings = require('./models/settings.json');
Expand Down
4 changes: 0 additions & 4 deletions controllers/admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ var userRoles = require('../models/userRoles.json');
var strategies = require('./strategies.json');
var loadPassport = require('../libs/passportLoader').loadPassport;
var strategyInstances = require('../libs/passportLoader').strategyInstances;
var scriptStorage = require('./scriptStorage');
var modelParser = require('../libs/modelParser');
var helpers = require('../libs/helpers');
var statusCodePage = require('../libs/templateHelpers').statusCodePage;
Expand Down Expand Up @@ -66,7 +65,6 @@ exports.userAdmin = function (aReq, aRes, aNext) {

// You can only see users with a role less than yours
User.find({ role: { $gt: authedUser.role } }, function (aErr, aUsers) {
var i = 0;
options.users = [];

aUsers.forEach(function (aUser) {
Expand Down Expand Up @@ -168,7 +166,6 @@ exports.adminUserUpdate = function (aReq, aRes, aNext) {

//
var options = {};
var tasks = [];

// Session
authedUser = options.authedUser = modelParser.parseUser(authedUser);
Expand Down Expand Up @@ -428,7 +425,6 @@ exports.authAsUser = function (aReq, aRes, aNext) {

//
var options = {};
var tasks = [];

// Session
authedUser = options.authedUser = modelParser.parseUser(authedUser);
Expand Down
3 changes: 1 addition & 2 deletions controllers/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ var loadPassport = require('../libs/passportLoader').loadPassport;
var strategyInstances = require('../libs/passportLoader').strategyInstances;
var Strategy = require('../models/strategy.js').Strategy;
var User = require('../models/user').User;
var userRoles = require('../models/userRoles.json');
var verifyPassport = require('../libs/passportVerify').verify;
var cleanFilename = require('../libs/helpers').cleanFilename;
var addSession = require('../libs/modifySessions').add;
Expand Down Expand Up @@ -122,7 +121,7 @@ exports.callback = function (aReq, aRes, aNext) {
if (openIdStrategies[strategy]) {
strategyInstance._verify = function (aId, aDone) {
verifyPassport(aId, strategy, username, aReq.session.user, aDone);
}
};
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

} else {
strategyInstance._verify =
function (aToken, aRefreshOrSecretToken, aProfile, aDone) {
Expand Down
1 change: 0 additions & 1 deletion controllers/discussion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,6 @@ exports.createComment = function (aReq, aRes, aNext) {
var topic = aReq.params.topic;
var authedUser = aReq.session.user;
var content = aReq.body['comment-content'];
var commentId = aReq.body['comment-id']; // for editing

if (!authedUser) { return aNext(); }

Expand Down
1 change: 0 additions & 1 deletion controllers/group.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ exports.list = function (aReq, aRes) {

var setupGroupSidePanel = function (aOptions) {
// Shortcuts
var group = aOptions.group;
var authedUser = aOptions.authedUser;

// Mod
Expand Down
4 changes: 2 additions & 2 deletions controllers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ exports.home = function (aReq, aRes) {
pageMetadata(options, ['Flagged Scripts', 'Moderation']);
}
}
};
}
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

function render() { aRes.render('pages/scriptListPage', options); }
function asyncComplete() { preRender(); render(); }
async.parallel(tasks, asyncComplete);
Expand Down Expand Up @@ -217,7 +217,7 @@ exports.register = function (aReq, aRes) {
var githubStrategy = _.findWhere(options.strategies, { strat: 'github' });
if (githubStrategy)
githubStrategy.selected = true;
};
}
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

function render() { aRes.render('pages/loginPage', options); }
function asyncComplete() { preRender(); render(); }
async.parallel(tasks, asyncComplete);
Expand Down
2 changes: 1 addition & 1 deletion controllers/issue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ exports.open = function (aReq, aRes, aNext) {
function preRender() {
// Page metadata
pageMetadata(options, ['New Issue', script.name]);
};
}
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

function render() { aRes.render('pages/scriptNewIssuePage', options); }
function asyncComplete() { preRender(); render(); }

Expand Down
1 change: 0 additions & 1 deletion controllers/moderation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ exports.removedItemPage = function (aReq, aRes, aNext) {

//
var options = {};
var tasks = [];

// Session
authedUser = options.authedUser = modelParser.parseUser(authedUser);
Expand Down
9 changes: 0 additions & 9 deletions controllers/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ var isDev = require('../libs/debug').isDev;
var isDbg = require('../libs/debug').isDbg;

//
var fs = require('fs');
var formidable = require('formidable');
var async = require('async');
var _ = require('underscore');
var sanitizeHtml = require('sanitize-html');
Expand Down Expand Up @@ -266,7 +264,6 @@ var getScriptPageTasks = function (aOptions) {

var setupScriptSidePanel = function (aOptions) {
// Shortcuts
var script = aOptions.script;
var authedUser = aOptions.authedUser;

// User
Expand All @@ -292,8 +289,6 @@ exports.view = function (aReq, aRes, aNext) {
var authedUser = aReq.session.user;

var installNameSlug = scriptStorage.getInstallName(aReq);
var scriptAuthor = aReq.params.username;
var scriptNameSlug = aReq.params.scriptname;
var isLib = aReq.params.isLib;

Script.findOne({
Expand Down Expand Up @@ -355,8 +350,6 @@ exports.edit = function (aReq, aRes, aNext) {
aReq.params.username = authedUser.name.toLowerCase();

var installNameSlug = scriptStorage.getInstallName(aReq);
var scriptAuthor = aReq.params.username;
var scriptNameSlug = aReq.params.scriptname;
var isLib = aReq.params.isLib;

Script.findOne({
Expand Down Expand Up @@ -398,8 +391,6 @@ exports.edit = function (aReq, aRes, aNext) {
options.searchBarPlaceholder = modelQuery.scriptListQueryDefaults.searchBarPlaceholder;
options.searchBarFormAction = modelQuery.scriptListQueryDefaults.searchBarFormAction;

var baseUrl = script && script.isLib ? '/libs/' : '/scripts/';

if (aReq.body.remove) {
// POST
scriptStorage.deleteScript(aScriptData.installName, function () {
Expand Down
2 changes: 0 additions & 2 deletions controllers/scriptStorage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ exports.getSource = function (aReq, aCallback) {

exports.sendScript = function (aReq, aRes, aNext) {
var accept = aReq.headers.accept;
var installName = null;

if (0 !== aReq.url.indexOf('/libs/') && accept === 'text/x-userscript-meta') {
return exports.sendMeta(aReq, aRes, aNext);
Expand Down Expand Up @@ -234,7 +233,6 @@ exports.getMeta = function (aChunks, aCallback) {
// get the user script header.
var str = '';
var i = 0;
var len = aChunks.length;
var header = null;

for (; i < aChunks.length; ++i) {
Copy link
Member

Choose a reason for hiding this comment

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

Just as a FYI I see an incomplete optimization from someone here which is why JSHint plucked line 237 out.

Expand Down
19 changes: 5 additions & 14 deletions controllers/user.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ var _ = require('underscore');
var util = require('util');

var Comment = require('../models/comment').Comment;
var Flag = require('../models/flag').Flag;
var Script = require('../models/script').Script;
var Strategy = require('../models/strategy').Strategy;
var User = require('../models/user').User;
Expand Down Expand Up @@ -97,7 +96,6 @@ var getUserPageTasks = function (aOptions) {

// Shortcuts
var user = aOptions.user;
var authedUser = aOptions.authedUser;

//--- Tasks

Expand Down Expand Up @@ -218,7 +216,7 @@ exports.userListPage = function (aReq, aRes, aNext) {
}
}
function render() { aRes.render('pages/userListPage', options); }
function asyncComplete(err) { if (err) { return aNext(); } else { preRender(); render(); } };
function asyncComplete(err) { if (err) { return aNext(); } else { preRender(); render(); } }
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

async.parallel(tasks, asyncComplete);
};

Expand Down Expand Up @@ -263,9 +261,6 @@ exports.view = function (aReq, aRes, aNext) {
// scriptListQuery: Defaults
modelQuery.applyScriptListQueryDefaults(scriptListQuery, options, aReq);

// scriptListQuery: Pagination
var pagination = options.pagination; // is set in modelQuery.apply___ListQueryDefaults

//--- Tasks

// UserPage tasks
Expand Down Expand Up @@ -461,8 +456,6 @@ exports.userScriptListPage = function (aReq, aRes, aNext) {
exports.userEditProfilePage = function (aReq, aRes, aNext) {
var authedUser = aReq.session.user;

var username = aReq.params.username;

User.findOne({
_id: authedUser._id
}, function (aErr, aUserData) {
Expand Down Expand Up @@ -858,7 +851,6 @@ exports.userGitHubImportScriptPage = function (aReq, aRes, aNext) {

//
var options = {};
var tasks = [];

// Session
authedUser = options.authedUser = modelParser.parseUser(authedUser);
Expand Down Expand Up @@ -1001,7 +993,7 @@ exports.userGitHubRepoPage = function (aReq, aRes, aNext) {
options.repo = aRepo;
options.repoAsEncoded = {
default_branch: encodeURI(options.repo.default_branch)
}
};
Copy link
Member

Choose a reason for hiding this comment

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

+1 here


github.gitdata.getJavascriptBlobs({
user: encodeURIComponent(aRepo.owner.login),
Expand All @@ -1025,7 +1017,7 @@ exports.userGitHubRepoPage = function (aReq, aRes, aNext) {

aCallback(null);
},
], aCallback)
], aCallback);
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

});

//---
Expand Down Expand Up @@ -1092,7 +1084,6 @@ exports.userManageGitHubPage = function (aReq, aRes, aNext) {
pageMetadata(options, ['Manage', 'GitHub']);

//
var TOO_MANY_SCRIPTS = 'GitHub user has too many scripts to batch import.';
tasks.push(function (aCallback) {
var githubUserName = aReq.query.user || authedUser.ghUsername;

Expand All @@ -1111,7 +1102,7 @@ exports.userManageGitHubPage = function (aReq, aRes, aNext) {
},
function (aGithubUser, aCallback) {
options.githubUser = aGithubUser;
console.log(githubUser);
console.log(aGithubUser);
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

User.findOne({
_id: authedUser._id
}, aCallback);
Expand Down Expand Up @@ -1148,7 +1139,7 @@ exports.userManageGitHubPage = function (aReq, aRes, aNext) {
console.log(aReq.body);
_.each(aReq.body, function (aRepo, aReponame) {
// Load all scripts in the repo
if (typeof aRepo === 'string' && reponame.substr(-4) === '_all') {
if (typeof aRepo === 'string' && aReponame.substr(-4) === '_all') {
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

Copy link
Member

Choose a reason for hiding this comment

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

Would be interesting to know how this works... I don't believe I've seen this but we've all come a long way since then.

Copy link
Member

Choose a reason for hiding this comment

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

@Zren
So for clarity sake since I've never seen this before yesterday... if someone names a repo starting out with "_all" this will allegedly import everything in that GH repo including libraries and userscripts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've never used this before too, but you can access that page at https://openuserjs.org/users/marti/github

Copy link
Member

Choose a reason for hiding this comment

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

Interesting and thanks Jerone... might be useful to have that listed at https://openuserjs.org/user/add/scripts so it's not an orphaned route. Since this is fixing this it might be useful to have an issue with this in it. e.g. #468

Copy link
Contributor

Choose a reason for hiding this comment

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

No because the logic behind that route is awful. It can go through hundreds of github api requests (Edit: which times out if the user has a lot of repos with logs of files/folders) and doesn't list all repositories if the users list of repos is paginated.

The route should be removed since I rewrote it to handle pagination and the like but sizzle wanted it left there. Rather than confusing abuser with 2 ways to

Copy link
Contributor

Choose a reason for hiding this comment

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

Import from github, I only linked to the less resource intensive method with pagination and left the old route unlinked to in case sizzle wanted to use it to import all scripts in a repo for testing.

aReponame = aRepo;
aRepo = aRepos[aReponame];

Expand Down
1 change: 0 additions & 1 deletion libs/collectiveRating.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ exports.getRating = getRating;
// expiring cache (either memory or DB based) to
// speed up voting and flagging
exports.getKarma = function (aUser, aMaxKarma, aCallback) {
var ratings = [];
var karma = 0;
Script.find({ _authorId: aUser._id }, 'rating', function (aErr, aScripts) {
if (aErr) { return aCallback(karma); }
Expand Down
4 changes: 2 additions & 2 deletions libs/githubClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var githubGitDataGetBlobAsUtf8 = function (aMsg, aCallback) {
},
function (aBlob, aCallback) {
var content = aBlob.content;
if (aBlob.encoding == 'base64') {
if (aBlob.encoding === 'base64') {
var buf = new Buffer(content, 'base64');
content = buf.toString('utf8');
}
Expand All @@ -69,7 +69,7 @@ var githubUserContentGetBlobAsUtf8 = function (aMsg, aCallback) {
request.get(url, aCallback);
},
function (aResponse, aBody, aCallback) {
if (aResponse.statusCode != 200)
if (aResponse.statusCode !== 200)
Copy link
Member

Choose a reason for hiding this comment

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

+1 here since it's not a string.

return aCallback(util.format('Status Code %s', aResponse.statusCode));

aCallback(null, aBody);
Expand Down
3 changes: 1 addition & 2 deletions libs/modelParser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ var Script = require('../models/script').Script;

var userRoles = require('../models/userRoles.json');
var renderMd = require('../libs/markdown').renderMd;
var helpers = require('../libs/helpers');
var getRating = require('../libs/collectiveRating').getRating;
var cleanFilename = require('../libs/helpers').cleanFilename;

Expand Down Expand Up @@ -396,7 +395,7 @@ var canUserPostTopicToCategory = function (aUser, aCategory) {
return false; // Not logged in.

// Check if this category requires a minimum role to post topics.
console.log(aCategory.roleReqToPostTopic, _.isNumber(aCategory.roleReqToPostTopic), aUser.role, aUser.role <= aCategory.roleReqToPostTopic)
console.log(aCategory.roleReqToPostTopic, _.isNumber(aCategory.roleReqToPostTopic), aUser.role, aUser.role <= aCategory.roleReqToPostTopic);
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

if (_.isNumber(aCategory.roleReqToPostTopic)) {
return aUser.role <= aCategory.roleReqToPostTopic;
} else {
Expand Down
2 changes: 1 addition & 1 deletion libs/modifySessions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ exports.destroy = function (aReq, aUser, aCallback) {
}
};

if (!aUser || !aUser.sessionIds) { return aCb('No sessions', null); }
if (!aUser || !aUser.sessionIds) { return aCallback('No sessions', null); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably fixed a bug.

Copy link
Member

Choose a reason for hiding this comment

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

Probably +1 here. ;)


async.each(aUser.sessionIds, function (aId, aCb) {
store.set(aId, emptySess, aCb);
Expand Down
2 changes: 1 addition & 1 deletion libs/passportVerify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,4 @@ exports.verify = function (aId, aStrategy, aUsername, aLoggedIn, aDone) {
}
}
);
}
};
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

8 changes: 4 additions & 4 deletions libs/remove.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function removeable(aModel, aContent, aUser, aCallback) {
exports.removeable = removeable;

function remove(aModel, aContent, aUser, aReason, aCallback) {
var remove = new Remove({
var removeModel = new Remove({
Copy link
Member

Choose a reason for hiding this comment

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

-0+ here... JSHint specific I think.

'model': aModel.modelName,
'content': aContent.toObject(),
'removed': new Date(),
Expand All @@ -60,7 +60,7 @@ function remove(aModel, aContent, aUser, aReason, aCallback) {
'_removerId': aUser._id
});

remove.save(function (aErr, aRemove) {
removeModel.save(function (aErr, aRemove) {
Copy link
Member

Choose a reason for hiding this comment

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

-0+ here... JSHint specific I think.

aContent.remove(function (aErr) { aCallback(aRemove); });
});
}
Expand Down Expand Up @@ -112,7 +112,7 @@ exports.findDeadorAlive = function (aModel, aQuery, aUser, aCallback) {
var rmQuery = { model: modelName };

if (!aErr && aContent) { return aCallback(true, aContent, null); }
if (modelName != 'User' && -1 === modelNames.indexOf(modelName)) {
if (modelName !== 'User' && -1 === modelNames.indexOf(modelName)) {
return aCallback(null, null, null);
}

Expand All @@ -126,7 +126,7 @@ exports.findDeadorAlive = function (aModel, aQuery, aUser, aCallback) {
return aCallback(false, null, aRemoved);
}

aCallback(false, new model(aRemoved.content), aRemoved); // TODO: Ambiguous
aCallback(false, new aModel(aRemoved.content), aRemoved);
Copy link
Member

Choose a reason for hiding this comment

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

+1 here

});
});
};
6 changes: 1 addition & 5 deletions libs/repoManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ var isDbg = require('../libs/debug').isDbg;
//
var https = require('https');
var async = require('async');
var util = require('util');
var _ = require('underscore');

var Strategy = require('../models/strategy').Strategy;
var User = require('../models/user').User;

var nil = require('./helpers').nil;
var github = require('../libs/githubClient');
Expand All @@ -38,7 +36,7 @@ function fetchRaw(aHost, aPath, aCallback) {
var req = https.request(options,
function (aRes) {
var bufs = [];
if (aRes.statusCode != 200) { console.log(aRes.statusCode); return aCallback([new Buffer('')]); }
if (aRes.statusCode !== 200) { console.log(aRes.statusCode); return aCallback([new Buffer('')]); }
Copy link
Member

Choose a reason for hiding this comment

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

+1 here since it's not a string

else {
aRes.on('data', function (aD) { bufs.push(aD); }); // TODO: Non-descript function parm
aRes.on('end', function () {
Expand Down Expand Up @@ -102,7 +100,6 @@ RepoManager.prototype.loadScripts = function (aCallback, aUpdate) {
var scriptStorage = require('../controllers/scriptStorage');
var arrayOfRepos = this.makeRepoArray();
var that = this;
var scripts = [];

// TODO: remove usage of makeRepoArray since it causes redundant looping
arrayOfRepos.forEach(function (aRepo) {
Expand Down Expand Up @@ -161,7 +158,6 @@ Repo.prototype.fetchUserScripts = function (aCallback) {
// Looks for user script in the current directory
// and initiates searches on subdirectories
Repo.prototype.parseTree = function (aTree, aPath, aDone) {
var object;
var trees = [];
var that = this;
var repos = this.manager.repos;
Expand Down
Loading