Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minification of margin and padding properties #17

Closed
hmert opened this issue Aug 7, 2011 · 7 comments
Closed

Minification of margin and padding properties #17

hmert opened this issue Aug 7, 2011 · 7 comments

Comments

@hmert
Copy link

hmert commented Aug 7, 2011

Could csso minify margins which's not set property?
like:

`````` .test{margin-left:4px;margin-bottom:4px;margin-top:14px;}to.test{margin:14px 0 4px 4px;}```

@Leonya
Copy link
Contributor

Leonya commented Aug 7, 2011

No, it cannot be minified like this because 'not set' does not necessarily mean '0'.

@hmert
Copy link
Author

hmert commented Aug 7, 2011

OK, but may give an option to choose this minification metod?

@Leonya
Copy link
Contributor

Leonya commented Aug 7, 2011

What do you mean? not set -> 0 is an invalid assumption, optional or not. Let's say you have two classes: .test { margin: 3px } and .test2 { margin-top: 4px; margin-right: 4px; margin-bottom: 4px }, you apply them both to an element: div class="test test2". In this case, test2 overrides all margin properties set in test, except for margin-left which is 3px. If test2 was minified like you suggested, we'd end up with the wrong value of 0.

All the minification done by CSSO strictly adheres to the CSS specification. Perhaps our docs are misleading when we label basic minifications as 'safe' and the more advanced ones as 'structural' - actually all of them are safe and must stay such. I'll update the docs.

@afelix
Copy link
Contributor

afelix commented Aug 7, 2011

I'm agree with Leonya, it is very unsafe to minify margins this way.

In any case the matter of CSS compressing is to compress CSS, not to correct it or to insert some absent values, sorry.

@hmert
Copy link
Author

hmert commented Aug 7, 2011

Leonya: at your case you're absolutely right and I totally agree with you. But my case is different from the general use. I do not need a safe minification. I know what CSSO will do because I'd allow to do that.

at *nix, you can remove files with force metod "rm -rf", that look like what I want. Advanced users may want too.

I forked the project. I'd add that option for me :) Thanks anyway, CSSO works perfectly.

@afelix
Copy link
Contributor

afelix commented Aug 7, 2011

hmert: keep in mind that you will have to fork it once again in september, it is CSSO2 in active development now — completely another codebase.

Please, comment this issue with your option implementation link after you will implement it. We will try to rethink this issue at least. :) Thank you.

@Leonya
Copy link
Contributor

Leonya commented Aug 7, 2011

hmert: well, feel free to implement it in your fork, but I still don't get your reasoning. If you are going to use it for your own HTML/CSS which is "immune" to this problem, then why don't you write the CSS in the shorthand form in the first place?

@hmert hmert closed this as completed Jul 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants