git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] userdiff: add support for Fountain documents
@ 2015-07-17 11:59 Zoë Blade
  2015-07-17 13:03 ` Johannes Schindelin
  0 siblings, 1 reply; 15+ messages in thread
From: Zoë Blade @ 2015-07-17 11:59 UTC (permalink / raw)
  To: git; +Cc: Zoë Blade

Add support for Fountain, a plain text screenplay format.  In the
structure of a screenplay, scenes are roughly analogous to functions,
in the sense that it makes your job slightly easier if you can see
which ones were changed in a given range of patches.
---
 Documentation/gitattributes.txt | 2 ++
 t/t4018-diff-funcname.sh        | 1 +
 t/t4018/fountain-scene          | 4 ++++
 userdiff.c                      | 2 ++
 4 files changed, 9 insertions(+)
 create mode 100644 t/t4018/fountain-scene

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 81fe586..e3b1de8 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -527,6 +527,8 @@ patterns are available:
 
 - `fortran` suitable for source code in the Fortran language.
 
+- `fountain` suitable for Fountain documents.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 1dbaa38..67373dc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -31,6 +31,7 @@ diffpatterns="
 	cpp
 	csharp
 	fortran
+	fountain
 	html
 	java
 	matlab
diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene
new file mode 100644
index 0000000..6b3257d
--- /dev/null
+++ b/t/t4018/fountain-scene
@@ -0,0 +1,4 @@
+EXT. STREET RIGHT OUTSIDE - DAY
+
+CHARACTER
+You didn't say the magic phrase, "ChangeMe".
diff --git a/userdiff.c b/userdiff.c
index 2ccbee5..5316b48 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -35,6 +35,8 @@ IPATTERN("fortran",
 	  * they would have been matched above as a variable anyway. */
 	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
 	 "|//|\\*\\*|::|[/<>=]="),
+PATTERNS("fountain", "^((INT|EST|EXT)?\\.[A-Z0-9' -]+)$",
+	 "[^ \t-]+"),
 PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
 	 "[^<>= \t]+"),
 PATTERNS("java",
-- 
2.5.0.rc2.28.g6003e7f.dirty

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-17 11:59 Zoë Blade
@ 2015-07-17 13:03 ` Johannes Schindelin
  2015-07-17 14:03   ` Zoë Blade
  0 siblings, 1 reply; 15+ messages in thread
From: Johannes Schindelin @ 2015-07-17 13:03 UTC (permalink / raw)
  To: Zoë Blade; +Cc: git

Hi Zoë,

On 2015-07-17 13:59, Zoë Blade wrote:
> Add support for Fountain, a plain text screenplay format.  In the
> structure of a screenplay, scenes are roughly analogous to functions,
> in the sense that it makes your job slightly easier if you can see
> which ones were changed in a given range of patches.

Interesting!

Maybe you want to add a paragraph explaining a bit more about Fountain, or at least link to http://fountain.io/?

In any case, you will need to sign off on your patch:

    https://github.com/git/git/blob/v2.4.6/Documentation/SubmittingPatches#L234-L286

Ciao,
Johannes

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-17 13:03 ` Johannes Schindelin
@ 2015-07-17 14:03   ` Zoë Blade
  2015-07-17 16:20     ` Johannes Schindelin
  0 siblings, 1 reply; 15+ messages in thread
From: Zoë Blade @ 2015-07-17 14:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

On 17 Jul 2015, at 14:03, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

> Maybe you want to add a paragraph explaining a bit more about Fountain, or at least link to http://fountain.io/?
> 
> In any case, you will need to sign off on your patch:
> 
>    https://github.com/git/git/blob/v2.4.6/Documentation/SubmittingPatches#L234-L286

Thanks, I'll amend it accordingly.  I originally mentioned the Fountain site in my rough draft of the commit message, but then removed it again after reading more of the patch submitting documentation and not spotting the nuance about when is and isn't a good time to include URLs.  No bother, I managed the commit message in another repo... :D  I'll bring it back and tidy it up a bit, then sign it off.

Thanks,
Zoë.

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

* [PATCH] userdiff: add support for Fountain documents
@ 2015-07-17 14:21 Zoë Blade
  2015-07-17 17:12 ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Zoë Blade @ 2015-07-17 14:21 UTC (permalink / raw)
  To: git; +Cc: Zoë Blade

