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

Add or preserve empty line between nested SCSS rules #1258

Closed
alejandroiglesias opened this issue Sep 20, 2017 · 17 comments
Closed

Add or preserve empty line between nested SCSS rules #1258

alejandroiglesias opened this issue Sep 20, 2017 · 17 comments

Comments

@alejandroiglesias
Copy link

alejandroiglesias commented Sep 20, 2017

Description

Even though I'm using newline_between_rules, it works only for top level (ie, zero-indented) rules (as confirmed by this changeset for the newline_between_rules PR). It's highly useful for SCSS code to keep newlines also for nested rules, so I think this would be a feature request. Basically, I think that if the newline_between_rules option is true, it should keep the newlines also for the nested rules. Until this gets changed, I sadly need to disable it for my SCSS code.

Input

The code looked like this before beautification:

.list-group {
  .list-group-item {
  }
  
  .list-group-icon {
  }
}

.list-group-condensed {
}

Expected Output

The code should have looked like this after beautification:

.list-group {
  .list-group-item {
  }

  .list-group-icon {
  }
}

.list-group-condensed {
}

Actual Output

The code actually looked like this after beautification:

.list-group {
  .list-group-item {
  }
  .list-group-icon {
  }
}

.list-group-condensed {
}

Steps to Reproduce

Beautify SCSS file with example input.

Environment

VSCode Beautify plugin version 1.1.1.

Settings

Example:

{
    "end_with_newline": true,
    "brace_style": "collapse,preserve-inline",
    "max_preserve_newlines": 2,
    "newline_between_rules": true,
    "space_around_combinator": true,
    "space_after_anon_function": true,
    "wrap_line_length": 100,
    "css": {
        "indent_size": 2
    }
}
@davemac
Copy link

davemac commented Oct 31, 2017

This issue is a showstopper for me, would be great to see it fixed - otherwise I am forced to disable SCSS beautification as well.

@bitwiseman
Copy link
Member

@davemac - Any help you can provide would be appreciated.

@MarkSky
Copy link

MarkSky commented Nov 3, 2017

Yes, I want this, too.

Now:

.preloader {
	height: 20px;
	.line {
		width: 1px;
		height: 12px;
		background: #38a0e5;
		margin: 0 1px;
		display: inline-block;
		&.line-1 {
			animation-delay: 800ms;
		}
		&.line-2 {
			animation-delay: 600ms;
		}
	}
	div {
		color: #38a0e5;
		font-family: 'Arial', sans-serif;
		font-size: 10px;
		margin: 5px 0;
	}
}

Expected:

.preloader {
    height: 20px;

    .line {
        width: 1px;
        height: 12px;
        background: #38a0e5;
        margin: 0 1px;
        display: inline-block;

        &.line-1 {
            animation-delay: 800ms;
        }

        &.line-2 {
            animation-delay: 600ms;
        }
    }

    div {
        color: #38a0e5;
        font-family: 'Arial', sans-serif;
        font-size: 10px;
        margin: 5px 0;
    }
}

Hope it will supported soon.

@alffer
Copy link

alffer commented Apr 19, 2018

+1

1 similar comment
@ricmas
Copy link

ricmas commented Apr 19, 2018

+1

@davemac
Copy link

davemac commented Apr 20, 2018

I'd like to help fix this issue, but not sure I have the correct skillset. What needs to be done to get this issue resolved?

@bitwiseman
Copy link
Member

bitwiseman commented Apr 20, 2018

@davemac
The main skill you need is either Javascript or Python (and a willingness to write a little of the other language). See the Contibuting.md and if that does not have enough information, please help update that with sufficient information for new contributors.

@lacroixdavid1
Copy link

+1

@lacroixdavid1
Copy link

@davemac I suggest you to take a look at original PR 44df1f1

@alphabt
Copy link

alphabt commented Aug 2, 2018

+1

@bitwiseman
Copy link
Member

@davemac @iczman
This issue is at least partially addressed in 1.8.0-rc4.
The preserve_newlines setting now works so if there are newlines in the original input the beautifier can be set to not remove them. However, It still does not add the empty line if it is missing.

@MacKLess
Copy link
Collaborator

MacKLess commented Aug 13, 2018

Resolved with the collapsing of empty {}. Here is my output, run on v1.8.0-rc5:

.list-group {
  .list-group-item {}

  .list-group-icon {}
}

.list-group-condensed {}

@alejandroiglesias, please feel free to check it yourself (http://jsbeautifier.org/)

@MarkSky I'm still working on addressing your example. Feel free to look over the code and see what it looks like for you (http://jsbeautifier.org/)

@alejandroiglesias
Copy link
Author

alejandroiglesias commented Aug 14, 2018

@MacKLess thanks for the heads up, it seems to be working as requested according to a quick test with the input I provided, but I'm not using JS Beautifier on an actual project to test thoroughly. Almost a year passed now and I had to find a solution that worked for me. In any case, it seems to be still valuable for a lot of people who requested it.

@alphabt
Copy link

alphabt commented Aug 14, 2018

I verified that it no longer removes newline on http://jsbeautifier.org (thanks!), but it would be even better if it automatically adds newline.

@MacKLess
Copy link
Collaborator

@MarkSky & @iczman

The force newline now appears to be working. Here's the output run on the v1.8.0-rc7 (with my PR):

.preloader {
  height: 20px;

  .line {
    width: 1px;
    height: 12px;
    background: #38a0e5;
    margin: 0 1px;
    display: inline-block;

    &.line-1 {
      animation-delay: 800ms;
    }

    &.line-2 {
      animation-delay: 600ms;
    }
  }

  div {
    color: #38a0e5;
    font-family: 'Arial', sans-serif;
    font-size: 10px;
    margin: 5px 0;
  }
}

If my PR is approved, you should be able to try it out for yourselves. Let me know if this doesn't address the issue.

@bitwiseman bitwiseman modified the milestones: v1.8.x, 1.8.0-rc8, 1.8.0 Aug 16, 2018
@shatkovski
Copy link

Is there an option to enable the classic behaviour?

I wish to get this output:

.list-group {
  .list-group-item {}
  .list-group-icon {}
}

.list-group-condensed {}

@bitwiseman
Copy link
Member

@shatkovski Sorry, there is no setting to get the classic behavior.
It would likely be easy to do.
Please open a new issue requesting it.

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

No branches or pull requests

10 participants