SublimeText / Sass

Sass and SCSS syntax for Sublime Text
https://packagecontrol.io/packages/Sass
MIT License
51 stars 8 forks source link

Nested properties don't highlight correctly #16

Closed yb53 closed 5 years ago

yb53 commented 6 years ago

Hi,

Firsdt of all, thank you for your hard work. Here are just 2 trivial regressions from the previous version, the first one about property nesting and the second about function parameters.

nesting1

Except for "size", all other nested parameters (weight, style, radius, appearance, etc) are not recognized and end up constituing a funny color pattern.

functions1

As for the function parameter, the "$foo" is not identified and not highlighted.

Of course it is the same for this kind of declarations: =background-size($property)  -webkit-background-size: $property  -moz-background-size: $property   background-size: $property

Thank you again for your work, and if you ever have a sparetime with nothing to do, then perhaps~

braver commented 6 years ago

@yb53 Hi, thanks for the feedback!

Property nesting has been broken to fix something else. I need to revise how the syntax decides if something is a selector or a property to fix this correctly. Sadly it's not a trivial problem.

As for parameters, Monokai doesn't highlight variables: they're just white. So this is correct:

screen shot 2018-10-31 at 10 21 03

However, ideally $foo would be orange here, or variable.parameter instead of variable.other.

screen shot 2018-10-31 at 10 23 14

Anyway, hope you enjoy the improvements the new syntax brings. These and other improvements are sure to follow.

braver commented 6 years ago

for the parameters, please track #17

cem90 commented 6 years ago

I have a similar issue with mixins and comments within the sass-highlighting. Some mixins are highlighted correctly, some are not.

Comments are not even possible with the shortcut inside Sublime Text 3.

Am I missing sth. or is that a real issue?

For example:

bildschirmfoto 2018-10-31 um 11 37 49

bildschirmfoto 2018-10-31 um 11 36 03

bildschirmfoto 2018-10-31 um 11 44 41

braver commented 6 years ago

I think I'm missing some key syntax features here for Sass specifically, like the =<mixin> and +<include mixin>: http://sass-lang.com/documentation/file.INDENTED_SYNTAX.html#Mixin_Directives

That should be an easy fix, I'll let you know what I come up with

cem90 commented 6 years ago

Okay cool. Thx a lot!

Specifically for Sass sintax, there are some more highlighting issues... but that would be too much to show here.

braver commented 6 years ago

@raedlime 2.0.4 should become available soon with fixes:

cem90 commented 6 years ago

Okay, nice. Waiting for the upgrade then.

cem90 commented 6 years ago

Got the upgrade now. Comments are possible now, but the thing with the mixins highlighting still doesn't work properly.

Example v.2: bildschirmfoto 2018-10-31 um 17 07 32

Example: v.1: bildschirmfoto 2018-10-31 um 17 08 14

The highlighting for using the mixins works fine, as far as I can see. bildschirmfoto 2018-10-31 um 17 11 18

braver commented 6 years ago

@raedlime this should be fixed in 2.0.5 already (although the @content keyword is missing):

screen shot 2018-10-31 at 17 28 44
cem90 commented 6 years ago

mh, okay. Idk what's wrong with my sass then. But it looks like, either the @content keyword breaks every thing following or the variable.

dr-dimitru commented 6 years ago

@braver

Anyway, hope you enjoy the improvements the new syntax brings. These and other improvements are sure to follow.

Can't enjoy it, all broken in deeply nested trees 😞

But, I love your fork of the lib, and don't wanna go to original package. Any options? What last stable version?

braver commented 6 years ago

@dr-dimitru can you give me a test file so I can see what’s wrong?

dr-dimitru commented 6 years ago

@braver sure, actually I found exact line was causing it, right after multiple :not(/*...*/):

input:not([type="radio"]):not([type="checkbox"]):not([type="range"]):not([type="image"]), select, textarea
  display: block
  transition-property: background-color, border-color
  transition-duration: 256ms
  transition-timing-function: ease
  background-color: $white
  border: 1px solid lighten($black, 75%)
  padding: 6px 10px
  border-radius: 2px
  width: 100%
  min-width: 80px
  font-weight: 400

And commenting with double-slash //

You should be able to reproduce on your end, details:

michaelblyons commented 6 years ago

It looks like a single :not(span) is enough to trigger the problem. The meta.function-call meta.group stacks never pop.

:not(span)
  display: block
braver commented 6 years ago

Thanks guys, that should be fixable.

braver commented 6 years ago

The :not() issue fixed in 89dab3a

michaelblyons commented 6 years ago

89dab3a fixed OP's case and :not(.class). Is :not(tag) valid Sass? Because that still doesn't pop.

(Maintenance of a popular package seems to be a huge chore. 😉)

braver commented 6 years ago

Maintenance of a popular package seems to be a huge chore

Yeah, some maintainers tend to go with not actually doing the maintenance 😉 This is all good though, lot's of use cases I don't come across myself a lot. Just some things to get right and if I set it up correctly I think it'll be peanuts later.

dr-dimitru commented 6 years ago

@michaelblyons @braver :not() can be used with any selector class, id, element name

braver commented 6 years ago

@dr-dimitru @michaelblyons thanks guys, caught those as well now.

dr-dimitru commented 6 years ago

@braver thanks for quick update, testing now. Everything seems to be good, but still feels like something off, like color schema changed, or ... idk...

I'll report if something will popup later.

cem90 commented 6 years ago

Most of the highlighting works pretty awesome. Thanks for the work on the issues found last week (y) But, I found some little issues this morning. I hope, I don't annoy you... but I think, we all want this to work properly :-)

Here are the little issues I'm talking about:

bildschirmfoto 2018-11-05 um 11 52 07 bildschirmfoto 2018-11-05 um 11 53 23

When I use "background-image" on it's own, it works perfectly:

bildschirmfoto 2018-11-05 um 11 52 22

braver commented 6 years ago

Yeah, most work last week was to iron out the other issues that were found. Property-nesting still doesn't work at all really, which is what you're seeing here.

braver commented 5 years ago

Should be fixed in 2.2!