Re: [PATCH] userdiff: add built-in pattern for CSS
- From
- William Duclot <william.duclot@ensimag.grenoble-inp.fr>
- Date
- Jun 3, 2016, 09:45 UTC
- Message-ID
- <20160603094544.GA3865@Messiaen>
- In-Reply-To
- <57511B2D.7040501@kdbg.org>
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