threads / patch / 42398

patchuserdiff: add built-in pattern for CSS

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

## tl;dr

21 messages between May 20, 2016 and Jun 6, 2016. Diffs are folded; open one to read it.

replies: 20people: 4as markdown or json

William Duclot· May 20, 2016, 13:28 UTC · lore
CSS is widely used, motivating it being included as a built-in pattern.

It must be noted that the word_regex for CSS (i.e. the regex defining what is a word in the language) does not consider '.' and '#' characters (in CSS selectors) to be part of the word. This behavior is documented by the test t/t4018/css-rule. The logic behind this behavior is the following: identifiers in CSS selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#' character are not part of the identifier, but an indicator of the nature of the identifier in HTML/XML (class or id). Diffing ".class1" and ".class2" must show that the class name is changed, but we still are selecting a class.

Signed-off-by: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
---
 Documentation/gitattributes.txt |  2 ++
 t/t4018-diff-funcname.sh        |  1 +
 t/t4018/css-rule                |  4 ++++
 t/t4034-diff-words.sh           |  1 +
 t/t4034/css/expect              | 16 ++++++++++++++++
 t/t4034/css/post                |  9 +++++++++
 t/t4034/css/pre                 |  9 +++++++++
 userdiff.c                      |  8 ++++++++
 8 files changed, 50 insertions(+)
 create mode 100644 t/t4018/css-rule
 create mode 100644 t/t4034/css/expect
 create mode 100644 t/t4034/css/post
 create mode 100644 t/t4034/css/pre
Show changes to 8 files +50 −0

Documentation/gitattributes.txt, t/t4018-diff-funcname.sh, t/t4018/css-rule, t/t4034-diff-words.sh, t/t4034/css/expect, t/t4034/css/post, t/t4034/css/pre, userdiff.c

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.
+
 - `fortran` suitable for source code in the Fortran language.
 
 - `fountain` suitable for Fountain documents.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 67373dc..1795ffc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -30,6 +30,7 @@ diffpatterns="
 	bibtex
 	cpp
 	csharp
+	css
 	fortran
 	fountain
 	html
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;
+}
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index f2f55fc..912df91 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -302,6 +302,7 @@ test_language_driver ada
 test_language_driver bibtex
 test_language_driver cpp
 test_language_driver csharp
+test_language_driver css
 test_language_driver fortran
 test_language_driver html
 test_language_driver java
diff --git a/t/t4034/css/expect b/t/t4034/css/expect
new file mode 100644
index 0000000..b025d88
--- /dev/null
+++ b/t/t4034/css/expect
@@ -0,0 +1,16 @@
+<BOLD>diff --git a/pre b/post<RESET>
+<BOLD>index 735f301..bdf6a90 100644<RESET>
+<BOLD>--- a/pre<RESET>
+<BOLD>+++ b/post<RESET>
+<CYAN>@@ -1,9 +1,9 @@<RESET>
+.<RED>class-form<RESET><GREEN>other-form<RESET> label.control-label {
+    margin-top: <RED>10<RESET><GREEN>15<RESET>px!important;
+    border : 10px <RED>dashed<RESET><GREEN>dotted<RESET> #C6C6C6;
+}<RESET>
+<RED>#CCCCCC<RESET>
+<RED>padding-bottom<RESET><GREEN>#CCCCCB<RESET>
+<GREEN>margin-left<RESET>
+150<RED>px<RESET><GREEN>em<RESET>
+10px
+<RED>!important<RESET>
+<RED>div<RESET><GREEN>li<RESET>.class#id
diff --git a/t/t4034/css/post b/t/t4034/css/post
new file mode 100644
index 0000000..bdf6a90
--- /dev/null
+++ b/t/t4034/css/post
@@ -0,0 +1,9 @@
+.other-form label.control-label {
+    margin-top: 15px!important;
+    border : 10px dotted #C6C6C6;
+}
+#CCCCCB
+margin-left
+150em
+10px
+li.class#id
diff --git a/t/t4034/css/pre b/t/t4034/css/pre
new file mode 100644
index 0000000..735f301
--- /dev/null
+++ b/t/t4034/css/pre
@@ -0,0 +1,9 @@
+.class-form label.control-label {
+    margin-top: 10px!important;
+    border : 10px dashed #C6C6C6;
+}
+#CCCCCC
+padding-bottom
+150px
+10px!important
+div.class#id
diff --git a/userdiff.c b/userdiff.c
index 6bf2505..0f9cfbe 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -148,6 +148,14 @@ PATTERNS("csharp",
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
+PATTERNS("css",
+	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
+	 /* -- */
+	 /* This regex comes from W3C CSS specs. Should theoretically also allow ISO 10646 characters U+00A0 and higher,
+	  * this not handled in this regex. */
+	 "-?[_a-zA-F][-_a-zA-F0-9]*" /* identifiers */
+	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
+),
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
-- 
2.8.2.403.gdf0b511.dirty
Junio C Hamano· May 20, 2016, 22:37 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
Show 12 quoted lines
> CSS is widely used, motivating it being included as a built-in pattern.
>
> It must be noted that the word_regex for CSS (i.e. the regex defining
> what is a word in the language) does not consider '.' and '#' characters
> (in CSS selectors) to be part of the word. This behavior is documented
> by the test t/t4018/css-rule.
> The logic behind this behavior is the following: identifiers in CSS
> selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#'
> character are not part of the identifier, but an indicator of the nature
> of the identifier in HTML/XML (class or id). Diffing ".class1" and
> ".class2" must show that the class name is changed, but we still are
> selecting a class.

In other words, if "div#foo" changed to "span#bar", word-diff would say that "div changed to span, # didn't change and foo changed to bar".

Which makes sense to me.

The above is not a request to change anything; just me thinking aloud to see if I agree with the reasoning.

Show 16 quoted lines
> diff --git a/userdiff.c b/userdiff.c
> index 6bf2505..0f9cfbe 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -148,6 +148,14 @@ PATTERNS("csharp",
>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>  	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
>  	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
> +PATTERNS("css",
> +	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
> +	 /* -- */
> +	 /* This regex comes from W3C CSS specs. Should theoretically also allow ISO 10646 characters U+00A0 and higher,
> +	  * this not handled in this regex. */
> +	 "-?[_a-zA-F][-_a-zA-F0-9]*" /* identifiers */
> +	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
> +),
Style:
	/*
         * This regex comes from ...
         * ...
         * but they are not handled with this regex.
         */
I wonder if IPATTERN() may make it easier to express the above.

Also, a-zA-F (twice seen in "identifiers" section) looks somewhat suspicious. a-fA-F or a-zA-Z I would understand, and I suspect this is a misspelled form of the latter.

William Duclot· May 24, 2016, 14:25 UTC · re: William Duclot · lore
CSS is widely used, motivating it being included as a built-in pattern.

It must be noted that the word_regex for CSS (i.e. the regex defining what is a word in the language) does not consider '.' and '#' characters (in CSS selectors) to be part of the word. This behavior is documented by the test t/t4018/css-rule. The logic behind this behavior is the following: identifiers in CSS selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#' character are not part of the identifier, but an indicator of the nature of the identifier in HTML/XML (class or id). Diffing ".class1" and ".class2" must show that the class name is changed, but we still are selecting a class.

Signed-off-by: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
---
changes since v1:
Fix a typo in the word_regex ("A-F" => "A-Z").
Clearer comment about ISO 10646 characters.
 Documentation/gitattributes.txt |  2 ++
 t/t4018-diff-funcname.sh        |  1 +
 t/t4018/css-rule                |  4 ++++
 t/t4034-diff-words.sh           |  1 +
 t/t4034/css/expect              | 16 ++++++++++++++++
 t/t4034/css/post                | 10 ++++++++++
 t/t4034/css/pre                 | 10 ++++++++++
 userdiff.c                      |  8 ++++++++
 8 files changed, 52 insertions(+)
 create mode 100644 t/t4018/css-rule
 create mode 100644 t/t4034/css/expect
 create mode 100644 t/t4034/css/post
 create mode 100644 t/t4034/css/pre
Show changes to 8 files +52 −0

Documentation/gitattributes.txt, t/t4018-diff-funcname.sh, t/t4018/css-rule, t/t4034-diff-words.sh, t/t4034/css/expect, t/t4034/css/post, t/t4034/css/pre, userdiff.c

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.
+
 - `fortran` suitable for source code in the Fortran language.
 
 - `fountain` suitable for Fountain documents.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 67373dc..1795ffc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -30,6 +30,7 @@ diffpatterns="
 	bibtex
 	cpp
 	csharp
+	css
 	fortran
 	fountain
 	html
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;
+}
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index f2f55fc..912df91 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -302,6 +302,7 @@ test_language_driver ada
 test_language_driver bibtex
 test_language_driver cpp
 test_language_driver csharp
+test_language_driver css
 test_language_driver fortran
 test_language_driver html
 test_language_driver java
diff --git a/t/t4034/css/expect b/t/t4034/css/expect
new file mode 100644
index 0000000..ed10393
--- /dev/null
+++ b/t/t4034/css/expect
@@ -0,0 +1,16 @@
+<BOLD>diff --git a/pre b/post<RESET>
+<BOLD>index b8ae0bb..fe500b7 100644<RESET>
+<BOLD>--- a/pre<RESET>
+<BOLD>+++ b/post<RESET>
+<CYAN>@@ -1,10 +1,10 @@<RESET>
+.<RED>class-form<RESET><GREEN>other-form<RESET> label.control-label {
+    margin-top: <RED>10<RESET><GREEN>15<RESET>px!important;
+    border : 10px <RED>dashed<RESET><GREEN>dotted<RESET> #C6C6C6;
+}<RESET>
+<RED>#CCCCCC<RESET><GREEN>#CCCCCB<RESET>
+10em<RESET>
+<RED>padding-bottom<RESET><GREEN>margin-left<RESET>
+150<RED>px<RESET><GREEN>em<RESET>
+10px
+<RED>!important<RESET>
+<RED>div<RESET><GREEN>li<RESET>.class#id
diff --git a/t/t4034/css/post b/t/t4034/css/post
new file mode 100644
index 0000000..fe500b7
--- /dev/null
+++ b/t/t4034/css/post
@@ -0,0 +1,10 @@
+.other-form label.control-label {
+    margin-top: 15px!important;
+    border : 10px dotted #C6C6C6;
+}
+#CCCCCB
+10em
+margin-left
+150em
+10px
+li.class#id
diff --git a/t/t4034/css/pre b/t/t4034/css/pre
new file mode 100644
index 0000000..b8ae0bb
--- /dev/null
+++ b/t/t4034/css/pre
@@ -0,0 +1,10 @@
+.class-form label.control-label {
+    margin-top: 10px!important;
+    border : 10px dashed #C6C6C6;
+}
+#CCCCCC
+10em
+padding-bottom
+150px
+10px!important
+div.class#id
diff --git a/userdiff.c b/userdiff.c
index 6bf2505..9273969 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -148,6 +148,14 @@ PATTERNS("csharp",
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
+PATTERNS("css",
+	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
+	 /* -- */
+	 /* This regex comes from W3C CSS specs. Should theoretically also allow ISO 10646 characters U+00A0 and higher,
+	  * but they are not handled with this regex. */
+	 "-?[_a-zA-Z][-_a-zA-Z0-9]*" /* identifiers */
+	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
+),
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
-- 
2.9.0.rc0.1.g521d471.dirty
Junio C Hamano· May 24, 2016, 19:06 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
Show 19 quoted lines
> CSS is widely used, motivating it being included as a built-in pattern.
>
> It must be noted that the word_regex for CSS (i.e. the regex defining
> what is a word in the language) does not consider '.' and '#' characters
> (in CSS selectors) to be part of the word. This behavior is documented
> by the test t/t4018/css-rule.
> The logic behind this behavior is the following: identifiers in CSS
> selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#'
> character are not part of the identifier, but an indicator of the nature
> of the identifier in HTML/XML (class or id). Diffing ".class1" and
> ".class2" must show that the class name is changed, but we still are
> selecting a class.
>
> Signed-off-by: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
> Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> ---
> changes since v1:
> Fix a typo in the word_regex ("A-F" => "A-Z").
> Clearer comment about ISO 10646 characters.

It is not a big deal for a small single-patch topic like this, but it often is hard to reviewers if you do not respond to comments you received and instead just send a new version of the patch with "changes since..." comment. Please make it a habit to do both.

I can see in the above "changes since v1" comment, you took the A-F/A-Z thing, but I cannot tell if you thought about PATTERNS vs IPATTERN and rejected IPATTERN with a good reason or if you simply missed it when reading the review comments you received, for example.

Three remaining issues are:
 - Have you considered using IPATTERN()?  PATTERNS() that defaults
   case sensitive match is suitable for real languages with fixed
   case keywords, but the pattern you are defining for CSS does not
   do anything special for any set of fixed-case built-in keywords,
   and appears to be better served by IPATTERN().
 - In our codebase, we format multi-line comments in a particular
   way, namely
	/*
         * A multi-line comment begins with slash asterisk
         * on its own line, and its closing asterisk slash
         * also is on its own line.
         */
 - Try not to write overlong lines.  If your expression needs to
   become long and there is no good place to fold lines, that is one
   thing, but an overlong comment is unexcuable, as you can fold
   lines anywhere between words.
Thanks.
Show 19 quoted lines
> diff --git a/userdiff.c b/userdiff.c
> index 6bf2505..9273969 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -148,6 +148,14 @@ PATTERNS("csharp",
>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>  	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
>  	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
> +PATTERNS("css",
> +	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
> +	 /* -- */
> +	 /* This regex comes from W3C CSS specs. Should theoretically also allow ISO 10646 characters U+00A0 and higher,
> +	  * but they are not handled with this regex. */
> +	 "-?[_a-zA-Z][-_a-zA-Z0-9]*" /* identifiers */
> +	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
> +),
>  { "default", NULL, -1, { NULL, 0 } },
>  };
>  #undef PATTERNS
William Duclot· May 24, 2016, 22:12 UTC · re: Junio C Hamano · lore

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

> It is not a big deal for a small single-patch topic like this, but
> it often is hard to reviewers if you do not respond to comments you
> received and instead just send a new version of the patch with
> "changes since..." comment.  Please make it a habit to do both.
Apologies, I am not quite used to work with a mailing list yet, I
will make sure to respect this in the future!
 
Show 5 quoted lines
>  - Have you considered using IPATTERN()?  PATTERNS() that defaults
>    case sensitive match is suitable for real languages with fixed
>    case keywords, but the pattern you are defining for CSS does not
>    do anything special for any set of fixed-case built-in keywords,
>    and appears to be better served by IPATTERN().

I did have considered IPATTERN(), but assumed that case-sensitive was default and case-insensitive was the exception. As the CSS pattern does not deal with letters at all it seemed sensible to me to follow the example of the HTML pattern, which use PATTERNS().

Show 8 quoted lines
>  - In our codebase, we format multi-line comments in a particular
>    way, namely
> 
> 	/*
>          * A multi-line comment begins with slash asterisk
>          * on its own line, and its closing asterisk slash
>          * also is on its own line.
>          */
I take good note of that. I took example on the fortran pattern
comment, should I fix it too while I'm at it?
 
>  - Try not to write overlong lines.  If your expression needs to
>    become long and there is no good place to fold lines, that is one
>    thing, but an overlong comment is unexcuable, as you can fold
>    lines anywhere between words.
Again, I take good note of that.
Thank you for your time!
Junio C Hamano· May 24, 2016, 22:18 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> I did have considered IPATTERN(), but assumed that case-sensitive was
> default and case-insensitive was the exception.
Don't assume, but ask ;-)
> As the CSS pattern
> does not deal with letters at all it seemed sensible to me to follow
> the example of the HTML pattern, which use PATTERNS().

Did you notice that HTML pattern has to do an [Hh] that would be unnecessary if it chose to use IPATTTERN()?

You do not have to ask a person, but instead ask the history. IPATTERN() was added at 909a5494 (userdiff.c: add builtin fortran regex patterns, 2010-09-10) when adding fortran support. Anything that existed before, including HTML, did [A-Za-z] when they could have done [a-z] if IPATTERN() existed back then.

Show 11 quoted lines
>>  - In our codebase, we format multi-line comments in a particular
>>    way, namely
>> 
>> 	   /*
>>          * A multi-line comment begins with slash asterisk
>>          * on its own line, and its closing asterisk slash
>>          * also is on its own line.
>>          */
>
> I take good note of that. I took example on the fortran pattern
> comment, should I fix it too while I'm at it?
Not "while you are at it".

Making existing things better is welcome but such a change shouldn't be mixed with addition of new things. You can do it as a separate patch, probably as a preliminary clean-up before your change, if you want to.

Show 8 quoted lines
>>  - Try not to write overlong lines.  If your expression needs to
>>    become long and there is no good place to fold lines, that is one
>>    thing, but an overlong comment is unexcuable, as you can fold
>>    lines anywhere between words.
>
> Again, I take good note of that.
>
> Thank you for your time!
Thank you for working on this.
Johannes Sixt· May 26, 2016, 21:11 UTC · re: William Duclot · lore

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

Am 24.05.2016 um 16:25 schrieb William Duclot:
> +PATTERNS("css",
> +	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",

This hunk header pattern is a bit too restrictive for my taste. Find below a few more test cases that you should squash in. One case fails because only the first CSS selector is picked up, for which I do not see a reason.

Another case fails because the opening brace is not on the line with the CSS selectors.

I think what the hunk header pattern should do is:
1. reject lines containing a colon (because that are properties)
2. if a line begins with a name in column 1, pick the whole line
See the cpp patterns: a pattern beginning with ! is a "reject" pattern.
Show changes to 5 files +20 −0

t/t4018/css-brace-in-col-1, t/t4018/css-rule, t/t4018/css-long-selector-list, t/t4018/css-prop-sans-indent, t/t4018/css-short-selector-list

diff --git a/t/t4018/css-brace-in-col-1 b/t/t4018/css-brace-in-col-1
new file mode 100644
index 0000000..7831577
--- /dev/null
+++ b/t/t4018/css-brace-in-col-1
@@ -0,0 +1,5 @@
+RIGHT label.control-label
+{
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-rule b/t/t4018/css-common
similarity index 100%
rename from t/t4018/css-rule
rename to t/t4018/css-common
diff --git a/t/t4018/css-long-selector-list b/t/t4018/css-long-selector-list
new file mode 100644
index 0000000..7ccd25d
--- /dev/null
+++ b/t/t4018/css-long-selector-list
@@ -0,0 +1,6 @@
+p.header,
+label.control-label,
+div ul#RIGHT {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-prop-sans-indent b/t/t4018/css-prop-sans-indent
new file mode 100644
index 0000000..a9e3c86
--- /dev/null
+++ b/t/t4018/css-prop-sans-indent
@@ -0,0 +1,5 @@
+RIGHT, label.control-label {
+margin-top: 10px!important;
+padding: 0;
+border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-short-selector-list b/t/t4018/css-short-selector-list
new file mode 100644
index 0000000..6a0bdee
--- /dev/null
+++ b/t/t4018/css-short-selector-list
@@ -0,0 +1,4 @@
+label.control, div ul#RIGHT {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
-- 
2.9.0.rc0.40.gb3c1388
William Duclot· May 27, 2016, 07:48 UTC · re: Johannes Sixt · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
>
>> As the CSS pattern
>> does not deal with letters at all it seemed sensible to me to follow
>> the example of the HTML pattern, which use PATTERNS().
> 
> Did you notice that HTML pattern has to do an [Hh] that would be
> unnecessary if it chose to use IPATTTERN()?
> 
> You do not have to ask a person, but instead ask the history.
> IPATTERN() was added at 909a5494 (userdiff.c: add builtin fortran
> regex patterns, 2010-09-10) when adding fortran support.  Anything
> that existed before, including HTML, did [A-Za-z] when they could
> have done [a-z] if IPATTERN() existed back then.
I hadn't noticed that the HTML pattern was older, indeed
Show 18 quoted lines
>>>  - In our codebase, we format multi-line comments in a particular
>>>    way, namely
>>> 
>>> 	   /*
>>>          * A multi-line comment begins with slash asterisk
>>>          * on its own line, and its closing asterisk slash
>>>          * also is on its own line.
>>>          */
>>
>> I take good note of that. I took example on the fortran pattern
>> comment, should I fix it too while I'm at it?
> 
> Not "while you are at it".
> 
> Making existing things better is welcome but such a change shouldn't
> be mixed with addition of new things.  You can do it as a separate
> patch, probably as a preliminary clean-up before your change, if you
> want to.
OK !
Johannes Sixt <j6t@kdbg.org> writes:
Show 11 quoted lines
> Am 24.05.2016 um 16:25 schrieb William Duclot:
>> +PATTERNS("css",
>> +	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
> 
> This hunk header pattern is a bit too restrictive for my taste. Find
> below a few more test cases that you should squash in. One case fails
> because only the first CSS selector is picked up, for which I do not
> see a reason.
> 
> Another case fails because the opening brace is not on the line with
> the CSS selectors.
Yes, it seems you're right !
Show 6 quoted lines
> I think what the hunk header pattern should do is:
> 
> 1. reject lines containing a colon (because that are properties)
> 2. if a line begins with a name in column 1, pick the whole line
> 
> See the cpp patterns: a pattern beginning with ! is a "reject" pattern.
That may be a good idea, I will look into that
Show 51 quoted lines
> diff --git a/t/t4018/css-brace-in-col-1 b/t/t4018/css-brace-in-col-1
> new file mode 100644
> index 0000000..7831577
> --- /dev/null
> +++ b/t/t4018/css-brace-in-col-1
> @@ -0,0 +1,5 @@
> +RIGHT label.control-label
> +{
> +    margin-top: 10px!important;
> +    border : 10px ChangeMe #C6C6C6;
> +}
> diff --git a/t/t4018/css-rule b/t/t4018/css-common
> similarity index 100%
> rename from t/t4018/css-rule
> rename to t/t4018/css-common
> diff --git a/t/t4018/css-long-selector-list b/t/t4018/css-long-selector-list
> new file mode 100644
> index 0000000..7ccd25d
> --- /dev/null
> +++ b/t/t4018/css-long-selector-list
> @@ -0,0 +1,6 @@
> +p.header,
> +label.control-label,
> +div ul#RIGHT {
> +    margin-top: 10px!important;
> +    border : 10px ChangeMe #C6C6C6;
> +}
> diff --git a/t/t4018/css-prop-sans-indent b/t/t4018/css-prop-sans-indent
> new file mode 100644
> index 0000000..a9e3c86
> --- /dev/null
> +++ b/t/t4018/css-prop-sans-indent
> @@ -0,0 +1,5 @@
> +RIGHT, label.control-label {
> +margin-top: 10px!important;
> +padding: 0;
> +border : 10px ChangeMe #C6C6C6;
> +}
> diff --git a/t/t4018/css-short-selector-list
> b/t/t4018/css-short-selector-list
> new file mode 100644
> index 0000000..6a0bdee
> --- /dev/null
> +++ b/t/t4018/css-short-selector-list
> @@ -0,0 +1,4 @@
> +label.control, div ul#RIGHT {
> +    margin-top: 10px!important;
> +    border : 10px ChangeMe #C6C6C6;
> +}
> --
> 2.9.0.rc0.40.gb3c1388
Thanks for the test cases, I'll look into that as soon as I have time
William Duclot· Jun 2, 2016, 22:48 UTC · re: William Duclot · lore
CSS is widely used, motivating it being included as a built-in pattern.

It must be noted that the word_regex for CSS (i.e. the regex defining what is a word in the language) does not consider '.' and '#' characters (in CSS selectors) to be part of the word. This behavior is documented by the test t/t4018/css-rule. The logic behind this behavior is the following: identifiers in CSS selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#' character are not part of the identifier, but an indicator of the nature of the identifier in HTML/XML (class or id). Diffing ".class1" and ".class2" must show that the class name is changed, but we still are selecting a class.

Logic behind the "pattern" regex is:
    1. reject lines containing a colon (properties)
    2. if a line begins with a name in column 1, pick the whole line

Credits to Johannes Sixt (j6t@kdbg.org) for the pattern regex and most of the tests.

Signed-off-by: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
---
Changes since V2:
    - pattern regex has changed
    - more tests
 Documentation/gitattributes.txt |  2 ++
 t/t4018-diff-funcname.sh        |  1 +
 t/t4018/css-brace-in-col-1      |  5 +++++
 t/t4018/css-common              |  4 ++++
 t/t4018/css-long-selector-list  |  6 ++++++
 t/t4018/css-prop-sans-indent    |  5 +++++
 t/t4018/css-rule                |  4 ++++
 t/t4018/css-short-selector-list |  4 ++++
 t/t4034-diff-words.sh           |  1 +
 t/t4034/css/expect              | 16 ++++++++++++++++
 t/t4034/css/post                | 10 ++++++++++
 t/t4034/css/pre                 | 10 ++++++++++
 userdiff.c                      | 12 ++++++++++++
 13 files changed, 80 insertions(+)
 create mode 100644 t/t4018/css-brace-in-col-1
 create mode 100644 t/t4018/css-common
 create mode 100644 t/t4018/css-long-selector-list
 create mode 100644 t/t4018/css-prop-sans-indent
 create mode 100644 t/t4018/css-rule
 create mode 100644 t/t4018/css-short-selector-list
 create mode 100644 t/t4034/css/expect
 create mode 100644 t/t4034/css/post
 create mode 100644 t/t4034/css/pre
Show changes to 13 files +80 −0

Documentation/gitattributes.txt, t/t4018-diff-funcname.sh, t/t4018/css-brace-in-col-1, t/t4018/css-common, t/t4018/css-long-selector-list, t/t4018/css-prop-sans-indent, t/t4018/css-rule, t/t4018/css-short-selector-list, t/t4034-diff-words.sh, t/t4034/css/expect, t/t4034/css/post, t/t4034/css/pre, userdiff.c

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.
+
 - `fortran` suitable for source code in the Fortran language.
 
 - `fountain` suitable for Fountain documents.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 67373dc..1795ffc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -30,6 +30,7 @@ diffpatterns="
 	bibtex
 	cpp
 	csharp
+	css
 	fortran
 	fountain
 	html
diff --git a/t/t4018/css-brace-in-col-1 b/t/t4018/css-brace-in-col-1
new file mode 100644
index 0000000..7831577
--- /dev/null
+++ b/t/t4018/css-brace-in-col-1
@@ -0,0 +1,5 @@
+RIGHT label.control-label
+{
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
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;
+}
diff --git a/t/t4018/css-long-selector-list b/t/t4018/css-long-selector-list
new file mode 100644
index 0000000..7ccd25d
--- /dev/null
+++ b/t/t4018/css-long-selector-list
@@ -0,0 +1,6 @@
+p.header,
+label.control-label,
+div ul#RIGHT {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-prop-sans-indent b/t/t4018/css-prop-sans-indent
new file mode 100644
index 0000000..a9e3c86
--- /dev/null
+++ b/t/t4018/css-prop-sans-indent
@@ -0,0 +1,5 @@
+RIGHT, label.control-label {
+margin-top: 10px!important;
+padding: 0;
+border : 10px ChangeMe #C6C6C6;
+}
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;
+}
diff --git a/t/t4018/css-short-selector-list b/t/t4018/css-short-selector-list
new file mode 100644
index 0000000..6a0bdee
--- /dev/null
+++ b/t/t4018/css-short-selector-list
@@ -0,0 +1,4 @@
+label.control, div ul#RIGHT {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index f2f55fc..912df91 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -302,6 +302,7 @@ test_language_driver ada
 test_language_driver bibtex
 test_language_driver cpp
 test_language_driver csharp
+test_language_driver css
 test_language_driver fortran
 test_language_driver html
 test_language_driver java
diff --git a/t/t4034/css/expect b/t/t4034/css/expect
new file mode 100644
index 0000000..ed10393
--- /dev/null
+++ b/t/t4034/css/expect
@@ -0,0 +1,16 @@
+<BOLD>diff --git a/pre b/post<RESET>
+<BOLD>index b8ae0bb..fe500b7 100644<RESET>
+<BOLD>--- a/pre<RESET>
+<BOLD>+++ b/post<RESET>
+<CYAN>@@ -1,10 +1,10 @@<RESET>
+.<RED>class-form<RESET><GREEN>other-form<RESET> label.control-label {
+    margin-top: <RED>10<RESET><GREEN>15<RESET>px!important;
+    border : 10px <RED>dashed<RESET><GREEN>dotted<RESET> #C6C6C6;
+}<RESET>
+<RED>#CCCCCC<RESET><GREEN>#CCCCCB<RESET>
+10em<RESET>
+<RED>padding-bottom<RESET><GREEN>margin-left<RESET>
+150<RED>px<RESET><GREEN>em<RESET>
+10px
+<RED>!important<RESET>
+<RED>div<RESET><GREEN>li<RESET>.class#id
diff --git a/t/t4034/css/post b/t/t4034/css/post
new file mode 100644
index 0000000..fe500b7
--- /dev/null
+++ b/t/t4034/css/post
@@ -0,0 +1,10 @@
+.other-form label.control-label {
+    margin-top: 15px!important;
+    border : 10px dotted #C6C6C6;
+}
+#CCCCCB
+10em
+margin-left
+150em
+10px
+li.class#id
diff --git a/t/t4034/css/pre b/t/t4034/css/pre
new file mode 100644
index 0000000..b8ae0bb
--- /dev/null
+++ b/t/t4034/css/pre
@@ -0,0 +1,10 @@
+.class-form label.control-label {
+    margin-top: 10px!important;
+    border : 10px dashed #C6C6C6;
+}
+#CCCCCC
+10em
+padding-bottom
+150px
+10px!important
+div.class#id
diff --git a/userdiff.c b/userdiff.c
index 6bf2505..00fc3bf 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -148,6 +148,18 @@ PATTERNS("csharp",
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
+IPATTERN("css",
+	 "!^.*;\n"
+	 "^[_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 */
+	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
+),
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
-- 
2.8.2.403.ge2646ba.dirty
Junio C Hamano· Jun 2, 2016, 23:07 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
Show 19 quoted lines
> diff --git a/userdiff.c b/userdiff.c
> index 6bf2505..00fc3bf 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -148,6 +148,18 @@ PATTERNS("csharp",
>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>  	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
>  	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
> +IPATTERN("css",
> +	 "!^.*;\n"
> +	 "^[_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 */
> +	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */

You could now lose A-F from the above two lines. In fact, the first line "identifiers" has "a-zA-F", which probably would work correctly under IPATTERN() to include 'G-Z' as part of the legit letters for identifiers, but is indeed misleading.

> +),
>  { "default", NULL, -1, { NULL, 0 } },
>  };
>  #undef PATTERNS
Johannes Sixt· Jun 3, 2016, 05:52 UTC · re: William Duclot · lore

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

Am 03.06.2016 um 00:48 schrieb William Duclot:
> Logic behind the "pattern" regex is:

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

Show 9 quoted lines
> 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"?

Show 10 quoted lines
> 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;
> +}
Show 10 quoted lines
> 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.

> +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".
Show 8 quoted lines
> +	 "^[_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.
> +	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
Here, too: it is an IPATTERN.
-- Hannes
Matthieu Moy· Jun 3, 2016, 06:41 UTC · re: Johannes Sixt · lore

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

Johannes Sixt <j6t@kdbg.org> writes:
Show 5 quoted lines
>> +IPATTERN("css",
>> +	 "!^.*;\n"
>
> Is there a difference between this and "!;\n"? Is it necessary to
> anchor the pattern at the beginning of the line?

Also, you don't want to force the end of line right after ;. I know trailing whitespaces are evil, but users may not deserve to be punished so hard when they have some ;-).

> 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):

Rejecting colon anywhere in the line would also reject valid patterns like this:

a:hover {
Rejecting it at end of line is probably a good trade-off.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Johannes Sixt· Jun 3, 2016, 06:56 UTC · re: Matthieu Moy · lore

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

Am 03.06.2016 um 08:41 schrieb Matthieu Moy:
Show 6 quoted lines
> Rejecting colon anywhere in the line would also reject valid patterns
> like this:
>
> a:hover {
>
> Rejecting it at end of line is probably a good trade-off.
Good point. Could be worth another test case.
-- Hannes
William Duclot· Jun 3, 2016, 09:45 UTC · re: Johannes Sixt · lore

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

On Fri, Jun 03, 2016 at 07:52:45AM +0200, Johannes Sixt wrote:
Show 18 quoted lines
> Am 03.06.2016 um 00:48 schrieb William Duclot:
>>Logic behind the "pattern" regex is:
> 
> The name of the macro parameter is "pattern", but the actual meaning
> is "function name" regex.
> 
>>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
 
Show 24 quoted lines
>>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;
>>+}
> 
>>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
Show 17 quoted lines
>>+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)
Show 14 quoted lines
>>+	 "^[_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.
> 
>>+	 "|-?[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

Junio C Hamano· Jun 3, 2016, 15:50 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> Here I have to disagree (with you and Junio): the IPATTERN is
> case-insensitive only on the "pattern" regex, not the "word_regex"
> regex.

Ahh, OK. Obviously both of us overlooked that. Thanks for pushing back.

> On the identifier line, I have "A-F" instead of "A-Z" though
Yeah, that does need updating.
William Duclot· Jun 6, 2016, 07:28 UTC · re: Junio C Hamano · lore

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

On Fri, Jun 03, 2016 at 08:50:50AM -0700, Junio C Hamano wrote:
Show 12 quoted lines
> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> 
> > Here I have to disagree (with you and Junio): the IPATTERN is
> > case-insensitive only on the "pattern" regex, not the "word_regex"
> > regex.
> 
> Ahh, OK.  Obviously both of us overlooked that.  Thanks for pushing
> back.
> 
> > On the identifier line, I have "A-F" instead of "A-Z" though
> 
> Yeah, that does need updating.
Note that I sent a V4 :)
Junio C Hamano· Jun 6, 2016, 18:00 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
Show 15 quoted lines
> On Fri, Jun 03, 2016 at 08:50:50AM -0700, Junio C Hamano wrote:
>> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
>> 
>> > Here I have to disagree (with you and Junio): the IPATTERN is
>> > case-insensitive only on the "pattern" regex, not the "word_regex"
>> > regex.
>> 
>> Ahh, OK.  Obviously both of us overlooked that.  Thanks for pushing
>> back.
>> 
>> > On the identifier line, I have "A-F" instead of "A-Z" though
>> 
>> Yeah, that does need updating.
>
> Note that I sent a V4 :)

Yup, thanks. Isn't that what I queued as 0719f3ee (userdiff: add built-in pattern for CSS, 2016-06-03)?

William Duclot· Jun 6, 2016, 20:45 UTC · re: Junio C Hamano · lore

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

On Mon, Jun 06, 2016 at 11:00:38AM -0700, Junio C Hamano wrote:
Show 20 quoted lines
> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> 
> > On Fri, Jun 03, 2016 at 08:50:50AM -0700, Junio C Hamano wrote:
> >> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> >> 
> >> > Here I have to disagree (with you and Junio): the IPATTERN is
> >> > case-insensitive only on the "pattern" regex, not the "word_regex"
> >> > regex.
> >> 
> >> Ahh, OK.  Obviously both of us overlooked that.  Thanks for pushing
> >> back.
> >> 
> >> > On the identifier line, I have "A-F" instead of "A-Z" though
> >> 
> >> Yeah, that does need updating.
> >
> > Note that I sent a V4 :)
> 
> Yup, thanks.  Isn't that what I queued as 0719f3ee (userdiff: add
> built-in pattern for CSS, 2016-06-03)?
It is, my bad
Junio C Hamano· Jun 6, 2016, 20:55 UTC · re: William Duclot · lore

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

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
>> Yup, thanks.  Isn't that what I queued as 0719f3ee (userdiff: add
>> built-in pattern for CSS, 2016-06-03)?
>
> It is, my bad

Not your bad at all. I am leaky and was asking you to double check; it was entirely possible that I missed your even newer patch and what was queued was the second from the last one.

Thanks.
William Duclot· Jun 3, 2016, 12:32 UTC · re: William Duclot · lore
CSS is widely used, motivating it being included as a built-in pattern.

It must be noted that the word_regex for CSS (i.e. the regex defining what is a word in the language) does not consider '.' and '#' characters (in CSS selectors) to be part of the word. This behavior is documented by the test t/t4018/css-rule. The logic behind this behavior is the following: identifiers in CSS selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#' character are not part of the identifier, but an indicator of the nature of the identifier in HTML/XML (class or id). Diffing ".class1" and ".class2" must show that the class name is changed, but we still are selecting a class.

Logic behind the "pattern" regex is:
    1. reject lines ending with a colon/semicolon (properties)
    2. if a line begins with a name in column 1, pick the whole line

Credits to Johannes Sixt (j6t@kdbg.org) for the pattern regex and most of the tests.

Signed-off-by: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
---
Changes since V3:
    - Add a few tests
    - Remove a redondant test
    - Handle trailing spaces
    - Reword in doc
    - Improvement of the pattern regex
 Documentation/gitattributes.txt |  2 ++
 t/t4018-diff-funcname.sh        |  1 +
 t/t4018/css-brace-in-col-1      |  5 +++++
 t/t4018/css-colon-eol           |  4 ++++
 t/t4018/css-colon-selector      |  5 +++++
 t/t4018/css-common              |  4 ++++
 t/t4018/css-long-selector-list  |  6 ++++++
 t/t4018/css-prop-sans-indent    |  5 +++++
 t/t4018/css-short-selector-list |  4 ++++
 t/t4018/css-trailing-space      |  5 +++++
 t/t4034-diff-words.sh           |  1 +
 t/t4034/css/expect              | 16 ++++++++++++++++
 t/t4034/css/post                | 10 ++++++++++
 t/t4034/css/pre                 | 10 ++++++++++
 userdiff.c                      | 12 ++++++++++++
 15 files changed, 90 insertions(+)
 create mode 100644 t/t4018/css-brace-in-col-1
 create mode 100644 t/t4018/css-colon-eol
 create mode 100644 t/t4018/css-colon-selector
 create mode 100644 t/t4018/css-common
 create mode 100644 t/t4018/css-long-selector-list
 create mode 100644 t/t4018/css-prop-sans-indent
 create mode 100644 t/t4018/css-short-selector-list
 create mode 100644 t/t4018/css-trailing-space
 create mode 100644 t/t4034/css/expect
 create mode 100644 t/t4034/css/post
 create mode 100644 t/t4034/css/pre