Add support for Fountain, a plain text screenplay format.  Git
facilitates not just programming specifically, but creative writing
in general, so it makes sense to also support other plain text
documents besides source code.

In the structure of a screenplay specifically, scenes are roughly
analogous to functions, in the sense that it makes your job easier
if you can see which ones were changed in a given range of patches.

More information about the Fountain format can be found on its
official website, at http://fountain.io .

Signed-off-by: Zoë Blade <zoe@bytenoise.co.uk>
---
 Documentation/gitattributes.txt | 2 ++
 t/t4018-diff-funcname.sh        | 1 +
 t/t4018/fountain-scene          | 4 ++++
 userdiff.c                      | 2 ++
 4 files changed, 9 insertions(+)
 create mode 100644 t/t4018/fountain-scene

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 81fe586..e3b1de8 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -527,6 +527,8 @@ patterns are available:
 
 - `fortran` suitable for source code in the Fortran language.
 
+- `fountain` suitable for Fountain documents.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 1dbaa38..67373dc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -31,6 +31,7 @@ diffpatterns="
 	cpp
 	csharp
 	fortran
+	fountain
 	html
 	java
 	matlab
diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene
new file mode 100644
index 0000000..6b3257d
--- /dev/null
+++ b/t/t4018/fountain-scene
@@ -0,0 +1,4 @@
+EXT. STREET RIGHT OUTSIDE - DAY
+
+CHARACTER
+You didn't say the magic phrase, "ChangeMe".
diff --git a/userdiff.c b/userdiff.c
index 2ccbee5..5316b48 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -35,6 +35,8 @@ IPATTERN("fortran",
 	  * they would have been matched above as a variable anyway. */
 	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
 	 "|//|\\*\\*|::|[/<>=]="),
