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

Preserve case of css custom variables. Fixes #648 #649

Merged

Conversation

p-a
Copy link
Contributor

@p-a p-a commented Oct 25, 2018

Fix matches var(--somename) and preserve case of the variable name.

Fixes #648

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Good work! Maybe we need check other places in postcss-merge-longhand on same problem. Can you be champion this?

@coveralls
Copy link

coveralls commented Oct 25, 2018

Coverage Status

Coverage increased (+0.001%) to 99.201% when pulling 08817ef on p-a:css-custom-property-preserve-case into 1b27f28 on cssnano:master.

@andyjansson
Copy link
Contributor

What happened to the diff?

@p-a
Copy link
Contributor Author

p-a commented Oct 25, 2018

@evilebottnawi Thanks! Well, sure, but I'm not sure when I will be able to find time to do it! Will put it on my TODO list!

@andyjansson sorry, VS Code must have "helped out" with whitespaces. EDIT: Apparently line breaks are CRLF à la Windows in the original file. On my Mac the default is just LF so VS Code changed all line breaks...

@p-a
Copy link
Contributor Author

p-a commented Oct 27, 2018

AFAICT there was only one place which lowercased strings, and that was taken care of by the initial commit. However, I added some more tests to ensure this.

@alexander-akait
Copy link
Member

/cc @andyjansson

@andyjansson
Copy link
Contributor

It's fine. In the long run, I'd probably prefer distinct formatting functions for widths, styles, and colors so we can handle any potential edge cases.

@p-a
Copy link
Contributor Author

p-a commented Oct 31, 2018

When do you expect to make the merge? Will there also be a new minor release after that? (would be great)

@alexander-akait
Copy link
Member

@p-a Today

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.

None yet

4 participants