Lgtm bugfixes (#4838)

* code tidy up: always evaluates

* tidy up: is always true

* tidy up: remove unused code

* always true/false variables

* unused variable

* tidy up: remove unused code in caretPosition.js

* for squash: Revert "tidy up: remove unused code in caretPosition.js"

The `if` condition was previously always true, so the body should be
preserved. If the body is preserved, other logic can be deleted. I
opened PR #4845 to clean it all up.

This reverts commit 75b03e5a7d.

* for squash: simplify

* for squash: Explain that the getter is used for its side effects

It's very weird to call a getter without using its return value. Add a
comment explaining why this is done so that the reader doesn't get
confused.

* for squash: Revert "tidy up: remove unused code"

The exception test was the purpose of the code.

This reverts commit 85153b1676.

* for squash: Log the tsort results

Co-authored-by: Richard Hansen <rhansen@rhansen.org>

src/static/js/AttributeManager.js

@@ -209,10 +209,8 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
       if (selStart[1] === selEnd[1] && selStart[0] === selEnd[0]) return false;
 
       if (selStart[0] !== selEnd[0]) { // -> More than one line selected
-        let hasAttrib = true;
-
         // from selStart to the end of the first line
-        hasAttrib = hasAttrib && rangeHasAttrib(
+        let hasAttrib = rangeHasAttrib(
             selStart, [selStart[0], rep.lines.atIndex(selStart[0]).text.length]);
 
         // for all lines in between

src/static/js/Changeset.js

@@ -1759,7 +1759,7 @@ exports.makeAttribsString = (opcode, attribs, pool) => {
     return '';
   } else if ((typeof attribs) === 'string') {
     return attribs;
-  } else if (pool && attribs && attribs.length) {
+  } else if (pool && attribs.length) {
     if (attribs.length > 1) {
       attribs = attribs.slice();
       attribs.sort();

src/static/js/ace2_inner.js

@@ -3782,7 +3782,7 @@ function Ace2Inner() {
 
   // We apply the height of a line in the doc body, to the corresponding sidediv line number
   const updateLineNumbers = () => {
-    if (!currentCallStack || currentCallStack && !currentCallStack.domClean) return;
+    if (!currentCallStack || !currentCallStack.domClean) return;
 
     // Refs #4228, to avoid layout trashing, we need to first calculate all the heights,
     // and then apply at once all new height to div elements

src/static/js/contentcollector.js

@@ -195,7 +195,7 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
       }
     }
 
-    if (listType === 'none' || !listType) {
+    if (listType === 'none') {
       delete state.lineAttributes.list;
     } else {
       state.lineAttributes.list = listType;

src/static/js/pluginfw/tsort.js

@@ -81,6 +81,7 @@ const tsortTest = () => {
 
   try {
     sorted = tsort(edges);
+    console.log('succeeded', sorted);
   } catch (e) {
     console.log(e.message);
   }