+PATTERNS("fountain", "^((INT|EST|EXT)?\\.[A-Z0-9' -]+)$",
+	 "[^ \t-]+"),
 PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
 	 "[^<>= \t]+"),
 PATTERNS("java",
-- 
2.5.0.rc2.28.g6003e7f.dirty

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-17 14:03   ` Zoë Blade
@ 2015-07-17 16:20     ` Johannes Schindelin
  0 siblings, 0 replies; 15+ messages in thread
From: Johannes Schindelin @ 2015-07-17 16:20 UTC (permalink / raw)
  To: Zoë Blade; +Cc: git

Hi Zoë,

On 2015-07-17 16:03, Zoë Blade wrote:
> On 17 Jul 2015, at 14:03, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> 
>> Maybe you want to add a paragraph explaining a bit more about Fountain, or at least link to http://fountain.io/?
>>
>> In any case, you will need to sign off on your patch:
>>
>>    https://github.com/git/git/blob/v2.4.6/Documentation/SubmittingPatches#L234-L286
> 
> Thanks, I'll amend it accordingly.  I originally mentioned the
> Fountain site in my rough draft of the commit message, but then
> removed it again after reading more of the patch submitting
> documentation and not spotting the nuance about when is and isn't a
> good time to include URLs.  No bother, I managed the commit message in
> another repo... :D  I'll bring it back and tidy it up a bit, then sign
> it off.

Thank you!
Dscho

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-17 14:21 Zoë Blade
@ 2015-07-17 17:12 ` Junio C Hamano
  2015-07-17 22:43   ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2015-07-17 17:12 UTC (permalink / raw)
  To: Zoë Blade; +Cc: git

Zoë Blade <zoe@bytenoise.co.uk> writes:

> Add support for Fountain, a plain text screenplay format.  Git
> facilitates not just programming specifically, but creative writing
> in general, so it makes sense to also support other plain text
> documents besides source code.
>
> In the structure of a screenplay specifically, scenes are roughly
> analogous to functions, in the sense that it makes your job easier
> if you can see which ones were changed in a given range of patches.
>
> More information about the Fountain format can be found on its
> official website, at http://fountain.io .
>
> Signed-off-by: Zoë Blade <zoe@bytenoise.co.uk>
> ---

The test looks a bit too brief (i.e. there is only one obvious
candidate to be picked as the funcname header in the input, so it is
very hard to break the expectation of the test even when the code or
pattern is modified incorrectly), but it would do for now.
Everything else looks trivially OK ;-)

Thanks, will queue.


>  Documentation/gitattributes.txt | 2 ++
>  t/t4018-diff-funcname.sh        | 1 +
>  t/t4018/fountain-scene          | 4 ++++
>  userdiff.c                      | 2 ++
>  4 files changed, 9 insertions(+)
>  create mode 100644 t/t4018/fountain-scene
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 81fe586..e3b1de8 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -527,6 +527,8 @@ patterns are available:
>  
>  - `fortran` suitable for source code in the Fortran language.
>  
> +- `fountain` suitable for Fountain documents.
> +
>  - `html` suitable for HTML/XHTML documents.
>  
>  - `java` suitable for source code in the Java language.
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index 1dbaa38..67373dc 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -31,6 +31,7 @@ diffpatterns="
>  	cpp
>  	csharp
>  	fortran
> +	fountain
>  	html
>  	java
>  	matlab
> diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene
> new file mode 100644
> index 0000000..6b3257d
> --- /dev/null
> +++ b/t/t4018/fountain-scene
> @@ -0,0 +1,4 @@
> +EXT. STREET RIGHT OUTSIDE - DAY
> +
> +CHARACTER
> +You didn't say the magic phrase, "ChangeMe".
> diff --git a/userdiff.c b/userdiff.c
> index 2ccbee5..5316b48 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -35,6 +35,8 @@ IPATTERN("fortran",
>  	  * they would have been matched above as a variable anyway. */
>  	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
>  	 "|//|\\*\\*|::|[/<>=]="),
> +PATTERNS("fountain", "^((INT|EST|EXT)?\\.[A-Z0-9' -]+)$",
> +	 "[^ \t-]+"),
>  PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
>  	 "[^<>= \t]+"),
>  PATTERNS("java",

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-17 17:12 ` Junio C Hamano
@ 2015-07-17 22:43   ` Junio C Hamano
  2015-07-19 12:30     ` Zoë Blade
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2015-07-17 22:43 UTC (permalink / raw)
  To: Zoë Blade; +Cc: git

Junio C Hamano <gitster@pobox.com> writes:

> Zoë Blade <zoe@bytenoise.co.uk> writes:
>
>> More information about the Fountain format can be found on its
>> official website, at http://fountain.io .

So I visited there.

>> +PATTERNS("fountain", "^((INT|EST|EXT)?\\.[A-Z0-9' -]+)$",
>> +	 "[^ \t-]+"),

After skimming http://fountain.io/syntax I am getting the impression
that this might be a bit too limiting.

 * Although uppercase is recommended for Scene Headings to increase
   readability, it is not required.

 * A line beginning with any of the following, followed by either a
   dot or a space, is considered a Scene Heading (unless the line is
   preceded by an exclamation point !). Case insensitive.

      INT
      EXT
      EST
      INT./EXT
      INT/EXT
      I/E

 * You can "force" a Scene Heading by starting the line with a
   single period.

 * Scene Headings can optionally be appended with Scene
   Numbers. Scene numbers are any alphanumerics (plus dashes and
   periods), wrapped in #.

So, it appears wrong to insist on capital letters in the patterns.
The pattern in the patch does not even accept punctuations on the
line other than apostrophe.  I won't judge if it is OK to limit to
US-ASCII ;-)

IPATTERNS("fountain",
    "^([.][^.]|(INT|EXT|EST|INT./EXT|INT/EXT|I/E)[. ]",
    "[^ \t-]+"),

or something like this, perhaps?

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-17 22:43   ` Junio C Hamano
@ 2015-07-19 12:30     ` Zoë Blade
  0 siblings, 0 replies; 15+ messages in thread
From: Zoë Blade @ 2015-07-19 12:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On 17 Jul 2015, at 23:43, Junio C Hamano <gitster@pobox.com> wrote:

> * Although uppercase is recommended for Scene Headings to increase
>   readability, it is not required.
> 
> * A line beginning with any of the following, followed by either a
>   dot or a space, is considered a Scene Heading (unless the line is
>   preceded by an exclamation point !). Case insensitive.
> 
>      INT
>      EXT
>      EST
>      INT./EXT
>      INT/EXT
>      I/E
> 
> * You can "force" a Scene Heading by starting the line with a
>   single period.
> 
> * Scene Headings can optionally be appended with Scene
>   Numbers. Scene numbers are any alphanumerics (plus dashes and
>   periods), wrapped in #.
> 
> So, it appears wrong to insist on capital letters in the patterns.
> The pattern in the patch does not even accept punctuations on the
> line other than apostrophe.  I won't judge if it is OK to limit to
> US-ASCII ;-)
> 
> IPATTERNS("fountain",
>    "^([.][^.]|(INT|EXT|EST|INT./EXT|INT/EXT|I/E)[. ]",
>    "[^ \t-]+"),
> 
> or something like this, perhaps?

Good points, thanks!

This regex should be a bit sturdier:

$ cat scenes.txt 
int. yes - day
INT. YES - DAY #1A#
EXT. YES - DAY
.YES TOO
!EXT. NO
INT/EXT YES - DAY
INT./EXT YES - DAY
I/E YES - DAY
no
NO
NO.
!.NO.
int yes - day
est yes - day
!EXT. NO - DAY

$ grep -E "^((\.|(([Ii][Nn][Tt]|[Ee][Ss][Tt]|[Ee][Xx][Tt])?\\.?|[Ii]([Nn][Tt])?\.?/[Ee]([Xx][Tt])?\.?) ).+)$" scenes.txt 
int. yes - day
INT. YES - DAY #1A#
EXT. YES - DAY
.YES TOO
INT/EXT YES - DAY
INT./EXT YES - DAY
I/E YES - DAY
int yes - day
est yes - day

Revised version of patch incoming...

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

* [PATCH] userdiff: add support for Fountain documents
@ 2015-07-19 12:31 Zoë Blade
  2015-07-20 21:17 ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Zoë Blade @ 2015-07-19 12:31 UTC (permalink / raw)
  To: git; +Cc: Zoë Blade

Add support for Fountain, a plain text screenplay format.  Git
facilitates not just programming specifically, but creative writing
in general, so it makes sense to also support other plain text
documents besides source code.

In the structure of a screenplay specifically, scenes are roughly
analogous to functions, in the sense that it makes your job easier
if you can see which ones were changed in a given range of patches.

More information about the Fountain format can be found on its
official website, at http://fountain.io .

Signed-off-by: Zoë Blade <zoe@bytenoise.co.uk>
---
 Documentation/gitattributes.txt | 2 ++
 t/t4018-diff-funcname.sh        | 1 +
 t/t4018/fountain-scene          | 4 ++++
 userdiff.c                      | 2 ++
 4 files changed, 9 insertions(+)
 create mode 100644 t/t4018/fountain-scene

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 81fe586..e3b1de8 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -527,6 +527,8 @@ patterns are available:
 
 - `fortran` suitable for source code in the Fortran language.
 
+- `fountain` suitable for Fountain documents.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 1dbaa38..67373dc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -31,6 +31,7 @@ diffpatterns="
 	cpp
 	csharp
 	fortran
+	fountain
 	html
 	java
 	matlab
diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene
new file mode 100644
index 0000000..6b3257d
--- /dev/null
+++ b/t/t4018/fountain-scene
@@ -0,0 +1,4 @@
+EXT. STREET RIGHT OUTSIDE - DAY
+
+CHARACTER
+You didn't say the magic phrase, "ChangeMe".
diff --git a/userdiff.c b/userdiff.c
index 2ccbee5..5a600d6 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -35,6 +35,8 @@ IPATTERN("fortran",
 	  * they would have been matched above as a variable anyway. */
 	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
 	 "|//|\\*\\*|::|[/<>=]="),
+PATTERNS("fountain", "^((\\.|(([Ii][Nn][Tt]|[Ee][Ss][Tt]|[Ee][Xx][Tt])?\\.?|[Ii]([Nn][Tt])?\\.?/[Ee]([Xx][Tt])?\\.?) ).+)$",
+	 "[^ \t-]+"),
 PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
 	 "[^<>= \t]+"),
 PATTERNS("java",
-- 
2.5.0.rc2.28.g6003e7f.dirty

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-19 12:31 Zoë Blade
@ 2015-07-20 21:17 ` Junio C Hamano
  2015-07-21 13:23   ` Zoë Blade
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2015-07-20 21:17 UTC (permalink / raw)
  To: Zoë Blade; +Cc: git

Zoë Blade <zoe@bytenoise.co.uk> writes:

> diff --git a/userdiff.c b/userdiff.c
> index 2ccbee5..5a600d6 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -35,6 +35,8 @@ IPATTERN("fortran",
>  	  * they would have been matched above as a variable anyway. */
>  	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
>  	 "|//|\\*\\*|::|[/<>=]="),
> +PATTERNS("fountain", "^((\\.|(([Ii][Nn][Tt]|[Ee][Ss][Tt]|[Ee][Xx][Tt])?\\.?|[Ii]([Nn][Tt])?\\.?/[Ee]([Xx][Tt])?\\.?) ).+)$",
> +	 "[^ \t-]+"),

Wouldn't IPATTERN() be a better choice here?

>  PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
>  	 "[^<>= \t]+"),
>  PATTERNS("java",

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

* [PATCH] userdiff: add support for Fountain documents
@ 2015-07-21 13:22 Zoë Blade
  2015-07-21 19:33 ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Zoë Blade @ 2015-07-21 13:22 UTC (permalink / raw)
  To: git; +Cc: Zoë Blade

Add support for Fountain, a plain text screenplay format.  Git
facilitates not just programming specifically, but creative writing
in general, so it makes sense to also support other plain text
documents besides source code.

In the structure of a screenplay specifically, scenes are roughly
analogous to functions, in the sense that it makes your job easier
if you can see which ones were changed in a given range of patches.

More information about the Fountain format can be found on its
official website, at http://fountain.io .

Signed-off-by: Zoë Blade <zoe@bytenoise.co.uk>
---
 Documentation/gitattributes.txt | 2 ++
 t/t4018-diff-funcname.sh        | 1 +
 t/t4018/fountain-scene          | 4 ++++
 userdiff.c                      | 2 ++
 4 files changed, 9 insertions(+)
 create mode 100644 t/t4018/fountain-scene

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 81fe586..e3b1de8 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -527,6 +527,8 @@ patterns are available:
 
 - `fortran` suitable for source code in the Fortran language.
 
+- `fountain` suitable for Fountain documents.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 1dbaa38..67373dc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -31,6 +31,7 @@ diffpatterns="
 	cpp
 	csharp
 	fortran
+	fountain
 	html
 	java
 	matlab
diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene
new file mode 100644
index 0000000..6b3257d
--- /dev/null
+++ b/t/t4018/fountain-scene
@@ -0,0 +1,4 @@
+EXT. STREET RIGHT OUTSIDE - DAY
+
+CHARACTER
+You didn't say the magic phrase, "ChangeMe".
diff --git a/userdiff.c b/userdiff.c
index 2ccbee5..eec939a 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -35,6 +35,8 @@ IPATTERN("fortran",
 	  * they would have been matched above as a variable anyway. */
 	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
 	 "|//|\\*\\*|::|[/<>=]="),
+IPATTERN("fountain", "^((\\.|((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ).+)$",
+	 "[^ \t-]+"),
 PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
 	 "[^<>= \t]+"),
 PATTERNS("java",
-- 
1.9.5 (Apple Git-50.3)

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-20 21:17 ` Junio C Hamano
@ 2015-07-21 13:23   ` Zoë Blade
  0 siblings, 0 replies; 15+ messages in thread
From: Zoë Blade @ 2015-07-21 13:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On 20 Jul 2015, at 22:17, Junio C Hamano <gitster@pobox.com> wrote:

>> +PATTERNS("fountain", "^((\\.|(([Ii][Nn][Tt]|[Ee][Ss][Tt]|[Ee][Xx][Tt])?\\.?|[Ii]([Nn][Tt])?\\.?/[Ee]([Xx][Tt])?\\.?) ).+)$",
>> +	 "[^ \t-]+"),
> 
> Wouldn't IPATTERN() be a better choice here?

Good point, thank you!  Much better:

IPATTERN("fountain", "^((\\.|((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ).+)$", "[^ \t-]+"),

It looks like some of the others might benefit from being case insensitive too, but I'm not sure, and at any rate it would warrant a separate patch.

I'll send another revision next... :)

Thanks,
Zoë.

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-21 13:22 [PATCH] userdiff: add support for Fountain documents Zoë Blade
@ 2015-07-21 19:33 ` Junio C Hamano
  2015-07-22 16:31   ` Zoë Blade
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2015-07-21 19:33 UTC (permalink / raw)
  To: Zoë Blade; +Cc: git

Zoë Blade <zoe@bytenoise.co.uk> writes:

Hmmmm, the pattern has too many question marks to make a simple
panda brain spin.

> "^((\\.|((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?) ).+)$"

it can start with a dot, or match "something" at the beginning,
followed by a SP (is a tab allowed there instead of SP, I have to
wonder).

One problem I noticed immediately: this allows a line that begins
with "...", which I learned in http://fountain.io/syntax when I
wrote $gmane/274127 is explicitly excluded.  A line that begin with
a dot followed by a non-dot is a forced scene heading.

Now disecting that "something" (i.e. not a forced scene heading),
which is this part ...

> ((int|est|ext)?\\.?|i(nt)?\\.?/e(xt)?\\.?)

... that gives us largely two choices:

 - It can begin with zero or one int/est/ext and can optionally be
   followed by a dot, or

 - It can be one of (i, int), optionally followed by a dot, followed
   by slash, followed by one of (e, ext), optionally followed by a
   dot.

Second problem.  Doesn't the early part of this "something" pattern
allow an empty string to match by having zero "int" and zero "."?

With this edit to the test vector (add either one of these two,
removing the other, before you run this test twice), you can see
that these over-eager matches in action.
----------------------------------------------------------------

 t/t4018/fountain-scene | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t4018/fountain-scene b/t/t4018/fountain-scene
index 6b3257d..94f0513 100644
--- a/t/t4018/fountain-scene
+++ b/t/t4018/fountain-scene
@@ -1,4 +1,7 @@
 EXT. STREET RIGHT OUTSIDE - DAY
 
+ An indented line is WRONG.
+...we do not want ellipses.
+
 CHARACTER
 You didn't say the magic phrase, "ChangeMe".

----------------------------------------------------------------

Perhaps the pattern is trying to be too clever for its own good.
I'd prefer to see us doing simple, stupid and obviously correct.

That "syntax" page says:

    You can "force" a Scene Heading by starting the line with a
    single period. ... Note that only a single leading period
    followed by an alphanumeric character will force a Scene
    Heading. This allows the writer to begin Action and Dialogue
    elements with ellipses ...

which would give us the first part, i.e. the line may start with a
dot and then an alnum

\\.[a-z0-9]

And then

    A line beginning with any of the following, followed by either a dot
    or a space, is considered a Scene Heading (unless the line is
    preceded by an exclamation point !). Case insensitive.

    INT
    EXT
    EST
    INT./EXT
    INT/EXT
    I/E

which translates to

(int|ext|est|int\\.?/ext|i/e)[. ]

So taking these all together, something like this?

^((\\.[a-z0-9]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$

I personally prefer to make it slightly lenient to exclude dot
instead of forcing US-ASCII view of alnum, i.e.

^((\\.[^.]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$

I'll queue a SQUASH??? on top while waiting for a response.

Thanks.

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-21 19:33 ` Junio C Hamano
@ 2015-07-22 16:31   ` Zoë Blade
  2015-07-29 11:19     ` Zoë Blade
  0 siblings, 1 reply; 15+ messages in thread
From: Zoë Blade @ 2015-07-22 16:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi again Junio!

Yes, your more elegant and accurate regex sounds much better than the way I was trying it. :)  Please, go ahead and use yours instead.  Thank you for your help!

Thanks,
Zoë.

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

* Re: [PATCH] userdiff: add support for Fountain documents
  2015-07-22 16:31   ` Zoë Blade
@ 2015-07-29 11:19     ` Zoë Blade
  0 siblings, 0 replies; 15+ messages in thread
From: Zoë Blade @ 2015-07-29 11:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi again!

Where's this at?  Your last regex looks perfect to me:

^((\\.[^.]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$

Do you need anything else from me?

Thanks,
Zoë.

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

end of thread, other threads:[~2015-07-29 11:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-21 13:22 [PATCH] userdiff: add support for Fountain documents Zoë Blade
2015-07-21 19:33 ` Junio C Hamano
2015-07-22 16:31   ` Zoë Blade
2015-07-29 11:19     ` Zoë Blade
  -- strict thread matches above, loose matches on Subject: below --
2015-07-19 12:31 Zoë Blade
2015-07-20 21:17 ` Junio C Hamano
2015-07-21 13:23   ` Zoë Blade
2015-07-17 14:21 Zoë Blade
2015-07-17 17:12 ` Junio C Hamano
2015-07-17 22:43   ` Junio C Hamano
2015-07-19 12:30     ` Zoë Blade
2015-07-17 11:59 Zoë Blade
2015-07-17 13:03 ` Johannes Schindelin
2015-07-17 14:03   ` Zoë Blade
2015-07-17 16:20     ` Johannes Schindelin

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