Show changes to 15 files +90 −0

Documentation/gitattributes.txt, t/t4018-diff-funcname.sh, t/t4018/css-brace-in-col-1, t/t4018/css-colon-eol, t/t4018/css-colon-selector, t/t4018/css-common, t/t4018/css-long-selector-list, t/t4018/css-prop-sans-indent, t/t4018/css-short-selector-list, t/t4018/css-trailing-space, t/t4034-diff-words.sh, t/t4034/css/expect, t/t4034/css/post, t/t4034/css/pre, userdiff.c

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..8882a3e 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 cascading style sheets.
+
 - `fortran` suitable for source code in the Fortran language.
 
 - `fountain` suitable for Fountain documents.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 67373dc..1795ffc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -30,6 +30,7 @@ diffpatterns="
 	bibtex
 	cpp
 	csharp
+	css
 	fortran
 	fountain
 	html
diff --git a/t/t4018/css-brace-in-col-1 b/t/t4018/css-brace-in-col-1
new file mode 100644
index 0000000..7831577
--- /dev/null
+++ b/t/t4018/css-brace-in-col-1
@@ -0,0 +1,5 @@
+RIGHT label.control-label
+{
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-colon-eol b/t/t4018/css-colon-eol
new file mode 100644
index 0000000..5a30553
--- /dev/null
+++ b/t/t4018/css-colon-eol
@@ -0,0 +1,4 @@
+RIGHT h1 {
+color:
+ChangeMe;
+}
diff --git a/t/t4018/css-colon-selector b/t/t4018/css-colon-selector
new file mode 100644
index 0000000..c6d71fb
--- /dev/null
+++ b/t/t4018/css-colon-selector
@@ -0,0 +1,5 @@
+RIGHT a:hover {
+    margin-top:
+    10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
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;
+}
diff --git a/t/t4018/css-long-selector-list b/t/t4018/css-long-selector-list
new file mode 100644
index 0000000..7ccd25d
--- /dev/null
+++ b/t/t4018/css-long-selector-list
@@ -0,0 +1,6 @@
+p.header,
+label.control-label,
+div ul#RIGHT {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-prop-sans-indent b/t/t4018/css-prop-sans-indent
new file mode 100644
index 0000000..a9e3c86
--- /dev/null
+++ b/t/t4018/css-prop-sans-indent
@@ -0,0 +1,5 @@
+RIGHT, label.control-label {
+margin-top: 10px!important;
+padding: 0;
+border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-short-selector-list b/t/t4018/css-short-selector-list
new file mode 100644
index 0000000..6a0bdee
--- /dev/null
+++ b/t/t4018/css-short-selector-list
@@ -0,0 +1,4 @@
+label.control, div ul#RIGHT {
+    margin-top: 10px!important;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4018/css-trailing-space b/t/t4018/css-trailing-space
new file mode 100644
index 0000000..32b5606
--- /dev/null
+++ b/t/t4018/css-trailing-space
@@ -0,0 +1,5 @@
+RIGHT label.control-label {
+    margin:10px;   
+    padding:10px;
+    border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index f2f55fc..912df91 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -302,6 +302,7 @@ test_language_driver ada
 test_language_driver bibtex
 test_language_driver cpp
 test_language_driver csharp
+test_language_driver css
 test_language_driver fortran
 test_language_driver html
 test_language_driver java
diff --git a/t/t4034/css/expect b/t/t4034/css/expect
new file mode 100644
index 0000000..ed10393
--- /dev/null
+++ b/t/t4034/css/expect
@@ -0,0 +1,16 @@
+<BOLD>diff --git a/pre b/post<RESET>
+<BOLD>index b8ae0bb..fe500b7 100644<RESET>
+<BOLD>--- a/pre<RESET>
+<BOLD>+++ b/post<RESET>
+<CYAN>@@ -1,10 +1,10 @@<RESET>
+.<RED>class-form<RESET><GREEN>other-form<RESET> label.control-label {
+    margin-top: <RED>10<RESET><GREEN>15<RESET>px!important;
+    border : 10px <RED>dashed<RESET><GREEN>dotted<RESET> #C6C6C6;
+}<RESET>
+<RED>#CCCCCC<RESET><GREEN>#CCCCCB<RESET>
+10em<RESET>
+<RED>padding-bottom<RESET><GREEN>margin-left<RESET>
+150<RED>px<RESET><GREEN>em<RESET>
+10px
+<RED>!important<RESET>
+<RED>div<RESET><GREEN>li<RESET>.class#id
diff --git a/t/t4034/css/post b/t/t4034/css/post
new file mode 100644
index 0000000..fe500b7
--- /dev/null
+++ b/t/t4034/css/post
@@ -0,0 +1,10 @@
+.other-form label.control-label {
+    margin-top: 15px!important;
+    border : 10px dotted #C6C6C6;
+}
+#CCCCCB
+10em
+margin-left
+150em
+10px
+li.class#id
diff --git a/t/t4034/css/pre b/t/t4034/css/pre
new file mode 100644
index 0000000..b8ae0bb
--- /dev/null
+++ b/t/t4034/css/pre
@@ -0,0 +1,10 @@
+.class-form label.control-label {
+    margin-top: 10px!important;
+    border : 10px dashed #C6C6C6;
+}
+#CCCCCC
+10em
+padding-bottom
+150px
+10px!important
+div.class#id
diff --git a/userdiff.c b/userdiff.c
index 6bf2505..2125d6d 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -148,6 +148,18 @@ PATTERNS("csharp",
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
+IPATTERN("css",
+	 "![:;][[:space:]]*$\n"
+	 "^[_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-Z][-_a-zA-Z0-9]*" /* identifiers */
+	 "|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
+),
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
-- 
2.9.0.rc1.1.geac644e
Johannes Sixt· Jun 3, 2016, 21:31 UTC · re: William Duclot · lore

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

Am 03.06.2016 um 14:32 schrieb William Duclot:
Show 29 quoted lines
> CSS is widely used, motivating it being included as a built-in pattern.
>
> It must be noted that the word_regex for CSS (i.e. the regex defining
> what is a word in the language) does not consider '.' and '#' characters
> (in CSS selectors) to be part of the word. This behavior is documented
> by the test t/t4018/css-rule.
> The logic behind this behavior is the following: identifiers in CSS
> selectors are identifiers in a HTML/XML document. Therefore, the '.'/'#'
> character are not part of the identifier, but an indicator of the nature
> of the identifier in HTML/XML (class or id). Diffing ".class1" and
> ".class2" must show that the class name is changed, but we still are
> selecting a class.
>
> Logic behind the "pattern" regex is:
>      1. reject lines ending with a colon/semicolon (properties)
>      2. if a line begins with a name in column 1, pick the whole line
>
> Credits to Johannes Sixt (j6t@kdbg.org) for the pattern regex and most
> of the tests.
>
> Signed-off-by: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
> Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> ---
> Changes since V3:
>      - Add a few tests
>      - Remove a redondant test
>      - Handle trailing spaces
>      - Reword in doc
>      - Improvement of the pattern regex
Thanks, I think we can take this version.
-- Hannes

← back to recent threads