Created attachment 99219 [details] [review]
Improved CSS syntax handling

Hi,

Here is the promised patch agains SVN trunk.

Regards,
Étienne.

Comment on attachment 99219 [details] [review]
Improved CSS syntax handling

Thanks for looking into this, indeed CSS is one of the lang files that could use many improvements, ideally identifing proper contexts and treating things differently depending on the contexts, too bad I do not know css very well...

Some generic comments on the patch:

 - can you also provide a patch to tests/testifiles.sh adding the things you highlight to the css example?
 - please use spaces when indenting lang files, your patch mixes tabs and spaces.


Other comments inline...


>Index: gtksourceview/language-specs/css.lang
>===================================================================
>--- gtksourceview/language-specs/css.lang	(révision 1767)
>+++ gtksourceview/language-specs/css.lang	(copie de travail)
>@@ -54,6 +54,7 @@
>     <style id="known-property-values" _name="Known Property Value" map-to="def:type"/>
>     <style id="at-rules" _name="at-rules" map-to="def:keyword"/>
>     <style id="keyword" _name="Keyword" map-to="def:keyword"/>
>+    <style id="html-tag" _name="HTML tag" map-to="def:identifier"/>
>   </styles>
> 

I do not like the name HTML tag very much... in theory it's not html specific. In fact I am not sure it's worth highlighting this at all... which is the added value? you are just matching any word here so it doesn't help the user to know if he's making a syntax mistake. I think it is similar to a variable name in C, so I would not highlight it. The goal is not to make the files as colorful as possible ;-)

>   <definitions>
>@@ -94,6 +95,25 @@
>       <keyword>lang</keyword>
>     </context>
> 
>+    <context id="selector-id" style-ref="def:operator">
>+      <match>#([a-zA-Z0-9]+)</match>
>+      <include>
>+	<context sub-pattern="1" style-ref="def:identifier"/>
>+      </include>
>+    </context>
>+
>+    <context id="selector-class" style-ref="def:operator">
>+      <match>\.([a-zA-Z0-9]+)</match>
>+      <include>
>+	<context sub-pattern="1" style-ref="def:identifier"/>
>+      </include>
>+    </context>
>+

a bit like what I said above, though it's true that #foo and .bar are syntax constructs... what other people think?

>+    <context id="selector-tag" style-ref="html-tag">
>+      <match>([a-zA-Z]+[1-6]?)</match>
>+    </context>
>+
>+
>     <context id="at-rules" style-ref="at-rules">
>       <prefix>^[ \t]*@</prefix>
>       <keyword>charset</keyword>
>@@ -107,12 +127,12 @@
>       <match>#([a-fA-F0-9]{6}|[a-fA-F0-9]{3})</match>
>     </context>
> 
>-    <context id="function" style-ref="function">
>-      <start>[a-zA-Z][a-z0-9-]+\(</start>
>-      <end>\)</end>
>+    <context id="function" style-ref="string">
>+      <match>([a-zA-Z][a-z0-9-]+)(\()[^\)]*(\))</match>

Not your fault, but are spaces before '(' really not allowed?

>       <include>
>-        <context ref="def:escape"/>
>-        <context ref="def:line-continue"/>
>+	<context sub-pattern="1" style-ref="function" />
>+	<context sub-pattern="2" style-ref="def:operator" />
>+	<context sub-pattern="3" style-ref="def:operator" />
>       </include>
>     </context>
> 

No strong opinion here, what do we do for functions a parens in other lang files?

>@@ -133,6 +153,7 @@
>     </context>
> 
>     <context id="property-names" style-ref="keyword">
>+      <suffix>[ \t]*:</suffix>

should the colums really be highlighted or should we just lookahead?
Btw, by opening a random css file to check, I noticed we miss the 'filter' property.

>       <keyword>azimuth</keyword>
>       <keyword>background-attachment</keyword>
>       <keyword>background-color</keyword>
>@@ -465,19 +486,19 @@
>       <keyword>yellow</keyword>
>     </context>
> 
>-    <context id="punctuators" style-ref="others-3">
>-      <match>[{}();,]</match>
>+    <context id="punctuators" style-ref="def:operator">
>+      <match>[{}();,:]</match>
>     </context>
> 
>-    <context id="attribute-value-delimiters" style-ref="others-2">
>+    <context id="attribute-value-delimiters" style-ref="def:operator">
>       <match>(\[|\])</match>
>     </context>
> 
>-    <context id="operators" style-ref="function">
>+    <context id="operators" style-ref="def:operator">
>       <match>[@%~|!=]</match>
>     </context>
> 
>-    <context id="selector-grammar" style-ref="others-3">
>+    <context id="selector-grammar" style-ref="def:operator">
>       <match>[*#.&gt;+]</match>
>     </context>
> 
>@@ -490,6 +511,8 @@
>         <context ref="unicode-character-reference"/>
>         <context ref="selector-pseudo-elements"/>
>         <context ref="selector-pseudo-classes"/>
>+	<context ref="selector-id"/>
>+	<context ref="selector-class"/>
>         <context ref="at-rules"/>
>         <context ref="hexadecimal-color"/>
>         <context ref="function"/>
>@@ -503,6 +526,7 @@
>         <context ref="attribute-value-delimiters"/>
>         <context ref="operators"/>
>         <context ref="selector-grammar"/>
>+	<context ref="selector-tag"/>
>       </include>
>     </context>
>

There have been many css.lang improvements on master, so I am going to close this one before it turns 10 years old :-)