Code Review Asked on December 2, 2021
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])/, "}nn$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>
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 /* } */value.includes("{")
of courseIf 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])/, "}nn$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>
Answered by konijn on December 2, 2021
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.
Answered by RoToRa on December 2, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP