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

Fix issue #37 (removing spaces on !important) #55

Merged
merged 7 commits into from
Apr 11, 2013
Merged

Fix issue #37 (removing spaces on !important) #55

merged 7 commits into from
Apr 11, 2013

Conversation

tml
Copy link
Contributor

@tml tml commented Mar 18, 2013

No description provided.

@evocateur
Copy link

This pull request has way too many disparate things going on. It should not include the color keyword changes or the trailing comma bits. Those are other pull requests.

@@ -218,6 +218,8 @@ public void compress(Writer out, int linebreakpos)
css = sb.toString();
// Remove spaces before the things that should not have spaces before them.
css = css.replaceAll("\\s+([!{};:>+\\(\\)\\],])", "$1");
// Restore spaces for !important
css = css.replaceAll("!important", " !important");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks fine. All others (save for the travis notifications) should be removed from this pull request.

Also, do we have tests that verify this change?

@tml
Copy link
Contributor Author

tml commented Apr 4, 2013

@evocateur Thanks for reviewing - those other changes are actually things that have already been merged into YUICompressor, they're just showing up here because I merged them into my own repo after the PR had already been issued. Is there an easy way to let the PR show that, or is it best to close this PR and start a new one?

I have a test, but it looks like I didn't have it on the PR; I'll fix that.

@tml
Copy link
Contributor Author

tml commented Apr 4, 2013

65ae061 and bc8cbae are the color commits, d6d8797 is the trailing commas code

@evocateur
Copy link

Oh, I see. You should probably merge upstream changes into this pull request branch, then. I think GitHub will magically notice and remove those commits from this pull request.

Assuming upstream is the remote for yui/yuicompressor:

git checkout master
git pull
git fetch upstream
git merge upstream/master master
git push origin master
git checkout pr37
git merge master
git push origin pr37

@tml
Copy link
Contributor Author

tml commented Apr 4, 2013

Pretty sure that's what I did last time that resulted in this weird state, but I just tried it again...doesn't seem to have changed anything for me?

@evocateur
Copy link

Looking at https://github.com/yui/yuicompressor/pull/55/files now, it only shows the change to CssCompressor.java. I'd say that did the trick. (As an added bonus, now the GitHub UI for merging this pull request won't fail)

tml added a commit that referenced this pull request Apr 11, 2013
Fix issue #37 (removing spaces on !important)
@tml tml merged commit 110179c into yui:master Apr 11, 2013
@tml tml deleted the pr37 branch April 11, 2013 20:26
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

Successfully merging this pull request may close these issues.

4 participants