SublimeText / Sass

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

Multiple regressions and suggestions after update #24

Closed sandrojohanides closed 5 years ago

sandrojohanides commented 6 years ago

Hey there, thanks for the update. I have found multiple regressions I think (would have to compare with previous version). Also some of these may be suggestions, because I’m not sure if they are broken or not. If there’s any more information I can give you, let me know :)

A before/after would be best I guess, is there a way to install a specific version of a package?

1.

screen shot 2018-11-01 at 09 38 44

2.

screen shot 2018-11-01 at 09 39 07

3.

screen shot 2018-11-01 at 09 40 08

4.

screen shot 2018-11-01 at 09 40 26

5.

screen shot 2018-11-01 at 09 41 03

6.

screen shot 2018-11-01 at 09 41 42

7.

screen shot 2018-11-01 at 09 42 02

8.

screen shot 2018-11-01 at 09 45 03

9.

screen shot 2018-11-01 at 09 46 23

10.

screen shot 2018-11-01 at 09 47 08

sandrojohanides commented 6 years ago

I installed the previous (1.) version to compare, so if you need before/after, let me know.

braver commented 6 years ago

Wow, lot's of feedback, thanks! I'll try to address them one by one:

@ $ . all have the same color, some of it I don’t mind, but $ not having the variable colors seems wrong

That's up to the color scheme what happens with the colors. But it's actually common and recommended to scope the punctuation differently: https://www.sublimetext.com/docs/3/scope_naming.html#keyword E.g. the -- in a --custom-property is also different.

header, aside, main look like strings, even without 'quotes'

In this context, they're actually (unquoted) strings. Only when they are used is the string interpreted as (part of) a selector in your example, but they could just as well be values for a property, or a property name. Strings don't always need quotes in CSS and unquoted strings are similarly colored to quoted strings.

@supports is a css thing, but @each is from Sass, not sure if they should look the same

They are differentiated in the scope (it ends with either sass/scss or css), so you could actually tweak your color scheme to give them a different color. Remember, a syntax is about identifying and labelling the different parts of your code, not about giving them a particular color.

minmax(auto, max-content) has variable highlighting, but both are native keywords for the minmax function

I think I need to look at the grid features in more detail to get this right. Thanks!

color(dark-grey) loses highlight color after the dash but font-size(tiny) has different whole different color (looks like a string) color(darker) have the same color, but color(white) look different, because white is a keyword (but the darker color is incorrect) width: fit-content highlight looks like a string, but is a keyword different color of the same custom function rem()

Good catches, some look really like legit bugs, will look into it!

point is they work kinda correct, but look too random

That does look a bit random, I'll see what makes sense here. Not all color schemes are as "wild" with their colors though, so this will be different for everyone.

@mixin parameter scope has different color than a variable

Yes, that's correct, parameters are different.

ie9 string after == has two colors, one for text, one for number

Hey, that's not right! 👍

:root is a selector, but looks like a variable

Nothing I can do there I'm afraid. I scoped it correctly, so the color scheme just makes it more confusing.

screen shot 2018-11-01 at 14 07 38

seemingly random highlighting of these rules and thier values

Ugh, those pesky vendor prefixes!

custom em() function looks differently in maps

Thanks for catching that!

braver commented 6 years ago

TODO:

cyril-lamotte commented 6 years ago

2018-11-05 09_18_23-d__www-drupal_mmh_src_src_mmh-corporate_docroot_themes_custom_mmh_sources_scss_c

Hi, thanks for your work, I figure that inline comment has been altered too ?

sandrojohanides commented 6 years ago

@cyril-lamotte this seems to be the problem: screen shot 2018-11-05 at 14 24 05 They work when the @import statement is on different line.

Also, my import file looks weird:

11.

screen shot 2018-11-05 at 14 26 13 screen shot 2018-11-05 at 14 26 32

12.

screen shot 2018-11-05 at 14 47 57 screen shot 2018-11-05 at 14 52 57 screen shot 2018-11-06 at 09 30 12

@braver is there a way to help you? Unfortunately, I don’t know much about the syntax scopes :/

braver commented 6 years ago

@sandrojohanides this is already very useful for me. I can take these examples to set up a test case and then make it pass the test. Having raw code in addition to the screenshot would be nice though.

sandrojohanides commented 6 years ago

