Add expand brace-style option to css beautifier #1796

ang-zeyu · 2020-05-08T14:11:37Z

Description

Source branch in your fork has meaningful name (not master)

Fixes Issue: #1776 ( adding a newline before the opening curly brace of selector(s) )
#1353

#1259 (partially, till the other options are implemented)

~~Name of the new option: curly-start-newline~~
~~Shorthand: -R for the cur in curly~~

Added brace-style=expand to the css beautifier

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

JavaScript implementation
Python implementation (NA if HTML beautifier)
Added Tests to data file(s)
Added command-line option(s) (NA if
README.md documents new feature/option(s)

ang-zeyu · 2020-05-11T01:11:54Z

js/src/css/beautifier.js

      }
      if (this._options.newline_between_rules && insideRule) {
        if (this._output.previous_line && this._output.previous_line.item(-1) !== '{') {
          this._output.ensure_empty_line_above('/', ',');
        }
      }
+


moved this section below to prevent ( when using newline_between_rules )
.a{} .b{} -->

.a /* there shouldn't be an empty line here */ {} .b /* there shouldn't be an empty line here */ {}

On the other hand the branching is to indent the { correctly

bitwiseman · 2020-05-18T22:38:22Z

README.md

@@ -323,6 +323,7 @@ CSS Beautifier Options:
  -e, --eol                          Character(s) to use as line terminators. (default newline - "\\n")
  -n, --end-with-newline             End output with newline
  -L, --selector-separator-newline   Add a newline between multiple selectors
+  -C, --curly-start-newline          Add a newline before the opening { bracket of selector(s)


What do you think of reusing the brace-style setting instead of create a new field specific to the css beautifier?
I ask because the formatting curly-start-newline creates is the same as brace-style=expand. The default behavior matches brace-style=collapse (or end-expand but these are the same in css.)

Don't change anything yet, I'd like to discuss and see if what I'm thinking makes sense to you.

Thanks for looking at this!

Think it would be more consistent to use brace-style as well (I missed it out before).
Perhaps a trimmed set of options like [collapse|expand|none], since as you mentioned end-expand dosen't make sense in css. I'm not too sure if preserve-inline makes sense though. (adding css to a minified file while preserving the minified styles maybe?)

I could expand the scope of this to include support for these if needed, or in separate PRs. Let me know what you think!

@ang-zeyu
I think it would be fine to start with support for just collapse and expand (and treat end-expand and none the same as collapse). Then you could open an issue to add support for preserve-inline and none and someone (or you) can do that at a later date in another PR.

That will get this cool new feature out for people to use with minimal added work for you now. Sound good?

Yup! will update this with brace-style=[collapse|expand] as a start then

bitwiseman

This is a great start!

ang-zeyu · 2020-05-30T11:37:50Z

ready for another look @bitwiseman

I noticed braces_on_own_line is still supported in the javascript options though, should that be added here as well?

bitwiseman · 2020-05-31T22:50:16Z

@ang-zeyu
re braces_on_own_line: no. You're implementing something new, backward compatibility is not your concern. If someone wants to use your feature, it is fine to require them to use current settings.

bitwiseman · 2020-05-31T22:56:09Z

test/data/css/tests.js

          input: 'a:first-child,a:first-child{color:red;div:first-child,div:hover{color:black;}}\na:first-child,a:first-child{color:red;div:first-child,div:hover{color:black;}}',
-          output: 'a:first-child,{{separator}}a:first-child {\n    color: red;{{new_rule}}\n    div:first-child,{{separator1}}div:hover {\n        color: black;\n    }\n}\n{{new_rule}}a:first-child,{{separator}}a:first-child {\n    color: red;{{new_rule}}\n    div:first-child,{{separator1}}div:hover {\n        color: black;\n    }\n}'


Excellent, thanks for cleaning up this formatting.

bitwiseman · 2020-05-31T23:04:06Z

js/src/css/options.js

@@ -38,6 +38,11 @@ function Options(options) {
  var space_around_selector_separator = this._get_boolean('space_around_selector_separator');
  this.space_around_combinator = this._get_boolean('space_around_combinator') || space_around_selector_separator;

+  var brace_style_split = this._get_selection_list('brace_style', ['collapse', 'expand']);


You'll need to accept the other values that are allowed by the javascript options, otherwise you'll get an error if someone passes them (via inheriting the option). You can ignore or convert them loop below.

👍 done. Linked two more related issues as well

the below commit reverts the jsbeautifyrc override

bitwiseman · 2020-07-07T23:25:01Z

js/src/css/options.js

+  this.brace_style = 'collapse';
+  for (var bs = 0; bs < brace_style_split.length; bs++) {
+    if (brace_style_split[bs] !== 'expand') {
+      // default to collapse, as only collapse|expand is implemented for now


bitwiseman

Sorry about the wait. Looks good.

ang-zeyu force-pushed the curly_start_newline branch 8 times, most recently from 1d2f12d to f682354 Compare May 11, 2020 01:06

ang-zeyu commented May 11, 2020

View reviewed changes

bitwiseman reviewed May 18, 2020

View reviewed changes

ang-zeyu force-pushed the curly_start_newline branch 2 times, most recently from 34f2dcc to ed044c4 Compare May 30, 2020 11:19

ang-zeyu changed the title ~~Add newline at css selector curly bracket option~~ Add expand brace-style option to css beautifier May 30, 2020

bitwiseman reviewed May 31, 2020

View reviewed changes

ang-zeyu force-pushed the curly_start_newline branch from ed044c4 to de50dc9 Compare June 4, 2020 13:17

Add expand brace-style option to css beautifier

5cb5390

ang-zeyu force-pushed the curly_start_newline branch from de50dc9 to 5cb5390 Compare June 4, 2020 13:25

ang-zeyu requested a review from bitwiseman July 2, 2020 15:38

bitwiseman reviewed Jul 7, 2020

View reviewed changes

bitwiseman approved these changes Jul 7, 2020

View reviewed changes

bitwiseman merged commit 49a4a4a into beautifier:master Jul 7, 2020

bitwiseman added this to the v1.11.x milestone Jul 7, 2020

bitwiseman linked an issue Jul 7, 2020 that may be closed by this pull request

Newline rule not working with css-like files #1776

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add expand brace-style option to css beautifier #1796

Add expand brace-style option to css beautifier #1796

ang-zeyu commented May 8, 2020 •

edited

Loading

ang-zeyu May 11, 2020

bitwiseman May 18, 2020

ang-zeyu May 20, 2020

bitwiseman May 28, 2020

ang-zeyu May 29, 2020

bitwiseman left a comment

ang-zeyu commented May 30, 2020

bitwiseman commented May 31, 2020

bitwiseman May 31, 2020

bitwiseman May 31, 2020 •

edited

Loading

ang-zeyu Jun 4, 2020 •

edited

Loading

bitwiseman Jul 7, 2020

bitwiseman left a comment

		input: 'a:first-child,a:first-child{color:red;div:first-child,div:hover{color:black;}}\na:first-child,a:first-child{color:red;div:first-child,div:hover{color:black;}}',
		output: 'a:first-child,{{separator}}a:first-child {\n color: red;{{new_rule}}\n div:first-child,{{separator1}}div:hover {\n color: black;\n }\n}\n{{new_rule}}a:first-child,{{separator}}a:first-child {\n color: red;{{new_rule}}\n div:first-child,{{separator1}}div:hover {\n color: black;\n }\n}'

Add expand brace-style option to css beautifier #1796

Add expand brace-style option to css beautifier #1796

Conversation

ang-zeyu commented May 8, 2020 • edited Loading

Description

Before Merge Checklist

ang-zeyu May 11, 2020

Choose a reason for hiding this comment

bitwiseman May 18, 2020

Choose a reason for hiding this comment

ang-zeyu May 20, 2020

Choose a reason for hiding this comment

bitwiseman May 28, 2020

Choose a reason for hiding this comment

ang-zeyu May 29, 2020

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

ang-zeyu commented May 30, 2020

bitwiseman commented May 31, 2020

bitwiseman May 31, 2020

Choose a reason for hiding this comment

bitwiseman May 31, 2020 • edited Loading

Choose a reason for hiding this comment

ang-zeyu Jun 4, 2020 • edited Loading

Choose a reason for hiding this comment

bitwiseman Jul 7, 2020

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

ang-zeyu commented May 8, 2020 •

edited

Loading

bitwiseman May 31, 2020 •

edited

Loading

ang-zeyu Jun 4, 2020 •

edited

Loading