7
\$\begingroup\$

In another Code Review, it was recommended that I alphabetize my CSS. So I made a tool to do this quickly in JavaScript.

Because CSS has two levels (selector on the outside, parameters on the inside), and they both need to be alphabetized, alphabetizing it is not as easy as just doing array.sort(). A custom tool is needed.

Feel free to comment on anything JavaScript. Here's my code.

"use strict";

class CSSAlphabetizer {
    alphabetize(s) {
        let lines, blocks;
        
        // TODO: this._validate();
        // TODO: this._beautify(); using regex and replace of { ; }
        
        lines = this._makeLinesArray(s);
        blocks = this._sortSelectors(lines);
        blocks = this._sortParameters(blocks);
        
        s = this._makeString(blocks);
        s = this._addBlankLineBetweenBlocks(s);
        s = s.trim();
        
        return s;
    }
    
    _addBlankLineBetweenBlocks(s) {
        return s.replace(/}\n([^\n])/, "}\n\n$1");
    }
    
    _makeString(blocks) {
        // blocks => lines
        let lines = [];
        for ( let block of blocks ) {
            let lines2 = block[1];
            for ( let line of lines2 ) {
                lines.push(line);
            }
        }
        
        // lines => string
        let s = "";
        for ( let line of lines ) {
            s += line[1] + "\n";
        }
        s = s.slice(0, s.length-1);
        return s;
    }
    
    _sortSelectors(lines) {
        /** [parameterTrimmed, [line, line, line, line]] */
        let blocks = [];
        /** [line, line, line, line] **/
        let buffer = [];
        let lineNumTracker = 0;
        let parameterTrimmed = "";
        let len = lines.length;
        
        for ( let i = 0; i < len; i++ ) {
            let line = lines[i];
            let lineNum = line[2];
            
            if ( ! parameterTrimmed && line[0].includes("{") ) {
                parameterTrimmed = line[0];
            }
            
            if ( lineNum !== lineNumTracker ) {
                lineNumTracker++;
                blocks.push([parameterTrimmed, buffer]);
                buffer = [];
                parameterTrimmed = "";
            }
            
            buffer.push(line);
            
            // Last line. Finish the block.
            if ( i === len-1 ) {
                lineNumTracker++;
                blocks.push([parameterTrimmed, buffer]);
                buffer = [];
                parameterTrimmed = "";
            }
        }
        
        blocks = blocks.sort();
        
        return blocks;
    }
    
    _sortParameters(blocks) {
        for ( let key in blocks ) {
            let lines = blocks[key][1];
            let lineBuffer = [];
            let sortBuffer = [];
            
            for ( let line of lines ) {
                let isParameter = line[3];
                
                if ( isParameter ) {
                    sortBuffer.push(line);
                } else {
                    if ( sortBuffer ) {
                        sortBuffer = sortBuffer.sort();
                        lineBuffer = lineBuffer.concat(sortBuffer);
                    }
                    
                    lineBuffer.push(line);
                }
            }
            
            blocks[key][1] = lineBuffer;
        }
        console.log(blocks);
        return blocks;
    }
    
    /** Makes an array of arrays. [trimmed, notTrimmed, blockNum, isParameter]. Having unique block numbers will be important later, when we are alphabetizing things. */
    _makeLinesArray(s) {
        // TODO: refactor to use associative array, more readable code, and less likely to have bugs if later we want to add/change/delete fields
        let lines = s.split("\n");
        let blockNum = 0;
        let isParameter = false;
        
        for ( let key in lines ) {
            let value = lines[key];
            
            if ( value.includes("}") ) {
                isParameter = false;
            }
            
            lines[key] = [
                // Trimmed line in first spot. Important for sorting.
                value.trim(),
                value,
                blockNum,
                isParameter,
            ];
            
            // When } is found, increment the block number. This keeps comment lines above block grouped with that block.
            if ( value.includes("}") ) {
                blockNum++;
            }
            
            if ( value.includes("{") ) {
                isParameter = true;
            }
        }
        
        console.log(lines);
        
        return lines;
    }
}

window.addEventListener('DOMContentLoaded', (e) => {
    let css = document.getElementById('css');
    let alphabetize = document.getElementById('alphabetize');
    
    // TODO: Combo box that loads tests.
    
    alphabetize.addEventListener('click', function(e) {
        let alphabetizer = new CSSAlphabetizer();
        css.value = alphabetizer.alphabetize(css.value);
    });
});
body {
    margin: 1em;
    width: 95%;
    max-width: 700px;
    font-family: sans-serif;
}

textarea {
    width: 100%;
    height: 360px;
    tab-size: 4;
}
<!DOCTYPE html>

<html lang="en-us">

<head>
    <title>CSS Alphabetizer</title>
    <link rel="stylesheet" href="style.css" />
    <script type="module" src="css-alphabetizer.js"></script>
</head>

<body>
    <h1>
    CSS Alphabetizer
    </h1>
    
    <p>
    Beautify your CSS. Sort your selectors alphabetically (which groups together #id's, .classes, and then elements), and sort your parameters alphabetically too. Parameters are nested within braces, so those aren't easy to sort with a normal A-Z sort.
    </p>
    
    <p>
    <textarea id="css">body {
    background-color: #999999;
    margin-bottom: 0;
}

#page-container {
    background-color: #DFDFDF;
    width: 1150px;
    margin: 12px auto 0 auto;
}

#page-container2 {
    padding: 12px 12px 0 12px;
}

header {
    background-color: white;
}

.category {
    display: inline-block;
    padding: 18px 25px;
    font-family: sans-serif;
}

.category:hover {
    background-color: #059BD8;
}</textarea>
    </p>
    
    <p>
    <button id="alphabetize">Alphabetize</button>
    </p>
    
    <p>
    Want to report a bug or request a feature? <a href="https://github.com/RedDragonWebDesign/css-alphabetizer/issues">Create an issue</a> on our GitHub.
    </p>
</body>

</html>

\$\endgroup\$
6
  • 1
    \$\begingroup\$ I completely agree with @Emma. That regex is want you want. \$\endgroup\$ – Rithik Banerjee Jul 23 '20 at 6:34
  • 1
    \$\begingroup\$ Nice. I like how you promote "groups together #id's, .classes, and then elements" as a feature instead of (at least) giving the option to (and writing the code for) strip # and . etc. for sorting - something I'd prefer. \$\endgroup\$ – iAmOren Jul 23 '20 at 10:13
  • \$\begingroup\$ Cool concept. It would be prudent to add annotations that would prevent regions of the code from being sorted. Consider that you have a table 40 columns wide. All columns except 5, 9, 15 and 21 have the same format. All rows have the same format other than 6, 9, 25. Do you really want to write a work around using CSS? Size may not matter but position sure does! \$\endgroup\$ – TinMan Jul 31 '20 at 20:42
  • 1
    \$\begingroup\$ CSS Inheritance, Cascade, and Specificity: If a rule from the same style sheet, with the same level of specificity exists, the rule that is declared last in the CSS document will be the one that is applied. \$\endgroup\$ – TinMan Aug 6 '20 at 19:44
  • 1
    \$\begingroup\$ JS Fiddle. This cruse example has two css blocks that both target p elements. The first block color: red; style is later replaced with color: black;. It would be best for the author to catch the inconsistencies, but unrealistic that everyone would be handled \$\endgroup\$ – TinMan Aug 6 '20 at 20:01
3
+50
\$\begingroup\$

From a long review;

  • You should use a beautifier it will turn if ( i === len-1 ) to if (i === len-1)

  • This could really use an IFFE to hide your worker functions and expose 1 actual function, the class seems overkill.

  • You can club together some statements

      blocks = this._sortSelectors(lines);
      blocks = this._sortParameters(blocks);
    

    can become

      blocks = sortParameters(sortSelectors(lines));
    

    and

      s = this._makeString(blocks);
      s = this._addBlankLineBetweenBlocks(s);
      s = s.trim();
    

    can become

      s = this.separateBlocks(makeString(blocks)).trim();
    
  • This looks write you wrote your own String.join()

      // lines => string
      let s = "";
      for ( let line of lines ) {
          s += line[1] + "\n";
      }
      s = s.slice(0, s.length-1);
      return s;
    

    you could just

      return lines.map(line=>line[1]).join('\n');
    
  • Always consider FP when dealing with list;

      let lines = [];
      for ( let block of blocks ) {
          let lines2 = block[1];
          for ( let line of lines2 ) {
              lines.push(line);
          }
      }
    

    could be

      const lines = blocks.map(block => block[1]).flat(); 
    
  • Minor nitpicking on this;

              // Trimmed line in first spot. Important for sorting.
              value.trim(),
    

    You could have written a custom sort function that does the trim()

  • Dont do console.log in final code

  • let vs. const always requires some contemplation

    • let value = lines[key]; should be const value = lines[key];
    • let isParameter = line[3]; should be a const
    • let lines = s.split("\n"); should be a const
  • Potential Parsing Problems

    • value.includes("}" would go funky with some comments /* } */
    • Also, value.includes("{") of course
  • If you like ternary operators then

          if ( value.includes("{") ) {
              isParameter = true;
          }
    

    could be

          isParameter = value.includes("{") ? true : isParameter;
    

    however I think your version reads better

  • The html could use a <meta charset="UTF-8">

  • if ( sortBuffer ) { always executes, you probably meant to go for if ( sortBuffer.length ) {, personally I dropped the if statement altogether for elegance, the code works either way

The below is my counter proposal, my FP skills were not good enough to convert sortSelectors and sortParameters but I am sure there is a way to make these functions cleaner;

/*jshint esnext:true */

const styleSorter = (function createStyleSorter(){
  "use strict";
  
  // TODO: this._validate();
  // TODO: this._beautify(); using regex and replace of { ; }
  function alphabetize(s){
    
    const lines = makeLines(s + '\n');
    const blocks = sortParameters(sortSelectors(lines));

    return separateBlocks(makeString(blocks)).trim();        
  }
  
  //TODO: Always document your regex expressions
  function separateBlocks(s) {
    return s.replace(/}\n([^\n])/, "}\n\n$1");
  }  
  
  function makeString(blocks){
    //get 2nd element of each block, flatten, get 2nd element again
    const lines = blocks.map(block => block[1]).flat().map(fields => fields[1]);
    return lines.join('\n');
  }
    
  //Makes an array of arrays. [trimmed, notTrimmed, blockNum, isParameter].
  //Having unique block numbers will be important later, when we sort
  //TODO: refactor with associative array <- Not sure that will help much?
  function makeLines(s) {
    let blockNum = 0;
    let isParameter = false;
        
    return s.split("\n").map((value, key) => {
          
      if ( value.includes("}") ) {
        isParameter = false;
      }
          
      //First value is trimmed for sorting
      const line = [value.trim(), value, blockNum, isParameter];
          
      if ( value.includes("}") ) {
        blockNum++;
      }
            
      if ( value.includes("{") ) {
        isParameter = true;
      }          
          
      return line;
    });
  }
  
  function sortSelectors(lines) {
    /** [parameterTrimmed, [line, line, line, line]] */
    let blocks = [];
    /** [line, line, line, line] **/
    let buffer = [];
    let lineNumTracker = 0;
    let parameterTrimmed = "";
    let len = lines.length;
        
    for ( let i = 0; i < len; i++ ) {
      let line = lines[i];
      let lineNum = line[2];
            
      if (!parameterTrimmed && line[0].includes("{") ) {
        parameterTrimmed = line[0];
      }
            
      if (lineNum !== lineNumTracker) {
        lineNumTracker++;
        blocks.push([parameterTrimmed, buffer]);
        buffer = [];
        parameterTrimmed = "";
      }
            
      buffer.push(line);
    }
        
    return blocks.sort();
  }
  
  function sortParameters(blocks) {
    const IS_PARAMETER = 3;
    for ( let key in blocks ) {
      let lines = blocks[key][1];
      let lineBuffer = [];
      let sortBuffer = [];
            
      for (let line of lines) {
                
        if (line[IS_PARAMETER]) {
          sortBuffer.push(line);
        } else {
          lineBuffer = lineBuffer.concat(sortBuffer.sort());
          lineBuffer.push(line);                    
        }
        
        blocks[key][1] = lineBuffer;
      }
      return blocks;
    }
  }
  
  return alphabetize;  
})();

window.addEventListener('DOMContentLoaded', (e) => {
  let css = document.getElementById('css');
  let alphabetize = document.getElementById('alphabetize');
    
  // TODO: Combo box that loads tests.
  alphabetize.addEventListener('click', function(e) {
    css.value = styleSorter(css.value);
  });
});
body {
    margin: 1em;
    width: 95%;
    max-width: 700px;
    font-family: sans-serif;
}

textarea {
    width: 100%;
    height: 360px;
    tab-size: 4;
}
<!DOCTYPE html>

<html lang="en-us">

<head>
    <meta charset="UTF-8">
    <title>CSS Alphabetizer</title>
    <link rel="stylesheet" href="style.css" />
    <script type="module" src="css-alphabetizer.js"></script>
</head>

<body>
    <h1>
    CSS Alphabetizer
    </h1>
    
    <p>
    Beautify your CSS. Sort your selectors alphabetically (which groups together #id's, .classes, and then elements), and sort your parameters alphabetically too. Parameters are nested within braces, so those aren't easy to sort with a normal A-Z sort.
    </p>
    
    <p>
    <textarea id="css">body {
    background-color: #999999;
    margin-bottom: 0;
}

#page-container {
    background-color: #DFDFDF;
    width: 1150px;
    margin: 12px auto 0 auto;
}

#page-container2 {
    padding: 12px 12px 0 12px;
}

header {
    background-color: white;
}

.category {
    display: inline-block;
    padding: 18px 25px;
    font-family: sans-serif;
}

.category:hover {
    background-color: #059BD8;
}</textarea>
    </p>
    
    <p>
    <button id="alphabetize">Alphabetize</button>
    </p>
    
    <p>
    Want to report a bug or request a feature? <a href="https://github.com/RedDragonWebDesign/css-alphabetizer/issues">Create an issue</a> on our GitHub.
    </p>
</body>

</html>

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the detailed response. I'll incorporate a lot of these changes. If you get a minute, can you give a summary of the advantages and disadvantages of functional programming? So far, I like that functions like .map() make code more concise. And I guess having many small functions is good for self documenting code. But I do not like having to spend mental effort on deciding if a variable is going to change or not (let vs const). And the const = function structure used above looks similar to a class. I also think that clubbing together statements might sometimes be less readable. \$\endgroup\$ – RedDragonWebDesign Jul 27 '20 at 14:06
  • \$\begingroup\$ The const function looks like a class; yes, but it returns 1 function. let vs. const has actually saved me a few times already where I was unwittingly overriding a variable. Finally, yes, clubbing is a matter of personal taste but can also prevent the creation of 1 time use variables. \$\endgroup\$ – konijn Jul 30 '20 at 10:54
3
\$\begingroup\$

To be honest, I didn't read all of the code. I gave up half way through. I believe this is due to two points:

  • You are not using the established CSS terms, e.g. "block" instead of "rule" and "parameter" instead of "declaration".

  • Most of the comments confused me more than they helped.

Additionally the parsing is too simple. It will choke on unexpected braces, for example content: "}", and on group rules such as media queries.

\$\endgroup\$
2
  • \$\begingroup\$ I feel your pain, could have been a comment? \$\endgroup\$ – konijn Jul 27 '20 at 10:39
  • \$\begingroup\$ Thanks for the honest feedback. I didn't realize that my code is hard to read, so that's good to know. I also added the 2 bugs you mentioned to my GitHub issues. \$\endgroup\$ – RedDragonWebDesign Jul 27 '20 at 14:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.