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

[NETBEANS-1644] Implement a minimal CSS beautifier #3060

Merged
merged 1 commit into from Jul 23, 2021

Conversation

matthiasblaesing
Copy link
Contributor

No description provided.

@matthiasblaesing matthiasblaesing added the CSS [ci] enable web job label Jul 18, 2021
@matthiasblaesing
Copy link
Contributor Author

@Chris2011 could you please give this a spin? This is a minimal approach to a CSS beautifier, that for now does not offer options to configure it, but IMHO improves the situation regarding CSS code, if you have minified or badly formatted CSS.

@Chris2011
Copy link
Contributor

Thx @matthiasblaesing will give it a try.

@ebarboni
Copy link
Contributor

seems ok would you make it for 12.5 ?

@Chris2011
Copy link
Contributor

Chris2011 commented Jul 21, 2021

I wanted to test the PR, but I can't build NB. Is this also working for CSS inside HTML files? From the code, it doesn't look like that, or do I miss smth?.

@Chris2011
Copy link
Contributor

Ok you tested this inside of a twig file.

@matthiasblaesing
Copy link
Contributor Author

This should work in all files where CSS is embedded and netbeans "knows" that it is CSS. The idea is, that property wrapping is only enabled inside rule blocks. Property definitions in style attributes will not be wrapped.

I created a test build, which can be found here:

https://doppel-helix.eu/NetBeans-dev-dev-8cd0778dd680165cd766e932a71fb2c00d9f864c-release.zip

That is current master (cca3859) merged with the head of this PR.

@Chris2011
Copy link
Contributor

Chris2011 commented Jul 21, 2021

Thx, it is working just fine in a CSS and SCSS file, just inside of an html file it looks like so:

Before:

<style>body{color:red;}</style>

After:

<style>body{
            color:red
        }</style>

But I would expect smth like this:

<style>
body{
    color:red
}
</style>

And also, no spaces where added but as you said, it is just a minimal one so having body{ instead of body { is better than nothing.

An Idea for later because of using LSP and already node and typescript etc. doesn't it make sense for a later version to use things that are already there and where used for such things instead of using Java? Just an idea. I mean using prettier where you just have a config make much more sense as to create it again by your own.

@matthiasblaesing
Copy link
Contributor Author

@Chris2011 Thanks for checking. Yes handling of embedded source is not optimal, but I have to see if it is possible to discern the cases:

  • CSS/SCSS/LESS stand alone
  • CSS in style tag
  • CSS in style attribute
  • CSS recognized in ID attributes
  • CSS in style tags, that contain other embedded code sequences (PHP)

The current state should be robust enough to work in any of these situations, but especially the last case makes it difficult to handle.

For the usage of node based tools: From my POV a straight no. NetBeans has a single hard dependency on an installed node when it comes to the typescript support. In that case I can live with it, as you'll need node anyway to do anything relevant with typescript sources anyway. The same is not true for CSS. CSS is used in JSF, JSP and JavaFX and for these groups there is zero reason to install a secondary tool, just to do some CSS formatting.

@Chris2011
Copy link
Contributor

So from my pov while I testet HTML files with <style> tag (formatting as shown), inline style (no formatting which is correct), in css and scss file, it looks good.

@matthiasblaesing matthiasblaesing added this to the 12.5 milestone Jul 22, 2021
@matthiasblaesing
Copy link
Contributor Author

@Chris2011 thank you for your input. I'll squash the commits and will merge, when test come back green. I'll look into a second approach using the CSS parser to format, but the HTML integration is a bit strange and I'll have to stare at bit longer at that.

@matthiasblaesing matthiasblaesing merged commit c4e729a into apache:master Jul 23, 2021
@matthiasblaesing matthiasblaesing deleted the css-formatter3 branch July 23, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS [ci] enable web job
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants