Skip to content

Fix/move charset top #70

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix/move charset top #70

wants to merge 4 commits into from

Conversation

maxledoux
Copy link
Contributor

@charset must be on the first line of the concatenated file. However, the current code checks if @charset is already at position 0 (at the very beginning of the file). Only if @charset is already at the beginning will the code then attempt to move it to the beginning (where it already is located).

This PR changes if ( 0 === strpos( $buf, '@charset' ) ) { to false !== strpos( $buf, '@charset' ) so that it checks if @charset is present anywhere in $buf. Then it deletes the instance of @charset from $buf before the existing code adds the @charset to the beginning of $buf. Otherwise, we end up with multiple @charset declarations in the concatenated file. There may be better ways to avoid this.

The following is an example of how I tested this:

style1.css:

body  {
	font-size: 12px;
}

style2.css:

.span {
	color: #fff;
}
@charset "UTF-8";
.class {
	color: #000;
}

With the current code the code the concatenated file looks like:

body{font-size: 12px}@charset "UTF-8";.span {color: #fff}.class {color: #000}

Notice the current code does move @charset, but to the top of the contents of the original file, not the top of the concatenated file.

With the commits in this PR the concatenated file would like like:

@charset "UTF-8";
body{font-size: 12px}.span {color: #fff}.class {color: #000}

With my first commit but without my second commit (86331aa) it looked like:

@charset "UTF-8";
body{font-size: 12px}@charset "UTF-8";.span {color: #fff}.class {color: #000}

Hope this is helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants