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

Jump to bottom

Some cleanup based on jshint #799

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Some cleanup based on jshint #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31ff306
Add more style and globals to .jshintrc
torhve Aug 2, 2016
d40b4b4
Style cleanup, remove unused functions and vars.
torhve Aug 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion .jshintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
{
"browser": true,
"devel": true,
"curly": true,
"funcscope": true,
"noempty": true,
"indent": true,
"nonbsp": true,
"strict": true,
"undef": true,
"unused": true,
"globals": {
"angular": false,
"weeChat": false,
"emojione": false,
"MathJax": false,
"renderMathInElement": false,
"Zlib": false,
"_": false,
"Notification": false,
"Favico": false
"Favico": false,
"Windows": false,
"setElectronBadge": false
}
}
19 changes: 10 additions & 9 deletions js/connection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ weechat.factory('connection',
var locked = false;

// Takes care of the connection and websocket hooks
var connect = function (host, port, passwd, ssl, noCompression, successCallback, failCallback) {
var connect = function(host, port, passwd, ssl, noCompression, successCallback, failCallback) {
$rootScope.passwordError = false;
connectionData = [host, port, passwd, ssl, noCompression];
var proto = ssl ? 'wss' : 'ws';
Expand All @@ -30,7 +30,7 @@ weechat.factory('connection',
var url = proto + "://" + host + ":" + port + "/weechat";
$log.debug('Connecting to URL: ', url);

var onopen = function () {
var onopen = function() {


// Helper methods for initialization commands
Expand Down Expand Up @@ -194,7 +194,7 @@ weechat.factory('connection',
};


var onclose = function (evt) {
var onclose = function(evt) {
/*
* Handles websocket disconnection
*/
Expand All @@ -210,7 +210,7 @@ weechat.factory('connection',
handleWrongPassword();
};

var handleClose = function (evt) {
var handleClose = function(evt) {
if (ssl && evt && evt.code === 1006) {
// A password error doesn't trigger onerror, but certificate issues do. Check time of last error.
if (typeof $rootScope.lastError !== "undefined" && (Date.now() - $rootScope.lastError) < 1000) {
Expand All @@ -229,7 +229,7 @@ weechat.factory('connection',
}
};

var onerror = function (evt) {
var onerror = function(evt) {
/*
* Handles cases when connection issues come from
* the relay.
Expand Down Expand Up @@ -276,7 +276,7 @@ weechat.factory('connection',

};

var attemptReconnect = function (bufferId, timeout) {
var attemptReconnect = function(bufferId, timeout) {
$log.info('Attempting to reconnect...');
var d = connectionData;
connect(d[0], d[1], d[2], d[3], d[4], function() {
Expand All @@ -289,7 +289,8 @@ weechat.factory('connection',
if (timeout >= 600000) {
// If timeout is ten minutes or more, give up
$log.info('Failed to reconnect, giving up');
handleClose();
// This code can't run since handleClose is defined inside connect
// handleClose();
Copy link
Member

Choose a reason for hiding this comment

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

we should fix that instead of removing it, I guess

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, but that doesn't have to be a part of this PR. And that code wouldn't be able to run anyway so I'm not changing anything really.

Copy link
Member

Choose a reason for hiding this comment

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

true but at least we now know that there's an issue here :)

} else {
$log.info('Failed to reconnect, scheduling next attempt in', timeout/1000, 'seconds');
// Clear previous timer, if exists
Expand All @@ -305,7 +306,7 @@ weechat.factory('connection',
};


var reconnect = function (evt) {
var reconnect = function() {
if (connectionData.length < 5) {
// something is wrong
$log.error('Cannot reconnect, connection information is missing');
Expand Down Expand Up @@ -334,7 +335,7 @@ weechat.factory('connection',
$rootScope.userdisconnect = true;
ngWebsockets.send(weeChat.Protocol.formatQuit());
// In case the backend doesn't repond we will close from our end
var closeTimer = setTimeout(function() {
setTimeout(function() {
ngWebsockets.disconnect();
// We pretend we are not connected anymore
// The connection can time out on its own
Expand Down
12 changes: 9 additions & 3 deletions js/filters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ weechat.filter('DOMfilter', ['$filter', '$sce', function($filter, $sce) {
}
}
// recurse
if (node === undefined || node === null) return;
if (node === undefined || node === null) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

hmpf I find that style cumbersome if the then-block fits on one line

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's more cumbersome. But it's easier to maintain, and easier to stick to one style and follow it. Less chance of introducing bugs later.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong preference, I just find it unnecessarily verbose and hard to read. But it assumes that everyone knows what they're doing so maybe it's best to not do it.

Would if (...) { return; } (in one line) be okay in your opinion? I'm not asking you to change it here but more of a general question

node = node.firstChild;
while (node) {
var nextNode = null;
Expand Down Expand Up @@ -200,8 +202,12 @@ weechat.filter('latexmath', function() {

weechat.filter('prefixlimit', function() {
return function(input, chars) {
if (isNaN(chars)) return input;
if (chars <= 0) return '';
if (isNaN(chars)) {
return input;
}
if (chars <= 0) {
return '';
}
if (input && input.length > chars) {
input = input.substring(0, chars);
return input + '+';
Expand Down
9 changes: 6 additions & 3 deletions js/glowingbear.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ weechat.controller('WeechatCtrl', ['$rootScope', '$scope', '$store', '$timeout',
}

// Check if user decides to save password, and copy it over
settings.addCallback('savepassword', function(newvalue) {
settings.addCallback('savepassword', function() {
if (settings.savepassword) {
// Init value in settings module
settings.setDefaults({'password': $scope.password});
Expand Down Expand Up @@ -637,8 +637,11 @@ weechat.controller('WeechatCtrl', ['$rootScope', '$scope', '$store', '$timeout',
function closest(elem, selector) {
var matchesSelector = elem.matches || elem.webkitMatchesSelector || elem.mozMatchesSelector || elem.msMatchesSelector;
while (elem) {
if (matchesSelector.call(elem, selector)) return elem;
else elem = elem.parentElement;
if (matchesSelector.call(elem, selector)) {
return elem;
} else {
elem = elem.parentElement;
}
}
}
closest($event.target, '.gb-modal').setAttribute('data-state', 'hidden');
Expand Down
4 changes: 3 additions & 1 deletion js/imgur.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ weechat.factory('imgur', ['$rootScope', function($rootScope) {
var process = function(image, callback) {

// Is it an image?
if (!image || !image.type.match(/image.*/)) return;
if (!image || !image.type.match(/image.*/)) {
return;
}

// New file reader
var reader = new FileReader();
Expand Down
8 changes: 0 additions & 8 deletions js/irc-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@
var IrcUtils = angular.module('IrcUtils', []);

IrcUtils.service('IrcUtils', [function() {
/**
* Escape a string for usage in a larger regexp
* @param str String to escape
* @return Escaped string
*/
var escapeRegExp = function(str) {
return str.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
};

/**
* Get a new version of a nick list, sorted by last speaker
Expand Down
4 changes: 3 additions & 1 deletion js/models.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ models.service('models', ['$rootScope', '$filter', function($rootScope, $filter)
getHistoryUp: getHistoryUp,
getHistoryDown: getHistoryDown,
isNicklistEmpty: isNicklistEmpty,
nicklistRequested: nicklistRequested
nicklistRequested: nicklistRequested,
allLinesFetched: allLinesFetched,
active: active,
};

};
Expand Down
5 changes: 5 additions & 0 deletions js/notifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
(function() {
'use strict';

var weechat = angular.module('weechat');

weechat.factory('notifications', ['$rootScope', '$log', 'models', 'settings', function($rootScope, $log, models, settings) {
Expand Down Expand Up @@ -227,3 +230,5 @@ weechat.factory('notifications', ['$rootScope', '$log', 'models', 'settings', fu
unreadCount: unreadCount
};
}]);

})();
2 changes: 1 addition & 1 deletion js/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ weechat.factory('settings', ['$store', '$rootScope', function($store, $rootScope
// Add a callback to be called whenever the value is changed
// It's like a free $watch and used to be called the observer
// pattern, but I guess that's too old-school for JS kids :>
this.addCallback = function(key, callback, callNow) {
this.addCallback = function(key, callback) {
if (this.callbacks[key] === undefined) {
this.callbacks[key] = [callback];
} else {
Expand Down
37 changes: 22 additions & 15 deletions js/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
(function() {
'use strict';

var weechat = angular.module('weechat');

weechat.factory('utils', function() {
// Helper to change style of a class
var changeClassStyle = function(classSelector, attr, value) {
_.each(document.getElementsByClassName(classSelector), function(e) {
e.style[attr] = value;
});
};
// Helper to get style from a class
var getClassStyle = function(classSelector, attr) {
_.each(document.getElementsByClassName(classSelector), function(e) {
return e.style[attr];
});
};
// Helper to change style of a class
var changeClassStyle = function(classSelector, attr, value) {
_.each(document.getElementsByClassName(classSelector), function(e) {
e.style[attr] = value;
});
};
// Helper to get style from a class
var getClassStyle = function(classSelector, attr) {
_.each(document.getElementsByClassName(classSelector), function(e) {
return e.style[attr];
});
};

var isMobileUi = function() {
// TODO don't base detection solely on screen width
Expand All @@ -35,18 +38,22 @@ weechat.factory('utils', function() {
var elem = document.createElement("link");
elem.rel = "stylesheet";
elem.href = css_url;
if (id)
if (id) {
elem.id = id;
}
var head = document.getElementsByTagName("head")[0];
head.appendChild(elem);
};


return {
changeClassStyle: changeClassStyle,
getClassStyle: getClassStyle,
changeClassStyle: changeClassStyle,
getClassStyle: getClassStyle,
isMobileUi: isMobileUi,
inject_script: inject_script,
inject_css: inject_css,
};
});

})();