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

Inline comments should be kept inline #417

Closed
maxstarkenburg opened this issue Jul 20, 2018 · 0 comments · Fixed by #849
Closed

Inline comments should be kept inline #417

maxstarkenburg opened this issue Jul 20, 2018 · 0 comments · Fixed by #849
Assignees
Labels
cosmetic Doesn't affects CSS semantics enhancement help wanted

Comments

@maxstarkenburg
Copy link

@maxstarkenburg maxstarkenburg commented Jul 20, 2018

This is a "repeat" of the ticket made at sass/sass#235 which presented the problem of code like:

.myselector {
  display: none; /* javascript will fix this */
}

compiling to:

.myselector {
  display: none;
  /* javascript will fix this */
}

That ticket was moved to ruby-sass and closed as on ice since it was deemed to be specific to Ruby Sass's deprecated implementation. However, I'm seeing it happen for me despite using Dart Sass. I use --style expanded for what it's worth.

My apologies if this is somehow being caused by interference with an old and probably only partially working ruby (when I run sass --help I am seeing references to Dart, e.g. --version Print the version of Dart Sass., though don't know if there's still possible room for interference, or how I might resolve that). Thanks for any possible work on this, or letting me know if there is simply some flag or style setting whose documentation I missed.

@nex3 nex3 added help wanted cosmetic Doesn't affects CSS semantics enhancement labels Jul 20, 2018
@nickbehrens nickbehrens self-assigned this Oct 16, 2019
nickbehrens pushed a commit to nickbehrens/dart-sass that referenced this issue Oct 16, 2019
nickbehrens added a commit to nickbehrens/dart-sass that referenced this issue Oct 16, 2019
nickbehrens added a commit to nickbehrens/dart-sass that referenced this issue Oct 16, 2019
nickbehrens added a commit to nickbehrens/dart-sass that referenced this issue Oct 16, 2019
nex3 added a commit to sass/sass-spec that referenced this issue Oct 16, 2019
nex3 added a commit to sass/sass-spec that referenced this issue Oct 16, 2019
@Goodwine Goodwine self-assigned this May 31, 2022
Goodwine pushed a commit to nickbehrens/dart-sass that referenced this issue May 31, 2022
parent 1a5102b
author Nick Behrens <nbehrens@google.com> 1571200514 -0700
committer Carlos Israel Ortiz García <goodwine@google.com> 1654024100 -0700

Delete now unused _indent helper method in serialize.dart.

Update lib/src/visitor/serialize.dart to stop using old-style int-based for loop.

Co-Authored-By: Natalie Weizenbaum <nweiz@google.com>

Address PR feedback.

Revert parser changes.  Instead look at node span line numbers to determine whether a loud comment is trailing.

Add some more test cases.

Remove TODO in _isTrailingComment helper and add a better comment in there.

Revert one unintentional chunk of edits to lib/src/parse/sass.dart from commit d937f87.

Address some review feedback

Update lib/src/visitor/serialize.dart to use var instead of CssNode as for loop variable.

Co-Authored-By: Natalie Weizenbaum <nweiz@google.com>

Addressing review feedback

Remove some now irrelevant comments after last commit.

Add some comments on some code I'd like some feedback on in the PR.

Fix typo in recently added comment.

Remove dupe impl of beforeChildren from CssStylesheet and some other minor cleanup.

Rewrite _visitChildren to be simpler.

Remove duplicate math import.

Restore for loop in visitChildren to how it used to be.

Comment cleanup.

Don't add a trailing newline after an only-child trailing comment

Minor style tweaks

Short-circuit _isTrailingComment in compressed mode

Update isTrailingComment to handle case where parent node contains left braces that don't open a child block.

Address PR feedback, fixing indexing issue in isTrailingComment.

Fix isTrailingComment to handle mixins that include loud comments.
Goodwine pushed a commit to nickbehrens/dart-sass that referenced this issue Jun 1, 2022
parent 1a5102b
author Nick Behrens <nbehrens@google.com> 1571200514 -0700
committer Carlos Israel Ortiz García <goodwine@google.com> 1654024100 -0700

Delete now unused _indent helper method in serialize.dart.

Update lib/src/visitor/serialize.dart to stop using old-style int-based for loop.

Co-Authored-By: Natalie Weizenbaum <nweiz@google.com>

Address PR feedback.

Revert parser changes.  Instead look at node span line numbers to determine whether a loud comment is trailing.

Add some more test cases.

Remove TODO in _isTrailingComment helper and add a better comment in there.

Revert one unintentional chunk of edits to lib/src/parse/sass.dart from commit d937f87.

Address some review feedback

Update lib/src/visitor/serialize.dart to use var instead of CssNode as for loop variable.

Co-Authored-By: Natalie Weizenbaum <nweiz@google.com>

Addressing review feedback

Remove some now irrelevant comments after last commit.

Add some comments on some code I'd like some feedback on in the PR.

Fix typo in recently added comment.

Remove dupe impl of beforeChildren from CssStylesheet and some other minor cleanup.

Rewrite _visitChildren to be simpler.

Remove duplicate math import.

Restore for loop in visitChildren to how it used to be.

Comment cleanup.

Don't add a trailing newline after an only-child trailing comment

Minor style tweaks

Short-circuit _isTrailingComment in compressed mode

Update isTrailingComment to handle case where parent node contains left braces that don't open a child block.

Address PR feedback, fixing indexing issue in isTrailingComment.

Fix isTrailingComment to handle mixins that include loud comments.
Goodwine added a commit that referenced this issue Jun 3, 2022
See sass/sass-spec#1485

- Update lib/src/visitor/serialize.dart to stop using old-style int-based for loop.
- Extend FileSpan with a .contains(targetSpan) method

Co-authored-by:  Nick Behrens <nbehrens@google.com>
Co-authored-by:  Carlos Israel Ortiz García <goodwine@google.com>
Co-Authored-By: Natalie Weizenbaum <nweiz@google.com>
Goodwine added a commit to sass/sass-spec that referenced this issue Jun 3, 2022
See Issue sass/dart-sass#417 
See PR sass/dart-sass#849

Co-authored-by: Carlos Israel Ortiz García <goodwine@google.com>
Co-Authored-By: Natalie Weizenbaum <nweiz@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cosmetic Doesn't affects CSS semantics enhancement help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants