From: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
To: Johannes Sixt <j6t@kdbg.org>
Cc: git@vger.kernel.org, simon.rabourg@ensimag.grenoble-inp.fr,
antoine.queru@ensimag.grenoble-inp.fr,
francois.beutin@ensimag.grenoble-inp.fr, gitster@pobox.com,
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Subject: Re: [PATCH] userdiff: add built-in pattern for CSS
Date: Fri, 3 Jun 2016 11:45:44 +0200 [thread overview]
Message-ID: <20160603094544.GA3865@Messiaen> (raw)
In-Reply-To: <57511B2D.7040501@kdbg.org>
On Fri, Jun 03, 2016 at 07:52:45AM +0200, Johannes Sixt wrote:
> 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
>>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
>>+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)
>>+ "^[_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
next prev parent reply other threads:[~2016-06-03 9:45 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-20 13:28 [PATCH] userdiff: add built-in pattern for CSS William Duclot
2016-05-20 22:37 ` Junio C Hamano
2016-05-24 14:25 ` William Duclot
2016-05-24 19:06 ` Junio C Hamano
2016-05-24 22:12 ` William Duclot
2016-05-24 22:18 ` Junio C Hamano
2016-05-26 21:11 ` Johannes Sixt
2016-05-27 7:48 ` William Duclot
2016-06-02 22:48 ` William Duclot
2016-06-02 23:07 ` Junio C Hamano
2016-06-03 5:52 ` Johannes Sixt
2016-06-03 6:41 ` Matthieu Moy
2016-06-03 6:56 ` Johannes Sixt
2016-06-03 9:45 ` William Duclot [this message]
2016-06-03 15:50 ` Junio C Hamano
2016-06-06 7:28 ` William Duclot
2016-06-06 18:00 ` Junio C Hamano
2016-06-06 20:45 ` William Duclot
2016-06-06 20:55 ` Junio C Hamano
2016-06-03 12:32 ` William Duclot
2016-06-03 21:31 ` Johannes Sixt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160603094544.GA3865@Messiaen \
--to=william.duclot@ensimag.grenoble-inp.fr \
--cc=antoine.queru@ensimag.grenoble-inp.fr \
--cc=francois.beutin@ensimag.grenoble-inp.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=matthieu.moy@grenoble-inp.fr \
--cc=simon.rabourg@ensimag.grenoble-inp.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).