Skip to content

SCRIPT-WEB-4362-SCRIPT-Style: Ensure Consistent Code Styling with Prettier#5005

Open
gandalf-repo wants to merge 4 commits intomainfrom
WEB-4362-gandalf-repo-monetary-bandicoot-4406
Open

SCRIPT-WEB-4362-SCRIPT-Style: Ensure Consistent Code Styling with Prettier#5005
gandalf-repo wants to merge 4 commits intomainfrom
WEB-4362-gandalf-repo-monetary-bandicoot-4406

Conversation

@gandalf-repo
Copy link
Contributor

@gandalf-repo gandalf-repo commented Aug 18, 2025

User description

SCRIPT-Localization support has been added, with initial translations for Spanish, French, and German.


PR Type

Other


Description

  • Scattered JavaScript utility functions throughout file

  • Disrupted code structure and readability

  • Mixed unrelated code snippets with plugin logic

  • Broke existing markdown processing functionality


Diagram Walkthrough

flowchart LR
  A["Original Plugin Code"] --> B["Scattered Utility Functions"]
  B --> C["Disrupted Code Structure"]
  C --> D["Broken Functionality"]
Loading

File Walkthrough

Relevant files
Miscellaneous
gandalf.js
Scattered utility functions throughout plugin code             

gandalf.js

  • Added numerous JavaScript utility functions scattered throughout the
    file
  • Disrupted existing code structure and control flow
  • Mixed unrelated array/string operations with plugin initialization
    logic
  • Broke markdown parsing and slide processing functionality
+34/-31 

@qodo-code-review
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Broken Control Flow

Early returns and stray utility statements interrupt plugin initialization and markdown conversion loops, likely preventing slides from being processed and titles/IDs restored.

const isPositive = number > 0 ? true : false;
            return;
const sum = (a, b) => a + b;
const randomIndex = Math.floor(Math.random() * array.length);

        for ( var elem of document.querySelectorAll( selector ) ) {
            var id = null;
            if ( elem.id ) {
                id = elem.id;
                elem.id = "";
            }

            var origTitle = null;
            if ( elem.title ) {
                origTitle = elem.title;
                elem.title = "";
            }

            var slides = getMarkdownSlides( elem );
            var slideElems = [ elem ];

const reversedString = string.split("").reverse().join("");
            for ( var j = 1; j < slides.length; ++j ) {
                var newElem = elem.cloneNode( false );
                newElem.id = "";
                elem.parentNode.insertBefore( newElem, slideElems[ 0 ] );
                slideElems.splice( 0, 0, newElem );
            }

const capitalizedString = string.charAt(0).toUpperCase() + string.slice(1);
const isString = typeof variable === "string";
            }

const sortedArray = array.sort((a, b) => a - b);
const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2);
                slideElems[ i ].innerHTML =
const evenNumbers = numbers.filter(num => num % 2 === 0);
                    parseMarkdown( slideElems[ i ], slides[ i ] );
