CSS stringify omits last semicolon in property declarations #49

Closed
kristoferjoseph opened this Issue Mar 11, 2013 · 9 comments

Projects

None yet

4 participants

@kristoferjoseph

After running rework on a css file the last property in the declarations is missing a semicolon.

Steps to reproduce

  1. run rework on a css file
.foo {
    background: var(var-background-color);
    border: 1px solid var(var-border-color);
}
  1. Observe results

Expected results

.foo {
    background: blue;
    border: 1px solid black;
}

Actual results

.foo {
    background: blue;
    border: 1px solid black
}

*notice the semicolon is missing

@ForbesLindesay

I don't think the semi-colon needs to be there for minified output. It should be there for un-minified output though.

@tj
Member
tj commented Mar 11, 2013

it'll break a bunch of tests to change now so im kinda meh about it haha, but I would agree that uncompressed might as well leave it on

@kristoferjoseph

I hear you. It would just mean updating the uncompressed *.css.out files. You either do it in rework or update the uncompressed files by hand, which is what I've been doing shrug

@tj
Member
tj commented Mar 11, 2013

well if you're using rework already there's no reason to even touch the output files really, that's not invalid CSS

@kristoferjoseph

You're right it isn't invalid css, but a lot of people work with strict coding guidelines. Adding a semicolon is more "correct", meaning consistent. Would you accept a pull request if I "fixed" this for uncompressed files?

@tj
Member
tj commented Mar 11, 2013

yeah I'm fine with it we would just have to update the tests in rework/css-stringify/rework-mixins etc

@kristoferjoseph

OK. Cool. Will put this together.

@sindresorhus

👍

@tj
Member
tj commented Jun 2, 2013

updated

@tj tj closed this Jun 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment