From ce83181ac3c1b377ddbf122740c65728ed0fc82e Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 22 Feb 2021 08:26:35 +0000 Subject: [PATCH] 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 75b03e5a7dc1ff9a8728ed2341fd9fe970d0615f. * 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 85153b167613b2513fff99e22b8ded8ea1e4547b. * for squash: Log the tsort results Co-authored-by: Richard Hansen --- src/node/db/API.js | 2 +- src/node/db/Pad.js | 2 +- src/node/utils/ExportHtml.js | 4 ++-- src/static/js/AttributeManager.js | 4 +--- src/static/js/Changeset.js | 2 +- src/static/js/ace2_inner.js | 2 +- src/static/js/contentcollector.js | 2 +- src/static/js/pluginfw/tsort.js | 1 + 8 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/node/db/API.js b/src/node/db/API.js index ea77ac7df..c262e078b 100644 --- a/src/node/db/API.js +++ b/src/node/db/API.js @@ -722,7 +722,7 @@ Example returns: */ exports.sendClientsMessage = async (padID, msg) => { - const pad = await getPadSafe(padID, true); + await getPadSafe(padID, true); // Throw if the padID is invalid or if the pad does not exist. padMessageHandler.handleCustomMessage(padID, msg); }; diff --git a/src/node/db/Pad.js b/src/node/db/Pad.js index 965f2793f..a34668987 100644 --- a/src/node/db/Pad.js +++ b/src/node/db/Pad.js @@ -87,7 +87,7 @@ Pad.prototype.appendRevision = async function appendRevision(aChangeset, author) // ex. getNumForAuthor if (author !== '') { - this.pool.putAttrib(['author', author || '']); + this.pool.putAttrib(['author', author]); } if (newRev % 100 === 0) { diff --git a/src/node/utils/ExportHtml.js b/src/node/utils/ExportHtml.js index 9d40111ed..38e5fb1a6 100644 --- a/src/node/utils/ExportHtml.js +++ b/src/node/utils/ExportHtml.js @@ -314,7 +314,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => { await hooks.aCallAll('getLineHTMLForExport', context); // To create list parent elements if ((!prevLine || prevLine.listLevel !== line.listLevel) || - (prevLine && line.listTypeName !== prevLine.listTypeName)) { + (line.listTypeName !== prevLine.listTypeName)) { const exists = _.find(openLists, (item) => ( item.level === line.listLevel && item.type === line.listTypeName) ); @@ -414,7 +414,7 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => { if ((!nextLine || !nextLine.listLevel || nextLine.listLevel < line.listLevel) || - (nextLine && line.listTypeName !== nextLine.listTypeName)) { + (line.listTypeName !== nextLine.listTypeName)) { let nextLevel = 0; if (nextLine && nextLine.listLevel) { nextLevel = nextLine.listLevel; diff --git a/src/static/js/AttributeManager.js b/src/static/js/AttributeManager.js index 1d8bd9376..6cda86c48 100644 --- a/src/static/js/AttributeManager.js +++ b/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 diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 75bb92b20..b47979c19 100644 --- a/src/static/js/Changeset.js +++ b/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(); diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index 429fef7cd..4858aa1b4 100644 --- a/src/static/js/ace2_inner.js +++ b/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 diff --git a/src/static/js/contentcollector.js b/src/static/js/contentcollector.js index 1c601cc54..c250008bb 100644 --- a/src/static/js/contentcollector.js +++ b/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; diff --git a/src/static/js/pluginfw/tsort.js b/src/static/js/pluginfw/tsort.js index e584ada29..117f8555c 100644 --- a/src/static/js/pluginfw/tsort.js +++ b/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); }