ben-eb / perfectionist

Beautify CSS files.
MIT License
229 stars 14 forks source link

spaces in url() #24

Closed silverwind closed 8 years ago

silverwind commented 8 years ago

Given this input with the default options:

*{
  background: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAACCAYAAACZgbYnAAAAEklEQVQImWMQERFpYLC1tf0PAAgOAnPnhxyiAAAAAElFTkSuQmCC) right repeat-y!important;
}

There's a space added after base64,, which probably shouldn't be there:

* {
  background: url(data:image/png;base64, iVBORw0KGgoAAAANSUhEUgAAAAEAAAACCAYAAACZgbYnAAAAEklEQVQImWMQERFpYLC1tf0PAAgOAnPnhxyiAAAAAElFTkSuQmCC) right repeat-y !important;
}
Mottie commented 8 years ago

Odd, I just used v2.1.2 and I did not see this happen... I am using Windows, maybe that's the difference?

silverwind commented 8 years ago

@Mottie Try this in the project folder:

$ node
> require("perfectionist").process("*{background: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAACCAYAAACZgbYnAAAAEklEQVQImWMQERFpYLC1tf0PAAgOAnPnhxyiAAAAAElFTkSuQmCC) right repeat-y!important;}").css
'* {\n    background: url(data:image/png;base64, iVBORw0KGgoAAAANSUhEUgAAAAEAAAACCAYAAACZgbYnAAAAEklEQVQImWMQERFpYLC1tf0PAAgOAnPnhxyiAAAAAElFTkSuQmCC) right repeat-y !important;\n}\n'

Oh, and make sure you ran npm install ;)

Mottie commented 8 years ago

Hmm, so I found a way to fix the problem, but I haven't really used npm as a build tool before... Without making any changes, I ran npm run test and it failed; maybe because I'm using Windows?

Anyway, to fix this issue I added the following code in index.js in two places (after line 86-7 & 181-2):

// Remove spaces before commas and keep only one space after.
rule.value = rule.value.replace(/(\s+)?,(\s)*/g, ', ');
rule.value = rule.value.replace(/base64,\s+/ig, 'base64,'); // new code
silverwind commented 8 years ago

I think you're better off try to match if the comma is inside an url(), there are other encodings than base64.

silverwind commented 8 years ago

@Mottie did you run npm install before running npm run test?

Mottie commented 8 years ago

yes, I had the most recent version installed.

silverwind commented 8 years ago

Tests are passing for me in a Windows 10 VM, post your error :wink:

Mottie commented 8 years ago

I uninstalled the global version just in case... this was run in my forked repo

...
# tests 89
tests 89
# pass  42
pass  42
# fail  47

npm ERR! Windows_NT 6.3.9600
npm ERR! argv "C:\\Program Files\\nodejs\\\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run" "test-unformatted"
npm ERR! node v0.12.7
npm ERR! npm  v2.11.3
npm ERR! code ELIFECYCLE
npm ERR! perfectionist@2.1.2 test-unformatted: `babel-tape-runner "src/**/__tests__/*.js"`
npm ERR! Exit status 1
npm ERR!
...
silverwind commented 8 years ago

Hmm I'm not seeing your fork on GitHub and above log looks like real failures, nothing about your setup.

Maybe try a fresh copy to be sure:

git clone https://github.com/ben-eb/perfectionist.git
cd perfectionist
npm install
npm run test

Oh, and update node :smiling_imp:

Mottie commented 8 years ago

LOL I didn't realize my npm was so old... anyway, I still get the error. Oh and I included everything LOL:

...
# tests 89
# pass  42
# fail  47

npm ERR! Windows_NT 10.0.10240
npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run" "test-unformatted"
npm ERR! node v4.2.1
npm ERR! npm  v2.14.7
npm ERR! code ELIFECYCLE
npm ERR! perfectionist@2.1.2 test-unformatted: `babel-tape-runner "src/**/__tests__/*.js"`
npm ERR! Exit status 1
...
silverwind commented 8 years ago

Oh, I see the issue, it's your git checking out CRLF but the tests expect LF. I suggest reinstalling git and selecting check out as-is, commit as-is during the installer.

Mottie commented 8 years ago

LOL, ok that did it, all tests pass now... I'm gonna delete the wall of spam in the comments above :laughing:

Mottie commented 8 years ago

So, I'm still not great with this npm build stuff... I think I found a better regex (example):

rule.value = rule.value.replace(/(\s*,\s*)(?=(?:[^"']|['"][^"']*["'])*$)/g, ', ');