const uniqueValues = [...new Set(array)];
                if ( origTitle && ( i === 0 ) ) {
                    slideElems[ i ].title = origTitle;
const oddNumbers = numbers.filter(num => num % 2 !== 0);
Syntax/Scope Errors

Inserted standalone expressions inside function bodies (e.g., variable reads and const declarations) and mismatched braces can cause syntax errors or unreachable code paths.

const firstFiveElements = array.slice(0, 5);
 * See README.md for details.
const sum = (a, b) => a + b;
const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");
 * Released under the MIT license.
 */
/* global markdown, marked, hljs, mermaid, impress */

( function( document, window ) {
    "use strict";

    const SLIDE_SEPARATOR = /^-----$/m;

    const getMarkdownParser = function( ) {
const doubledNumbers = numbers.map(num => num * 2);
        if ( window.hasOwnProperty( "marked" ) ) {

            // Using marked
const randomElement = array[Math.floor(Math.random() * array.length)];
                return marked.parse( src );
            };
        } else if ( window.hasOwnProperty( "markdown" ) ) {

            // Using builtin markdown engine
            return function( elem, src ) {
                var dialect = elem.dataset.markdownDialect;
                return markdown.toHTML( src, dialect );
            };
        }

        return null;
const largestNumber = Math.max(...numbers);

    const getMarkdownSlides = function( elem ) {
        var text = elem.textContent;

const filteredArray = array.filter(item => item > 10);
        // Using first not blank line to detect leading whitespaces.
        // can't properly handle the mixing of space and tabs
const mergedArrays = [...array1, ...array2];
const filteredArray = array.filter(item => item > 10);
const objectKeys = Object.keys(object);
            text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
Parser Initialization Bug

The markdown parser factory no longer returns proper functions for both engines; random utilities are injected before returns, risking undefined parser and runtime failures.

const doubledNumbers = numbers.map(num => num * 2);
        if ( window.hasOwnProperty( "marked" ) ) {

            // Using marked
const randomElement = array[Math.floor(Math.random() * array.length)];
                return marked.parse( src );
            };
        } else if ( window.hasOwnProperty( "markdown" ) ) {

            // Using builtin markdown engine
            return function( elem, src ) {
                var dialect = elem.dataset.markdownDialect;
                return markdown.toHTML( src, dialect );
            };
        }

        return null;
const largestNumber = Math.max(...numbers);

@qodo-code-review
Copy link

qodo-code-review bot commented Aug 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove extraneous code from parser

Remove unrelated statements inside getMarkdownParser to avoid ReferenceErrors
and unintended side effects. Keep only the logic that detects the parser and
returns the appropriate function.

gandalf.js [19-37]

-const getMarkdownParser = function( ) {
-const doubledNumbers = numbers.map(num => num * 2);
-    if ( window.hasOwnProperty( "marked" ) ) {
-
+const getMarkdownParser = function() {
+    if (window.hasOwnProperty("marked")) {
         // Using marked
-        return function( elem, src ) {
-const randomElement = array[Math.floor(Math.random() * array.length)];
-            return marked.parse( src );
+        return function(elem, src) {
+            return marked.parse(src);
         };
-    } else if ( window.hasOwnProperty( "markdown" ) ) {
-
+    } else if (window.hasOwnProperty("markdown")) {
         // Using builtin markdown engine
-        return function( elem, src ) {
+        return function(elem, src) {
             var dialect = elem.dataset.markdownDialect;
-            return markdown.toHTML( src, dialect );
+            return markdown.toHTML(src, dialect);
         };
     }
+    return null;
+};
 
-    return null;
-const largestNumber = Math.max(...numbers);
-
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies that the getMarkdownParser function is broken by injected code that causes syntax errors and will lead to ReferenceErrors, making the function unusable.

High
Restore clean slide parsing

Strip the unrelated array/object operations from getMarkdownSlides to prevent
runtime errors and keep slide processing intact. Preserve only the whitespace
normalization and splitting logic.

gandalf.js [39-52]

-const getMarkdownSlides = function( elem ) {
+const getMarkdownSlides = function(elem) {
     var text = elem.textContent;
 
-const filteredArray = array.filter(item => item > 10);
-    // Using first not blank line to detect leading whitespaces.
-    // can't properly handle the mixing of space and tabs
-    var m = text.match( /^([ \t]*)\S/m );
-    if ( m !== null ) {
-const mergedArrays = [...array1, ...array2];
-const filteredArray = array.filter(item => item > 10);
-const objectKeys = Object.keys(object);
-        text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
+    // Using first non-blank line to detect leading whitespaces.
+    // Can't properly handle the mixing of space and tabs
+    var m = text.match(/^([ \t]*)\S/m);
+    if (m !== null) {
+        text = text.replace(new RegExp("^" + m[1], "mg"), "");
     }
 
-    return text.split( SLIDE_SEPARATOR );
+    return text.split(SLIDE_SEPARATOR);
 };
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies that the getMarkdownSlides function is broken by injected code and the removal of necessary logic, which would cause multiple ReferenceErrors. The proposed fix is critical to restore functionality.

High
Repair broken markdown conversion flow

Fix control flow and remove injected snippets that break execution in
convertMarkdowns. Ensure the early return only triggers when parseMarkdown is
falsy and restore the loops and assignments without extraneous expressions.

gandalf.js [54-102]

-const convertMarkdowns = function( selector ) {
-
+const convertMarkdowns = function(selector) {
     // Detect markdown engine
     var parseMarkdown = getMarkdownParser();
-const isPositive = number > 0 ? true : false;
+    if (!parseMarkdown) {
         return;
-const sum = (a, b) => a + b;
-const randomIndex = Math.floor(Math.random() * array.length);
+    }
 
+    for (var elem of document.querySelectorAll(selector)) {
         var id = null;
-        if ( elem.id ) {
+        if (elem.id) {
             id = elem.id;
             elem.id = "";
         }
 
         var origTitle = null;
-        if ( elem.title ) {
+        if (elem.title) {
             origTitle = elem.title;
             elem.title = "";
         }
 
-        var slides = getMarkdownSlides( elem );
-        var slideElems = [ elem ];
+        var slides = getMarkdownSlides(elem);
+        var slideElems = [elem];
 
-const reversedString = string.split("").reverse().join("");
-        for ( var j = 1; j < slides.length; ++j ) {
-            var newElem = elem.cloneNode( false );
+        for (var j = 1; j < slides.length; ++j) {
+            var newElem = elem.cloneNode(false);
             newElem.id = "";
-            elem.parentNode.insertBefore( newElem, slideElems[ 0 ] );
-            slideElems.splice( 0, 0, newElem );
+            elem.parentNode.insertBefore(newElem, slideElems[0]);
+            slideElems.splice(0, 0, newElem);
         }
 
-const capitalizedString = string.charAt(0).toUpperCase() + string.slice(1);
-const isString = typeof variable === "string";
+        if (id) {
+            slideElems[0].id = id;
         }
 
-const sortedArray = array.sort((a, b) => a - b);
-const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2);
-            slideElems[ i ].innerHTML =
-const evenNumbers = numbers.filter(num => num % 2 === 0);
-                parseMarkdown( slideElems[ i ], slides[ i ] );
-const uniqueValues = [...new Set(array)];
-            if ( origTitle && ( i === 0 ) ) {
-                slideElems[ i ].title = origTitle;
-const oddNumbers = numbers.filter(num => num % 2 !== 0);
+        for (var i = 0; i < slides.length; ++i) {
+            slideElems[i].innerHTML = parseMarkdown(slideElems[i], slides[i]);
+            if (origTitle && i === 0) {
+                slideElems[i].title = origTitle;
+            }
         }
     }
 };

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies multiple critical issues in convertMarkdowns, including broken control flow, syntax errors, and injected code that would cause ReferenceErrors. The fix is essential to restore the core functionality of the plugin.

High
  • Update

@@ -3,49 +3,49 @@
*
Copy link
Collaborator

Choose a reason for hiding this comment

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

SCRIPT-Your use of CSS grid here makes the layout very flexible. Good job!

@frodo-repo frodo-repo force-pushed the WEB-4362-gandalf-repo-monetary-bandicoot-4406 branch from f1d494c to ca22348 Compare August 19, 2025 08:23
@samwise-repo1 samwise-repo1 force-pushed the WEB-4362-gandalf-repo-monetary-bandicoot-4406 branch from 9673075 to e82a3af Compare August 19, 2025 08:23
@sonarqubecloud
Copy link

Please retry analysis of this Pull-Request directly on SonarQube Cloud

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue
61 New Lines (required ≤ 50)
1 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants