Re: [PATCH] userdiff: add built-in pattern for CSS

On Fri, Jun 03, 2016 at 07:52:45AM +0200, Johannes Sixt wrote:

Am 03.06.2016 um 00:48 schrieb William Duclot:

quoted

Logic behind the "pattern" regex is:

The name of the macro parameter is "pattern", but the actual meaning
is "function name" regex.

quoted

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..81f60ad 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt

@@ -525,6 +525,8 @@ patterns are available:

 - `csharp` suitable for source code in the C# language.

+- `css` suitable for source code in the CSS language.

CSS is not so much source code. How about "suitable for cascaded
style sheets"?

Technically correct yes
 

quoted

diff --git a/t/t4018/css-common b/t/t4018/css-common
new file mode 100644
index 0000000..84ed754
--- /dev/null
+++ b/t/t4018/css-common

@@ -0,0 +1,4 @@
+RIGHT label.control-label {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}

quoted

diff --git a/t/t4018/css-rule b/t/t4018/css-rule
new file mode 100644
index 0000000..84ed754
--- /dev/null
+++ b/t/t4018/css-rule

@@ -0,0 +1,4 @@
+RIGHT label.control-label {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}

These two are the same. Please pick only one. I propose the name
"common" because it is how CSS rules are written most commonly.

Right, I was distracted

quoted

+IPATTERN("css",
+	 "!^.*;\n"

Is there a difference between this and "!;\n"? Is it necessary to
anchor the pattern at the beginning of the line?

In the commit message you talk about colon (':'), but you actually
use a semicolon (';'). Thinking a bit more about it, rejecting lines
with either one would be even better. Consider this case (without
the indentation):

   h1 {
   color:
   red;
   }

(New test case, hint, hint!) Therefore, it could be: "![:;]\n".

You're right! (plus the potential trailing spaces)

quoted

+	 "^[_a-z0-9].*$",
+	 /* -- */
+	 /*
+	  * This regex comes from W3C CSS specs. Should theoretically also
+	  * allow ISO 10646 characters U+00A0 and higher,
+	  * but they are not handled in this regex.
+	  */
+	 "-?[_a-zA-F][-_a-zA-F0-9]*" /* identifiers */

Drop A-F.

quoted

+	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */

Here, too: it is an IPATTERN.

Here I have to disagree (with you and Junio): the IPATTERN is
case-insensitive only on the "pattern" regex, not the "word_regex"
regex. It can be seen in the macro definition, and a quick test confirm
that (and we can see that the fortran word_regex, for example, bother
with uppercase and lowercase even thought it use IPATTERN).
On the identifier line, I have "A-F" instead of "A-Z" though