git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH/RFC] Add userdiff built-in pattern for CSS code
@ 2016-05-19 10:06 William Duclot
  2016-05-19 14:45 ` Matthieu Moy
  0 siblings, 1 reply; 4+ messages in thread
From: William Duclot @ 2016-05-19 10:06 UTC (permalink / raw
  To: git; +Cc: simon.rabourg, matthieu.moy, antoine.queru, francois.beutin

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.

Add the info in documentation that CSS is now built-in.
---
 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

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..715a1fd 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 theorically 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.gdc9c9d0.dirty

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH/RFC] Add userdiff built-in pattern for CSS code
  2016-05-19 10:06 [PATCH/RFC] Add userdiff built-in pattern for CSS code William Duclot
@ 2016-05-19 14:45 ` Matthieu Moy
  2016-05-19 17:41   ` Eric Sunshine
  0 siblings, 1 reply; 4+ messages in thread
From: Matthieu Moy @ 2016-05-19 14:45 UTC (permalink / raw
  To: William Duclot
  Cc: git, simon rabourg, matthieu moy, antoine queru, francois beutin

> Subject: [PATCH/RFC] Add userdiff built-in pattern for CSS code

We normally write subject lines as "<subsystem>: <description",
hence that would be:

userdiff: add built-in pattern for CSS

william.duclot@ensimag.grenoble-inp.fr wrote:
> 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.

This text wasn't properly wrapped. Please wrap around 72 columns (M-q on Emacs,
or google "text wrap $youreditor" for others).

Also, saying _what_ your patch does is not the most important question here.
Focus on the _why_ in the commit message.

(We had a real-life off-list discussion about this and I agree that it's a
sensible behavior, but others may disagree)

> Add the info in documentation that CSS is now built-in.

This doesn't add much to the patch (we can already see that from the patch
itself). I'd remove it.

Missing sign-off (yours, and you can add mine to mark the fact that I allow
you to contribute to Git as part of your student project).

> +PATTERNS("css",
> +	 "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
> +	 /* -- */
> +	 /* This regex comes from W3C CSS specs. Should theorically also allow ISO

s/theorically/theoretically/

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH/RFC] Add userdiff built-in pattern for CSS code
  2016-05-19 14:45 ` Matthieu Moy
@ 2016-05-19 17:41   ` Eric Sunshine
  2016-05-19 18:10     ` William Duclot
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Sunshine @ 2016-05-19 17:41 UTC (permalink / raw
  To: Matthieu Moy
  Cc: William Duclot, Git List, simon rabourg, antoine queru,
	francois beutin

On Thu, May 19, 2016 at 10:45 AM, Matthieu Moy
<matthieu.moy@grenoble-inp.fr> wrote:
>> Subject: [PATCH/RFC] Add userdiff built-in pattern for CSS code
>> [...snip...]
>> Add the info in documentation that CSS is now built-in.
>
> This doesn't add much to the patch (we can already see that from the patch
> itself). I'd remove it.

I think you meant to say this "doesn't add much to the *commit
message*", and that the sentence should be removed from the commit
message, while retaining the actual documentation update in the patch.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH/RFC] Add userdiff built-in pattern for CSS code
  2016-05-19 17:41   ` Eric Sunshine
@ 2016-05-19 18:10     ` William Duclot
  0 siblings, 0 replies; 4+ messages in thread
From: William Duclot @ 2016-05-19 18:10 UTC (permalink / raw
  To: Eric Sunshine
  Cc: Matthieu Moy, Git List, simon rabourg, antoine queru,
	francois beutin

> On Thu, May 19, 2016 at 10:45 AM, Matthieu Moy
> <matthieu.moy@grenoble-inp.fr> wrote:
> >> Add the info in documentation that CSS is now built-in.
> >
> > This doesn't add much to the patch (we can already see that from the patch
> > itself). I'd remove it.
> 
> I think you meant to say this "doesn't add much to the *commit
> message*", and that the sentence should be removed from the commit
> message, while retaining the actual documentation update in the patch.
> 
That's of course how I understood it, but thanks for pointing it out !

Beside that, all of Matthieu Moy comments seem correct to me and will be taken into account.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-05-19 18:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-19 10:06 [PATCH/RFC] Add userdiff built-in pattern for CSS code William Duclot
2016-05-19 14:45 ` Matthieu Moy
2016-05-19 17:41   ` Eric Sunshine
2016-05-19 18:10     ` William Duclot

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