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

JSHint fixes (including indentation & whitespace); #406

Closed
wants to merge 3 commits into from
Closed

JSHint fixes (including indentation & whitespace); #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a04f0a
JSHint fixes (including indentation & whitespace);
jerone Nov 6, 2014
0adffb6
JSHint fixes (including indentation & whitespace);
jerone Nov 6, 2014
da64506
Merge branch 'master' of https://github.com/OpenUserJs/OpenUserJS.org…
jerone Nov 6, 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: 1 addition & 0 deletions .jshintignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**/ace/**.js
Copy link
Member

Choose a reason for hiding this comment

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

Not everyone uses JSHint... and I think Zren mentioned to you that this could be done in your .git folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow-up #97

Copy link
Member

Choose a reason for hiding this comment

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

#97 is currently stale (not to mention problematic) and you might notice I didn't include it in the #249 milestones. See #97 (comment).

1 change: 1 addition & 0 deletions .weignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**/ace/**.js
Copy link
Member

Choose a reason for hiding this comment

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

"we" ??... again not everyone uses whatever this is... and I think Zren mentioned to you that this could be done in your .git folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

7 changes: 3 additions & 4 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ app.use(express.methodOverride());
// Order is very important here (i.e mess with at your own risk)
app.use(express.cookieParser());
app.use(express.session({
secret: sessionSecret,
store: sessionStore
}));
secret: sessionSecret,
Copy link
Member

Choose a reason for hiding this comment

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

moot whitespace adjustment

store: sessionStore
}));
app.use(passport.initialize());
app.use(modifySessions.init(sessionStore));
app.use(app.router);
Expand All @@ -68,7 +68,6 @@ app.engine('html', require('./libs/muExpress').renderFile(app));
app.set('view engine', 'html');
app.set('views', __dirname + '/views');


// Setup minification
// Order is important here as Ace will fail with an invalid content encoding issue
if (process.env.NODE_ENV === 'production') {
Expand Down
8 changes: 4 additions & 4 deletions controllers/_template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ exports.example = function (aReq, aRes, aNext) {
var tasks = [];

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

Expand Down Expand Up @@ -57,7 +57,7 @@ exports.example = function (aReq, aRes, aNext) {
function preRender() {
// Pagination
options.paginationRendered = pagination.renderDefault(aReq);
};
}
function render() { aRes.render('pages/_templatePage', options); }
function asyncComplete() { preRender(); render(); }

Expand All @@ -76,8 +76,9 @@ exports.example = function (aReq, aRes, aNext) {
scriptListQuery.find({ isLib: false });

// Scripts: Query: Search
if (aReq.query.q)
if (aReq.query.q) {
modelQuery.parseScriptSearchQuery(scriptListQuery, aReq.query.q);
}

// Scripts: Query: Sort
modelQuery.parseModelListSort(scriptListQuery, aReq.query.orderBy, aReq.query.orderDir, function () {
Expand Down Expand Up @@ -108,4 +109,3 @@ exports.example = function (aReq, aRes, aNext) {
//---
async.parallel(tasks, asyncComplete);
};

39 changes: 22 additions & 17 deletions controllers/admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ exports.userAdmin = function (aReq, aRes, aNext) {
if (!userIsAdmin(aReq)) { return aNext(); }

// You can only see users with a role less than yours
User.find({ role: { $gt: thisUser.role } }, function (aErr, aUsers) { // TODO: STYLEGUIDE.md conformance needed here
// TODO: STYLEGUIDE.md conformance needed here
Copy link
Member

Choose a reason for hiding this comment

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

TODO:, NOTE:, BUG: (most editors support these) and Watchpoint (whatever casing) should never move from their intended lines... they are technically "followup" comments and not part of the code base.

User.find({ role: { $gt: thisUser.role } }, function (aErr, aUsers) {
var i = 0;
options.users = [];

Expand Down Expand Up @@ -95,14 +96,14 @@ exports.adminUserView = function (aReq, aRes, aNext) {
// Nothing fancy, just the stringified user object
User.findOne({ '_id': id, role: { $gt: thisUser.role } },
function (aErr, aUser) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is your indentation program removing a necessary space here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it wants function to be after the previous comma ,.

if (aErr || !aUser) { return aNext(); }
if (aErr || !aUser) { return aNext(); }

aRes.render('userAdmin', {
user: {
info: JSON.stringify(aUser.toObject(), null, ' ')
}
});
aRes.render('userAdmin', {
user: {
info: JSON.stringify(aUser.toObject(), null, ' ')
}
});
});
};

var jsonModelMap = {
Expand Down Expand Up @@ -130,18 +131,21 @@ exports.adminJsonView = function (aReq, aRes, aNext) {
options.isMod = authedUser && authedUser.isMod;
options.isAdmin = authedUser && authedUser.isAdmin;

if (!options.isAdmin)
if (!options.isAdmin) {
return aRes.send(403, { status: 403, message: 'Not an admin.' });
}

var model = jsonModelMap[modelname];
if (!model)
if (!model) {
return aRes.send(400, { status: 400, message: 'Invalid model.' });
}

model.findOne({
_id: id
}, function (aErr, aObj) {
if (aErr || !aObj)
if (aErr || !aObj) {
return aRes.send(404, { status: 404, message: 'Id doesn\'t exist.' });
}

aRes.json(aObj);
});
Expand Down Expand Up @@ -232,7 +236,7 @@ exports.adminPage = function (aReq, aRes, aNext) {

//---
async.parallel(tasks, function (aErr) {
if (aErr) return aNext();
if (aErr) { return aNext(); }
aRes.render('pages/adminPage', options);
});
};
Expand Down Expand Up @@ -285,7 +289,7 @@ exports.adminApiKeysPage = function (aReq, aRes, aNext) {

//---
async.parallel(tasks, function (aErr) {
if (aErr) return aNext();
if (aErr) { return aNext(); }
aRes.render('pages/adminApiKeysPage', options);
});
};
Expand All @@ -303,14 +307,15 @@ exports.adminNpmLsView = function (aReq, aRes, aNext) {
options.isMod = authedUser && authedUser.isMod;
options.isAdmin = authedUser && authedUser.isAdmin;

if (!options.isAdmin)
if (!options.isAdmin) {
return aRes.send(403, { status: 403, message: 'Not an admin.' });
}

exec('npm ls --json', function(aErr, aStdout, aStderr) {
if (aErr) return aRes.send(501, { status: 501, message: 'Not implemented.' });
exec('npm ls --json', function (aErr, aStdout, aStderr) {
Copy link
Member

Choose a reason for hiding this comment

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

nice catch

if (aErr) { return aRes.send(501, { status: 501, message: 'Not implemented.' }); }
aRes.json(JSON.parse(aStdout));
});
}
};

// Manage oAuth strategies without having to restart the server
// When new keys are added, we load the new strategy
Expand All @@ -322,7 +327,7 @@ exports.apiAdminUpdate = function (aReq, aRes, aNext) {

postStrats = Object.keys(aReq.body).map(function (aPostStrat) {
var values = aReq.body[aPostStrat];
return { name: aPostStrat, id: values[0], key: values[1] }
return { name: aPostStrat, id: values[0], key: values[1] };
});

Strategy.find({}, function (aErr, aStrats) {
Expand Down
61 changes: 31 additions & 30 deletions controllers/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,30 +72,32 @@ exports.auth = function (aReq, aRes, aNext) {

User.findOne({ name: { $regex: new RegExp('^' + username + '$', 'i') } },
function (aErr, aUser) {
var strategies = null;
var strat = null;

if (aUser) {
strategies = aUser.strategies;
strat = strategies.pop();

if (aReq.session.newstrategy) { // authenticate with a new strategy
delete aReq.session.newstrategy;
} else if (!strategy) { // use an existing strategy
strategy = strat;
} else if (strategies.indexOf(strategy) === -1) {
// add a new strategy but first authenticate with existing strategy
aReq.session.newstrategy = strategy;
strategy = strat;
} // else use the strategy that was given in the POST
}
var strategies = null;
Copy link
Member

Choose a reason for hiding this comment

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

Again with your indentation taking out a necessary spacing.

var strat = null;

if (aUser) {
strategies = aUser.strategies;
strat = strategies.pop();

if (aReq.session.newstrategy) {
// authenticate with a new strategy
delete aReq.session.newstrategy;
} else if (!strategy) {
// use an existing strategy
strategy = strat;
} else if (strategies.indexOf(strategy) === -1) {
// add a new strategy but first authenticate with existing strategy
aReq.session.newstrategy = strategy;
strategy = strat;
} // else use the strategy that was given in the POST
}

if (!strategy) {
return aRes.redirect('/register');
} else {
return auth();
}
});
if (!strategy) {
return aRes.redirect('/register');
} else {
return auth();
}
});
};

exports.callback = function (aReq, aRes, aNext) {
Expand All @@ -116,21 +118,20 @@ exports.callback = function (aReq, aRes, aNext) {
if (openIdStrategies[strategy]) {
strategyInstance._verify = function (aId, aDone) {
verifyPassport(aId, strategy, username, aReq.session.user, aDone);
}
};
} else {
strategyInstance._verify =
function (aToken, aRefreshOrSecretToken, aProfile, aDone) {
aReq.session.profile = aProfile;
verifyPassport(aProfile.id, strategy, username, aReq.session.user, aDone);
}
aReq.session.profile = aProfile;
verifyPassport(aProfile.id, strategy, username, aReq.session.user, aDone);
};
}

// This callback will happen after the verify routine
var authenticate = passport.authenticate(strategy, function (aErr, aUser, aInfo) {
if (aErr) { return aNext(aErr); }
if (!aUser) {
return aRes.redirect(doneUrl + (doneUrl === '/' ? 'register' : '')
+ '?authfail');
return aRes.redirect(doneUrl + (doneUrl === '/' ? 'register' : '') + '?authfail');
}

aReq.logIn(aUser, function (aErr) {
Expand Down Expand Up @@ -161,4 +162,4 @@ exports.callback = function (aReq, aRes, aNext) {
});

authenticate(aReq, aRes, aNext);
}
};
32 changes: 19 additions & 13 deletions controllers/discussion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ exports.categoryListPage = function (aReq, aRes, aNext) {

//---
async.parallel(tasks, function (aErr) {
if (aErr) aNext();
if (aErr) { aNext(); }

//--- PreRender
// discussionList
Expand Down Expand Up @@ -124,8 +124,9 @@ exports.list = function (aReq, aRes, aNext) {
var categorySlug = aReq.route.params.category;

var category = _.findWhere(categories, { slug: categorySlug });
if (!category)
if (!category) {
return aNext();
}

//
var options = {};
Expand Down Expand Up @@ -171,7 +172,7 @@ exports.list = function (aReq, aRes, aNext) {

//---
async.parallel(tasks, function (aErr) {
if (aErr) return aNext();
if (aErr) { return aNext(); }

//--- PreRender
// discussionList
Expand Down Expand Up @@ -211,8 +212,9 @@ exports.show = function (aReq, aRes, aNext) {
var topic = aReq.route.params.topic;

var category = _.findWhere(categories, { slug: categorySlug });
if (!category)
if (!category) {
return aNext();
}

findDiscussion(category.slug, topic, function (aDiscussionData) {
if (!aDiscussionData) { return aNext(); }
Expand Down Expand Up @@ -257,7 +259,7 @@ exports.show = function (aReq, aRes, aNext) {

//---
async.parallel(tasks, function (aErr) {
if (aErr) return aNext();
if (aErr) { return aNext(); }

//--- PreRender
// commentList
Expand All @@ -280,14 +282,16 @@ exports.show = function (aReq, aRes, aNext) {
exports.newTopic = function (aReq, aRes, aNext) {
var authedUser = aReq.session.user;

if (!authedUser)
if (!authedUser) {
return aRes.redirect('/login');
}

var categorySlug = aReq.route.params.category;

var category = _.findWhere(categories, { slug: categorySlug });
if (!category)
if (!category) {
return aNext();
}

//
var options = {};
Expand Down Expand Up @@ -394,16 +398,18 @@ exports.postTopic = postTopic;
exports.createTopic = function (aReq, aRes, aNext) {
var authedUser = aReq.session.user;

if (!authedUser)
if (!authedUser) {
return aRes.redirect('/login');
}

var categorySlug = aReq.route.params.category;
var topic = aReq.body['discussion-topic'];
var content = aReq.body['comment-content'];

var category = _.findWhere(categories, { slug: categorySlug });
if (!category)
if (!category) {
return aNext();
}

//
var options = {};
Expand All @@ -423,8 +429,8 @@ exports.createTopic = function (aReq, aRes, aNext) {
postTopic(authedUser, category.slug, topic, content, false, function (aDiscussion) {
if (!aDiscussion) { return exports.newTopic(aReq, aRes, aNext); }

aRes.redirect(encodeURI(aDiscussion.path
+ (aDiscussion.duplicateId ? '_' + aDiscussion.duplicateId : '')));
aRes.redirect(encodeURI(aDiscussion.path +
Copy link
Member

Choose a reason for hiding this comment

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

Okay with current STYLEGUIDE.md with post operator splitting

(aDiscussion.duplicateId ? '_' + aDiscussion.duplicateId : '')));
});
};

Expand All @@ -442,8 +448,8 @@ exports.createComment = function (aReq, aRes, aNext) {
if (!discussion) { return aNext(); }

postComment(user, discussion, content, false, function (err, discussion) {
aRes.redirect(encodeURI(discussion.path
+ (discussion.duplicateId ? '_' + discussion.duplicateId : '')));
aRes.redirect(encodeURI(discussion.path +
(discussion.duplicateId ? '_' + discussion.duplicateId : '')));
});
});
};
8 changes: 3 additions & 5 deletions controllers/document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ exports.view = function (aReq, aRes, aNext) {
options.isMod = authedUser && authedUser.isMod;
options.isAdmin = authedUser && authedUser.isAdmin;


if (document) {
documentPath = 'views/includes/documents';

Expand Down Expand Up @@ -90,10 +89,9 @@ exports.view = function (aReq, aRes, aNext) {
aCallback(null);
});
}
], aCallback)
], aCallback);
});
}
else {
} else {
// Page metadata
pageMetadata(options, ['About', 'About']);

Expand All @@ -118,7 +116,7 @@ exports.view = function (aReq, aRes, aNext) {
return statusCodePage(aReq, aRes, aNext, {
statusCode: aErr.statusCode,
statusMessage: aErr.statusMessage
})
});
}

aRes.render('pages/documentPage', options);
Expand Down
Loading