From: Johannes Sixt <j6t@kdbg.org>
Date: Fri, 03 Jun 2016 05:52:45 GMT
Subject: Re: [PATCH] userdiff: add built-in pattern for CSS
Message-ID: <57511B2D.7040501@kdbg.org>
In-Reply-To: <20160602224809.5167-1-william.duclot@ensimag.grenoble-inp.fr>

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"?

> 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.

> +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".

> +	 "^[_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