@braver https://gist.github.com/sandrojohanides/6fc15dacad3b612584a493f1b26030bc ^ Here they are, if you want to see how they looked like 'before', save it as .scss and look at it with the P233 version, but I don’t have to tell you that. Also, I copied all of the cases, even the non-bug ones, you’ll know which to use.

I’m currently still using the old version, but will test yours when you put out a new release.

braver commented 6 years ago

A new release is out that should fix the issues with import statements. Thanks for testing and reporting everyone!

braver commented 6 years ago

👆 this one is also fixed now!

sandrojohanides commented 6 years ago

@braver @imports seem to work great! But no all comments are fixed, there’s still the case with a comment after a comma (updated previous comment and gist).

sandrojohanides commented 6 years ago

9.

screen shot 2018-11-06 at 09 35 49 screen shot 2018-11-06 at 09 36 04

taunoha commented 6 years ago

After updating to the latest version I'm also having some issues with the syntax highlight (SCSS):

Before screenshot 2018-11-06 16 23 57

After screenshot 2018-11-06 16 20 59

treechime commented 5 years ago

I've noticed a couple of things too...

  1. Coloured bracket on nested element with ampersand in selector (it might be difficult to see with my theme, but it's purple when it should be white)
  2. No space after colon causes incorrect url highlighting (I prefer not adding spaces as the rivers this creates in the code makes it difficult to read imo)

image

treechime commented 5 years ago

Another interesting one. It seems two ampersands in the selector with the declaration on one line also breaks the highlighting in odd ways (line 2 & 3)? Spaces don't make a difference in this instance.

image

braver commented 5 years ago

FIY a little update here. The primary trick of both LESS and SCSS is differentiating between properties and selectors, especially when dealing with pseudo selectors (they make them look like properties). I thought I found a shortcut that would still make them stick very closely to the CSS syntax. Sadly, that breaks in various less obvious ways. So, I’m currently rewriting this to be rock solid, no shortcuts. It’ll be next level 💪🏻😎. But it probably also means I need to recreate small things like auto-complete as well, so it’s not ready yet.

sandrojohanides commented 5 years ago

@braver thank you for your work 🔥

cyril-lamotte commented 5 years ago

Hi,

I got this today :

2018-11-15 09_45_17-d__www-drupal_cridf_src_src_portailidf-d8_projet_web_themes_custom_portail_idf_s

braver commented 5 years ago

Alright, status update! ✨ I got it working as I hoped it would for LESS. Will be porting that to SCSS, which should be easy (SCSS is a much easier language to work with), so expect more news and maybe even a release in the coming days.

braver commented 5 years ago

Hey all, if you're using the SCSS syntax you may want to check master. I want to do some more tests and I need to check the autocompletion stuff etc., but it should be a lot more robust. Going through some test files it already looks a lot better. Let me know what you think!

taunoha commented 5 years ago

@braver thank you for the update! I tested the new SCSS syntax and got this:

Before: screenshot 2018-11-06 16 23 57

After: screenshot 2018-11-06 16 20 59

After using current SCSS syntax in Master: screenshot 2018-11-22 21 44 57

Expected: screenshot 2018-11-22 21 52 42

braver commented 5 years ago

@taunoha can you share the code so that I can copy-paste to recreate those snippets?

taunoha commented 5 years ago

@taunoha can you share the code so that I can copy-paste to recreate those snippets?

Here it is:

@each $icon in $icons-16 {
    %icon-#{$icon}--primary {
        background-image: svg('icon__#{$icon}-16', '[fill]: #{theme-color("primary")}');
    }
}

@each $icon in $icons-16 {
    %icon-#{$icon}--muted {
        background-image: svg('icon__#{$icon}-16', '[fill]: #{$text-muted}');
    }
}

https://codeshare.io/amm0rj

braver commented 5 years ago

@taunoha I think master now covers those cases as well. It won't match the old highlighting, but it should be correct.

sandrojohanides commented 5 years ago

Hey @braver,

  1. When I install this package by adding this repository into Sublime Text, does it install the code from master?
  2. When do you want me to send you new or persisting usecases? I don’t know if you’re done with this batch yet.

Thank you!

braver commented 5 years ago

@sandrojohanides I'm not sure what you mean by adding this repository into Sublime Text, do you add the repository to package control? I'm not sure what that does, but I would assume it pulls the code from master. I have one or two fixes lined up that I will push today, if you find anything after that please let me know.

braver commented 5 years ago

2.2 should cover most of what has been mentioned here. If there are problems remaining, please open a new issue.