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

Tokenize style attributes as CSS #170

Merged
merged 7 commits into from Jan 8, 2018

Conversation

Projects
None yet
4 participants
@50Wliu
Member

50Wliu commented Sep 17, 2017

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Tokenizes the value of the style attribute as CSS.

Alternate Designs

N/A

Benefits

CSS rules in style attributes will be more visually pronounced.

Possible Drawbacks

Multiline or unquoted style values will be tokenized incorrectly. The first is due to a side-effect of the clamping I had to implement to make sure that the CSS stops correctly, the second is simply because I haven't implemented it yet (and yes, it is possible: Chrome recognizes style=color:red;background:blue correctly).

Applicable Issues

Closes #135

@50Wliu

This comment has been minimized.

Member

50Wliu commented Sep 17, 2017

html-style-attribute-css

'1':
'name': 'punctuation.definition.string.begin.html'
'2':
'name': 'source.css.style.html'

This comment has been minimized.

@50Wliu

50Wliu Sep 17, 2017

Member

I am open to what this should be scoped as.

@50Wliu

This comment has been minimized.

Member

50Wliu commented Sep 17, 2017

Note: autocomplete-css does not work with inline style attributes yet. This is partly because the meta.property-list.css scope is missing, though there's probably other hurdles.

50Wliu added some commits Sep 17, 2017

Bah
@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Oct 25, 2017

Try this:

diff --git a/grammars/html.cson b/grammars/html.cson
index 527e567..13010c5 100644
--- a/grammars/html.cson
+++ b/grammars/html.cson
@@ -560,7 +560,7 @@
     'name': 'meta.attribute-with-value.style.html'
     'patterns': [
       {
-        'match': '(")(.+)(")'
+        'match': '(")([^"]*)(")'
         'name': 'string.quoted.double.html'
         'captures':
           '1':
@@ -569,20 +569,37 @@
             'name': 'source.css.style.html'
             'patterns': [
               {
-                'include': '#embedded-code'
-              }
-              {
-                'include': '#entities'
-              }
-              {
-                'include': 'source.css#rule-list-innards'
+                'match': '.+'
+                'name': 'meta.property-list.css'
+                'captures':
+                  '0':
+                    'patterns': [
+                      {
+                        'include': '#embedded-code'
+                      }
+                      {
+                        'include': '#entities'
+                      }
+                      {
+                        'include': 'source.css#rule-list-innards'
+                      }
+                    ]
               }
             ]
           '3':
             'name': 'punctuation.definition.string.end.html'
+            'patterns': [
+              {
+                'match': '"'
+                'name': 'source.css.style.html'
+                'captures':
+                  '0':
+                    'name': 'meta.property-list.css'
+              }
+            ]
       }
       {
-        'match': "(')(.+)(')"
+        'match': "(')([^']*)(')"
         'name': 'string.quoted.single.html'
         'captures':
           '1':
@@ -591,17 +608,34 @@
             'name': 'source.css.style.html'
             'patterns': [
               {
-                'include': '#embedded-code'
-              }
-              {
-                'include': '#entities'
-              }
-              {
-                'include': 'source.css#rule-list-innards'
+                'match': '.+'
+                'name': 'meta.property-list.css'
+                'captures':
+                  '0':
+                    'patterns': [
+                      {
+                        'include': '#embedded-code'
+                      }
+                      {
+                        'include': '#entities'
+                      }
+                      {
+                        'include': 'source.css#rule-list-innards'
+                      }
+                    ]
               }
             ]
           '3':
             'name': 'punctuation.definition.string.end.html'
+            'patterns': [
+              {
+                'match': '\''
+                'name': 'source.css.style.html'
+                'captures':
+                  '0':
+                    'name': 'meta.property-list.css'
+              }
+            ]
       }
       {
         'match': '([^\\s&>"\'<=`]|&(?=>))+'

End quotes will require CSS similar to what we have in Blade, this will fix the end quote looking different for any style that doesn't override this directly:
https://github.com/jawee/language-blade/blob/master/styles/language-blade.atom-text-editor.less

@50Wliu

This comment has been minimized.

Member

50Wliu commented Oct 28, 2017

End quotes will require CSS

Can you give an example? I'd really like to not have to do that.

Also, unquoted strings still need the property list scope.

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Oct 29, 2017

This is required or else situations like this happen: atom/atom#716

Unfortunately currently this is fixed in Themes by overriding these cases manually. This is a relic from Textmate times.
https://github.com/atom/atom-dark-syntax/blob/5200d7349f1be5ff3f82f28ea1d06de732b454c3/styles/syntax.less#L60-L74

The currently proposed solution in use in Blade will do exactly as we want and reset the style for that one exact case without themes having to do it themselves. Provided that themes can override package stylesheets, this will not set any limitations on their end and possibly avoids giving advice like atom/language-php#286.

Unlike themes from Textmate days, CSS allows masking this perfectly and works with any theme. It's a very elegant workaround.

@50Wliu

This comment has been minimized.

Member

50Wliu commented Oct 30, 2017

I'm afraid I still don't understand :/. The example you gave is targeting .string .punctuation.section.embedded .source with special styles. However, the quotes for this PR currently look like this:
html-style-quote-scopes

Seems to me like they're "normal" quotes, and the atom-dark-syntax section doesn't apply?

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Oct 30, 2017

Try atom-dark syntax theme.***

E: I am seeing a white end quote with oneatom dark.

*** Something happened during switching themes, sorry for any confusion caused.

white

@50Wliu

This comment has been minimized.

Member

50Wliu commented Oct 30, 2017

Hmm, still no repro for me:
atom-dark-syntax-quotes

50Wliu added some commits Jan 8, 2018

@50Wliu

This comment has been minimized.

Member

50Wliu commented Jan 8, 2018

It's been a while but I think we resolved the problems with the quotes. If I recall correctly there may still be some problems with autocompletion not working due to the end quote not being scoped as CSS but I'd much rather have syntax highlighting as a first step :).

@50Wliu 50Wliu merged commit 2d1c225 into master Jan 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@50Wliu 50Wliu deleted the wl-style-attribute branch Jan 8, 2018

@Arcanemagus Arcanemagus referenced this pull request Mar 30, 2018

Closed

Syntax different in Windows and Linux #45

1 of 1 task complete
@javajeff18

This comment has been minimized.

javajeff18 commented Mar 30, 2018

The One Light theme that is stock with Atom, is now hard to use. Styles are now the same color as the text (content). This makes the job very difficult.
This is how my Windows one light theme looks:
one-light
This is how my Windows one dark theme looks:
one-dark

This is the way the one light looks in Linux, which uses version 1.24.1.
one-light-1241
I would love my entire style to be one color to make it easier to differentiate between my content and code. Thanks!

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Apr 2, 2018

@javajeff18 add this to your custom stylesheet (File -> Stylesheet):

.syntax--string.syntax--html
.syntax--source.syntax--css.syntax--style.syntax--html {
  &, & .syntax--css {
    all: unset;
  }
}

Edit: changed to work with all quote types

@javajeff18

This comment has been minimized.

javajeff18 commented Apr 3, 2018

Thank you so much for this customization tip! It is awesome! Now I can clearly see separation between content and inline styles. I have no idea why everyone would not want this, and I think the theme likely needs to adapt to the changes in Atom.

image

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented Apr 3, 2018

It depends on the theme you are using. Atom Dark for instance renders the CSS differently, making the CSS properties more visible:

image

If you wish to have this improved for the theme you are using, consider opening an issue at the repository of the theme.

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