diff: "lisp" userdiff_driver

From: "Scott L. Burson" <[email protected]>

The "scheme" driver doesn't quite work for Common Lisp. This driver is very generic and should work for almost any dialect of Lisp, including Common Lisp.

---
diff: "lisp" userdiff_driver

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2000%2Fslburson%2Flisp-userdiff_driver-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2000/slburson/lisp-userdiff_driver-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2000

 userdiff.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index fe710a68bf..e127b4a1f1 100644
@@ -249,6 +249,14 @@ PATTERNS("kotlin",
 	 "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
 	 /* unary and binary operators */
 	 "|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
+PATTERNS("lisp",
+	 /* Either an unindented left paren, or a slightly indented line
+	  * starting with "(def" */
+	 "^((\\(|:space:{1,2}\\(def).*)$",
+	 /* Common Lisp symbol syntax allows arbitrary strings between vertical bars */
+	 "\\|([^\\\\]|\\\\\\\\|\\\\\\|)*\\|"
+	 /* All other words are delimited by spaces or parentheses/brackets/braces */
+	 "|([^][(){} \t])+"),
 PATTERNS("markdown",
 	 "^ {0,3}#{1,6}[ \t].*",
 	 /* -- */

base-commit: fd372d9b1a69a01a676398882bbe3840bf51fe72

gitgitgadget

[Cc the author of the Scheme driver]

Am 15.11.25 um 11:17 schrieb Scott L. Burson via GitGitGadget:

"Doesn't quite work" is unfortunately a description of the problem that does not help a reviewer decide whether this change is justified. Please add a lot more details why the Scheme driver is unsuitable for Lisp and why a new driver is needed.

It is customary to mark changes to the drivers in the subject line with "userdiff:". Have a look at git log userdiff.c. It would be appreciated to stay away from nerdy tokens like "userdiff_driver" when the change can be summarized in plain English language.

Here is text I propose for the commit message:

----
Common Lisp has top-level forms 'defun' and 'deftype' that are not matched by the current Scheme pattern. Also, it is more common when defining user macros intended as top-level forms to prefix their names with "def" instead of "define"; such forms are also not matched. And some such forms don't even begin with "def".

On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives.
----

I discussed the pattern with some other experienced Common Lisp developers on a mailing list, and this is what I settled on after incorporating their feedback.

Will do.

That is a good question. I don't think so, but I don't work in Scheme. I see that you have CC'ed Atharva Raykar; let's see whether he would have any objection.

I would point out that Scheme is a dialect of Lisp, not the other way around. (Lisp is unusual in being a family of languages, rather than a single language.) And having a separate "lisp" driver might aid discoverability.

But I understand: Scheme got their driver in first, and you have to fight against the tendency of the driver list to grow unboundedly.

Ooh, that reminds me: if we do decide to add a "lisp" driver, I'll also need to add it to 'Documentation/gitattributes.adoc'.

Noted.

It's not important, but technically, Common Lisp allows an escaped backslash between vertical bars, but the R7RS formal grammar does not. However, I just tried Chicken Scheme, which claims to be at least partially R7RS compliant, and it does accept the escaped backslash. I am left to conclude that Scheme implementors think that the omission of the escaped backslash from the R7RS formal grammar is an oversight (I think so too).

Of course, no one would actually write a symbol name with an escaped backslash in it unless they were submitting to an obfuscated Lisp contest. So we are really being pedantic here. Still, may as well allow it.

Atharva, any comments?

-- Scott

Without "going there," I think there are enough differences to warrant a different driver. (OTOH, I have sometimes wanted to teach the Scheme driver that most "def" things are probably definitions.) Our indentation is less rigid in that indented forms may be more deeply nested than only one or two spaces (and we of course have more definition forms than "only things starting with def"), and I don't understand the downthread desire to not permit tabs.

As for Scheme community, I'll suggest asking on the Racket channels (Discourse is probably best if you want a mailing-list-like discussion?)

D. Ben Knoble

The proposal on the table is not to replace the current Scheme regexp with the Lisp one, but rather to disjoin them, so as to match any line that either one would match. So you don't need to worry about false negatives.

The thing about tabs was that, the way I had initially written the regexp, it would have matched a line starting with one or two tabs, or a tab and a space -- but no other whitespace -- followed by "(def". This was not my intention. I was just trying to match lines starting with a space or two and "(def". Again, this is in addition to the lines currently being matched by the Scheme regexp.

I have asked on Reddit under /r/scheme, and got only the same objection that you offered ("define" etc. forms more deeply nested), with the same resolution (disjoining the new pattern to the existing one).

Reddit says my post got some 4300 views. That seems like an adequate sample to me, but I can try elsewhere if you think it's important.

I have an updated version of the patch ready; I will submit it shortly.

As we generally assume that the file being edited is syntactically sound, even if one lisp variant understands "(deffoo" and others do not, it should be generally fine for the pattern to say something like "at the beginning of the line, optionally following a few spaces, four-letter sequence '(def' is likely to be the beginning of a function definition", as long as there is some convention that user defined functions and macros, unless they are to behave similarly to "(defun", would not be named so confusingly to start with d-e-f.

It would be nice if a single set of rules can cover what existing scheme patterns cover, Emacs lisp, and Common lisp.

Hmm, we haven't heard from Atharva. I'll try asking around in the Scheme community.

The regex I proposed has a bug. The use of the Posix character class is incorrect, because that class includes tabs. I will replace it with a literal space. Also, many Lisps, including Common Lisp in its default configuration, are case-insensitive, and at least in the 1970s, it wasn't completely unheard-of to write Lisp code in uppercase; I'll change the entry to use 'IPATTERN'.

Agreed, but this is not the most important point. The greater potential for false positives comes from the rule (in my proposal) that a left parenthesis in column 0 is taken as indicating a top-level definition, without even looking at the following characters. Although Lisp dialects certainly vary, I have not seen one in which standard indentation practice does not indent internal expressions; certainly, Lisp mode in Emacs indents them. And, I think the rule really does need to be that broad, because top-level forms don't always begin with "def"; indeed, one can put any executable expression at top level in a source file to perform load-time initializations.

It's only when there is some indentation that I think the regex needs to require a word beginning with "def".

Agreed. I do think it would be a little better for non-Scheme users if the single driver were named "lisp" instead of "scheme". Renaming the driver out from under the Scheme community, though, seems like it would be unfriendly, even after a deprecation period.

One solution would be to add an aliasing mechanism to the driver table. Perhaps there would be other use cases for it. If you would consider a patch along these lines, I can code it up.

I didn't respond to that part as I didn't know if you were serious or joking ;-).

Exactly, but the more important question is are they considered as the beginning of an important, and sematically distinct, block, just like the beginning of a function is. I am somewhat negative to the "anything not indented is a beginning of a significant group", as I do not know how well it meshes with the "(defXX is a beginning of a function", when they are used together.

It is not a particularly interesting part of the problem, simply because as the first approximation, we can just advertise "you can mark your lisp files as 'scheme'". A more interesting issue is if we can indeed come up with such a superset of patterns that can cover all Lisp variants that matter.

Common Lisp, Emacs Lisp, and other dialects have some top-level forms, most importantly 'defun', that are not matched by the current Scheme pattern. Also, it is common in these dialects, when defining user macros intended as top-level forms, to prefix their names with "def" instead of "define"; such forms are also not currently matched. Some such forms don't even begin with "def".

On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives.

This commit disjoins a regexp employing these heuristics to the existing Scheme regexp, so it will still match everything that it did previously.

Changes since v1:

* unified with Scheme driver
* fixed whitespace bug (tabs were allowed incorrectly)
* made "(def" case-insensitive
* improved commit summary line
* improved commit description

Scott L. Burson (2):
diff: "lisp" userdiff_driver
merge with Scheme regexp; fix bugs

 userdiff.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

base-commit: fd372d9b1a69a01a676398882bbe3840bf51fe72
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2000%2Fslburson%2Flisp-userdiff_driver-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2000/slburson/lisp-userdiff_driver-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2000

Range-diff vs v1:

 1:  da99bb0bcd = 1:  da99bb0bcd diff: "lisp" userdiff_driver
 -:  ---------- > 2:  86315aa3e3 merge with Scheme regexp; fix bugs

gitgitgadget

From: "Scott L. Burson" <[email protected]>

The "scheme" driver doesn't quite work for Common Lisp. This driver is very generic and should work for almost any dialect of Lisp, including Common Lisp.

---
 userdiff.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index fe710a68bf..e127b4a1f1 100644
@@ -249,6 +249,14 @@ PATTERNS("kotlin",
 	 "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
 	 /* unary and binary operators */
 	 "|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
+PATTERNS("lisp",
+	 /* Either an unindented left paren, or a slightly indented line
+	  * starting with "(def" */
+	 "^((\\(|:space:{1,2}\\(def).*)$",
+	 /* Common Lisp symbol syntax allows arbitrary strings between vertical bars */
+	 "\\|([^\\\\]|\\\\\\\\|\\\\\\|)*\\|"
+	 /* All other words are delimited by spaces or parentheses/brackets/braces */
+	 "|([^][(){} \t])+"),
 PATTERNS("markdown",
 	 "^ {0,3}#{1,6}[ \t].*",
 	 /* -- */

gitgitgadget

On Wed, Nov 26, 2025, 6:38 PM Scott L. Burson via GitGitGadget <[email protected]> wrote:
[duplicate of first message]

Hmm. Somehow I have screwed things up so that GitGitGadget sends out only the first commit, not including the changes in the second, nor using the PR title and the text in the description. Sorry for the noise. Is there an easy fix, or do I need to make a new PR?

-- Scott

Am 27.11.25 um 11:32 schrieb Scott L. Burson:

I think all went as expected as far as GGG is concerend. You made a new commit on top of the one in the earlier round, and then asked GGG to submit the PR to the mailing list. GGG made a patch series with a cover letter and the two patches. That's expected, because the first patch hasn't been integrated in upstream Git, yet.

If you intended to send just the second patch (as a fixup of the first one), then GGG cannot help you, not even if you make another pull request.

But you shouldn't have sent a fixup commit anyway, but that is a matter I'll address in a separate message.

-- Hannes

From: "Scott L. Burson" <[email protected]>

This commit merges (by disjoining) the new generic Lisp regexp into the existing Scheme regexp. It also fixes two bugs: the new regexp was unintentionally allowing tabs, and the matching of "(def" should be case-insensitive.

---
 userdiff.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

index e127b4a1f1..b67dfddbef 100644
@@ -249,14 +249,6 @@ PATTERNS("kotlin",
 	 "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
 	 /* unary and binary operators */
 	 "|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
-PATTERNS("lisp",
-	 /* Either an unindented left paren, or a slightly indented line
-	  * starting with "(def" */
-	 "^((\\(|:space:{1,2}\\(def).*)$",
-	 /* Common Lisp symbol syntax allows arbitrary strings between vertical bars */
-	 "\\|([^\\\\]|\\\\\\\\|\\\\\\|)*\\|"
-	 /* All other words are delimited by spaces or parentheses/brackets/braces */
-	 "|([^][(){} \t])+"),
 PATTERNS("markdown",
 	 "^ {0,3}#{1,6}[ \t].*",
 	 /* -- */

@@ -352,14 +344,21 @@ PATTERNS("rust",
 	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"
 	 "|[-+*\\/<>%&^|=!:]=|<<=?|>>=?|&&|\\|\\||->|=>|\\.{2}=|\\.{3}|::"),
 PATTERNS("scheme",
-	 "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$",
+	 /* A possibly indented left paren followed by a Scheme keyword. */
+	 "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$\n"
+	 /*
+	  * For other Lisp dialects: either an unindented left paren, or a
+	  * slightly indented line starting with "(def".
+	  */
+	 "^((\\(| {1,2}\\([Dd][Ee][Ff]).*)$",
 	 /*
-	  * R7RS valid identifiers include any sequence enclosed
-	  * within vertical lines having no backslashes
+	  * The union of R7RS and Common Lisp symbol syntax: allows arbitrary
+	  * strings between vertical bars, including escaped backslashes and
+	  * vertical bars.
 	  */
-	 "\\|([^\\\\]*)\\|"
+	 "\\|([^\\\\]|\\\\\\\\|\\\\\\|)*\\|"
 	 /* All other words should be delimited by spaces or parentheses */
-	 "|([^][)(}{[ \t])+"),
+	 "|([^][)(}{ \t])+"),
 PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
 	 "\\\\[a-zA-Z@]+|\\\\.|([a-zA-Z0-9]|[^\x01-\x7f])+"),
 { .name = "default", .binary = -1 },

gitgitgadget

Am 27.11.25 um 03:38 schrieb Scott L. Burson via GitGitGadget:

	[\t ]*(
		\((
			(
				define|def(
					struct|syntax|class|method
					|rules|record|proto|alias
				)?
			)[-*/ \t]
			|
			(
				library|module|struct|class
			)[*+ \t]
		).*
	)$

	/*
	 * An unindented opening parenthesis identifies a top-level
         * structure in all Lisp dialects.
	 */
	"^(\\(.*)$\n",

	"^  ?(\\([Dd][Ee][Ff].*)$",

	\|([^\\]|\\\\|\\\|)*\|

Am 27.11.25 um 17:09 schrieb Johannes Sixt:

Actually, no. Regular expressions don't choose the first match if a different alternative gives a longer match in total. For example, for the change

-  (let ((|one two| |three four|)))
+  (let ((|1 two| |three four|)))

we get to see the word diff

(let (([-|one two| |three four|-]{+|1 two| |three four|+})))

but the desired result is

(let (([-|one two|-]{+|1 two|+} |three four|)))

The problem isn't new with the proposed change, but if we change the RE, we could fix this at the same time. I think it helps to include | in the bracket expression. It may be worth its own patch that also adds a test in t/t4034/scheme/.

The worddiff test case is a bit too sloppy. I've tightened it in the patch below. You may want to make it the first of your series. (If you do, don't forget to apply your sign-off when you cherry-pick the commit.) It is also available here:
https://github.com/j6t/git/tree/userdiff-scheme
(commit 8f6cb42a02cc).

----- 8< -----
userdiff: tighten word-diff test case of the scheme driver

The scheme driver separates identifiers only at parentheses of all sorts and whitespace, except that vertical bars act as brackets that enclose an identifier.

The test case attempts to demonstrate the vertical bars with a change from 'some-text' to '|a greeting|'. However, this misses the goal because the same word coloring would be applied if '|a greeting|' were parsed as two words.

Have an identifier between vertical bars with a space in both the pre- and the post-image and change only one side of the space to show that the single word exists between the vertical bars.

Also add cases that change parentheses of all kinds in a sequence of parentheses to show that they are their own word each.

---
 t/t4034/scheme/expect | 5 +++--
 t/t4034/scheme/post   | 1 +
 t/t4034/scheme/pre    | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

index 496cd5de8c..138abe9f56 100644
@@ -2,10 +2,11 @@
 <BOLD>index 74b6605..63b6ac4 100644<RESET>
 <BOLD>--- a/pre<RESET>
 <BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,6 +1,6 @@<RESET>
+<CYAN>@@ -1,7 +1,7 @@<RESET>
 (define (<RED>myfunc a b<RESET><GREEN>my-func first second<RESET>)
   ; This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function.
   (<RED>this\place<RESET><GREEN>that\place<RESET> (+ 3 4))
-  (define <RED>some-text<RESET><GREEN>|a greeting|<RESET> "hello")
+  (define <RED>|the greeting|<RESET><GREEN>|a greeting|<RESET> "hello")
+  ({<RED>}<RESET>(([<RED>]<RESET>(func-n)<RED>[<RESET>]))<RED>{<RESET>})
   (let ((c (<RED>+ a b<RESET><GREEN>add1 first<RESET>)))
     (format "one more than the total is %d" (<RED>add1<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))

index 63b6ac4f87..0e3bab101d 100644
@@ -2,5 +2,6 @@
   ; This is a (moderately) cool function.
   (that\place (+ 3 4))
   (define |a greeting| "hello")
+  ({(([(func-n)]))})
   (let ((c (add1 first)))
     (format "one more than the total is %d" (+ c second))))

index 74b6605357..03d77c7c43 100644
@@ -1,6 +1,7 @@
 (define (myfunc a b)
   ; This is a really cool function.
   (this\place (+ 3 4))
-  (define some-text "hello")
+  (define |the greeting| "hello")
+  ({}(([](func-n)[])){})
   (let ((c (+ a b)))
     (format "one more than the total is %d" (add1 c))))

2.52.0.rc0.206.g6c0125c11f

Ah, good catch.

I noticed another bug. At least in Common Lisp, and I expect also in Scheme, while backslash and vertical bar are the only characters that must be escaped to be included, in fact any character _may_ be escaped. (This came to my attention when Emacs Paredit escaped a double-quote for me, between vertical bars, unnecessarily. Of course, in a string, double-quote would need to be escaped.)

So the correct regexp, with both of these bugs fixed, is

    "\\|([^|\\\\]|\\\\.)*\\|"

Or, without the C quoting:

    \|([^|\\]|\\.)*\|

For example, for

Done.

I updated the existing test to check for both of these bugs (and verified that it did so by reintroducing them). It's all in this one patch.

The branch now contains two commits, yours and mine. Do I just do /submit at this point, or do I need to submit them separately?

Am 14.01.26 um 07:18 schrieb Scott L. Burson:

You should submit both commits with a single /submit command.

Before you do so, though, you may want to update the authorship of my patch (git commit --author="Johannes Sixt <[email protected]>" --amend). That you have appended your own sign-off to the commit message is very much appreciated.

-- Hannes

Common Lisp, Emacs Lisp, and other dialects have some top-level forms, most importantly 'defun', that are not matched by the current Scheme pattern. Also, it is common in these dialects, when defining user macros intended as top-level forms, to prefix their names with "def" instead of "define"; such forms are also not currently matched. Some such forms don't even begin with "def".

On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives.

This commit disjoins a regexp employing these heuristics to the existing Scheme regexp, so it will still match everything that it did previously.

Johannes Sixt (1):
userdiff: tighten word-diff test case of the scheme driver

Scott L. Burson (1):
userdiff: extend Scheme support to cover other Lisp dialects

 Documentation/gitattributes.adoc           |  1 +
 t/t4018/scheme-lisp-defun-a                |  4 ++++
 t/t4018/scheme-lisp-defun-b                |  4 ++++
 t/t4018/scheme-lisp-eval-when              |  4 ++++
 t/t4018/{scheme-module => scheme-module-a} |  0
 t/t4018/scheme-module-b                    |  6 ++++++
 t/t4034/scheme/expect                      |  5 +++--
 t/t4034/scheme/post                        |  3 ++-
 t/t4034/scheme/pre                         |  3 ++-
 userdiff.c                                 | 22 ++++++++++++++++------
 10 files changed, 42 insertions(+), 10 deletions(-)
 create mode 100644 t/t4018/scheme-lisp-defun-a
 create mode 100644 t/t4018/scheme-lisp-defun-b
 create mode 100644 t/t4018/scheme-lisp-eval-when
 rename t/t4018/{scheme-module => scheme-module-a} (100%)
 create mode 100644 t/t4018/scheme-module-b

base-commit: 8745eae506f700657882b9e32b2aa00f234a6fb6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2000%2Fslburson%2Flisp-userdiff_driver-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2000/slburson/lisp-userdiff_driver-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2000

Range-diff vs v2:

 1:  da99bb0bcd < -:  ---------- diff: "lisp" userdiff_driver
 2:  86315aa3e3 < -:  ---------- merge with Scheme regexp; fix bugs
 -:  ---------- > 1:  e20ac5b6a6 userdiff: tighten word-diff test case of the scheme driver
 -:  ---------- > 2:  fb4c8dc5d4 userdiff: extend Scheme support to cover other Lisp dialects

gitgitgadget

From: Johannes Sixt <[email protected]>

The scheme driver separates identifiers only at parentheses of all sorts and whitespace, except that vertical bars act as brackets that enclose an identifier.

The test case attempts to demonstrate the vertical bars with a change from 'some-text' to '|a greeting|'. However, this misses the goal because the same word coloring would be applied if '|a greeting|' were parsed as two words.

Have an identifier between vertical bars with a space in both the pre- and the post-image and change only one side of the space to show that the single word exists between the vertical bars.

Also add cases that change parentheses of all kinds in a sequence of parentheses to show that they are their own word each.

---
 t/t4034/scheme/expect | 5 +++--
 t/t4034/scheme/post   | 1 +
 t/t4034/scheme/pre    | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

index 496cd5de8c..138abe9f56 100644
@@ -2,10 +2,11 @@
 <BOLD>index 74b6605..63b6ac4 100644<RESET>
 <BOLD>--- a/pre<RESET>
 <BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,6 +1,6 @@<RESET>
+<CYAN>@@ -1,7 +1,7 @@<RESET>
 (define (<RED>myfunc a b<RESET><GREEN>my-func first second<RESET>)
   ; This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function.
   (<RED>this\place<RESET><GREEN>that\place<RESET> (+ 3 4))
-  (define <RED>some-text<RESET><GREEN>|a greeting|<RESET> "hello")
+  (define <RED>|the greeting|<RESET><GREEN>|a greeting|<RESET> "hello")
+  ({<RED>}<RESET>(([<RED>]<RESET>(func-n)<RED>[<RESET>]))<RED>{<RESET>})
   (let ((c (<RED>+ a b<RESET><GREEN>add1 first<RESET>)))
     (format "one more than the total is %d" (<RED>add1<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))

index 63b6ac4f87..0e3bab101d 100644
@@ -2,5 +2,6 @@
   ; This is a (moderately) cool function.
   (that\place (+ 3 4))
   (define |a greeting| "hello")
+  ({(([(func-n)]))})
   (let ((c (add1 first)))
     (format "one more than the total is %d" (+ c second))))

index 74b6605357..03d77c7c43 100644
@@ -1,6 +1,7 @@
 (define (myfunc a b)
   ; This is a really cool function.
   (this\place (+ 3 4))
-  (define some-text "hello")
+  (define |the greeting| "hello")
+  ({}(([](func-n)[])){})
   (let ((c (+ a b)))
     (format "one more than the total is %d" (add1 c))))

gitgitgadget

From: "Scott L. Burson" <[email protected]>

Common Lisp has top-level forms, such as 'defun' and 'defmacro', that are not matched by the current Scheme pattern. Also, it is more common in CL, when defining user macros intended as top-level forms, to prefix their names with "def" instead of "define"; such forms are also not matched. And some top-level forms don't even begin with "def".

On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives.

---
 Documentation/gitattributes.adoc           |  1 +
 t/t4018/scheme-lisp-defun-a                |  4 ++++
 t/t4018/scheme-lisp-defun-b                |  4 ++++
 t/t4018/scheme-lisp-eval-when              |  4 ++++
 t/t4018/{scheme-module => scheme-module-a} |  0
 t/t4018/scheme-module-b                    |  6 ++++++
 t/t4034/scheme/expect                      |  2 +-
 t/t4034/scheme/post                        |  2 +-
 t/t4034/scheme/pre                         |  2 +-
 userdiff.c                                 | 22 ++++++++++++++++------
 10 files changed, 38 insertions(+), 9 deletions(-)
 create mode 100644 t/t4018/scheme-lisp-defun-a
 create mode 100644 t/t4018/scheme-lisp-defun-b
 create mode 100644 t/t4018/scheme-lisp-eval-when
 rename t/t4018/{scheme-module => scheme-module-a} (100%)
 create mode 100644 t/t4018/scheme-module-b

index f20041a323..a9ce5adef9 100644
@@ -912,6 +912,7 @@ patterns are available:
 - `rust` suitable for source code in the Rust language.
 
 - `scheme` suitable for source code in the Scheme language.
+Also handles Emacs Lisp, Common Lisp, and most other dialects.
 
 - `tex` suitable for source code for LaTeX documents.
 

new file mode 100644
index 0000000000..c3c750f76d
@@ -0,0 +1,4 @@
+(defun some-func (x y z) RIGHT
+  (let ((a x)
+        (b y))
+        (ChangeMe a b)))

new file mode 100644
index 0000000000..21be305968
@@ -0,0 +1,4 @@
+(macrolet ((foo (x) `(bar ,x)))
+  (defun mumble (x) ; RIGHT
+    (when (> x 0)
+      (foo x)))) ; ChangeMe

new file mode 100644
index 0000000000..5d941d7e0e
@@ -0,0 +1,4 @@
+(eval-when (:compile-toplevel :load-toplevel :execute)  ; RIGHT
+  (set-macro-character #\?
+		       (lambda (stream char)
+			 `(make-pattern-variable ,(read stream)))))  ; ChangeMe

similarity index 100%
rename from t/t4018/scheme-module
rename to t/t4018/scheme-module-a

new file mode 100644
index 0000000000..77bc0c5eff
@@ -0,0 +1,6 @@
+(module A
+  (export with-display-exception)
+  (extern (display-exception display-exception))
+  (def (with-display-exception thunk) RIGHT
+    (with-catch (lambda (e) (display-exception e (current-error-port)) e)
+      thunk ChangeMe)))

index 138abe9f56..72592665f1 100644
@@ -6,7 +6,7 @@
 (define (<RED>myfunc a b<RESET><GREEN>my-func first second<RESET>)
   ; This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function.
   (<RED>this\place<RESET><GREEN>that\place<RESET> (+ 3 4))
-  (define <RED>|the greeting|<RESET><GREEN>|a greeting|<RESET> "hello")
+  (define <RED>|the \greeting|<RESET><GREEN>|a \greeting|<RESET> |hello there|)
   ({<RED>}<RESET>(([<RED>]<RESET>(func-n)<RED>[<RESET>]))<RED>{<RESET>})
   (let ((c (<RED>+ a b<RESET><GREEN>add1 first<RESET>)))
     (format "one more than the total is %d" (<RED>add1<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))

index 0e3bab101d..450cc234f7 100644
@@ -1,7 +1,7 @@
 (define (my-func first second)
   ; This is a (moderately) cool function.
   (that\place (+ 3 4))
-  (define |a greeting| "hello")
+  (define |a \greeting| |hello there|)
   ({(([(func-n)]))})
   (let ((c (add1 first)))
     (format "one more than the total is %d" (+ c second))))

index 03d77c7c43..ba8b8ac0a4 100644
@@ -1,7 +1,7 @@
 (define (myfunc a b)
   ; This is a really cool function.
   (this\place (+ 3 4))
-  (define |the greeting| "hello")
+  (define |the \greeting| |hello there|)
   ({}(([](func-n)[])){})
   (let ((c (+ a b)))
     (format "one more than the total is %d" (add1 c))))

index fe710a68bf..b5412e6bc3 100644
@@ -344,14 +344,24 @@ PATTERNS("rust",
 	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"
 	 "|[-+*\\/<>%&^|=!:]=|<<=?|>>=?|&&|\\|\\||->|=>|\\.{2}=|\\.{3}|::"),
 PATTERNS("scheme",
-	 "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$",
 	 /*
-	  * R7RS valid identifiers include any sequence enclosed
-	  * within vertical lines having no backslashes
+	  * An unindented opening parenthesis identifies a top-level
+	  * expression in all Lisp dialects.
 	  */
-	 "\\|([^\\\\]*)\\|"
-	 /* All other words should be delimited by spaces or parentheses */
-	 "|([^][)(}{[ \t])+"),
+	 "^(\\(.*)$\n"
+	 /* For Scheme: a possibly indented left paren followed by a keyword. */
+	 "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$\n"
+	 /*
+	  * For all Lisp dialects: a slightly indented line starting with "(def".
+	  */
+	 "^  ?(\\([Dd][Ee][Ff].*)$",
+	 /*
+	  * The union of R7RS and Common Lisp symbol syntax: allows arbitrary
+	  * strings between vertical bars, including any escaped characters.
+	  */
+	 "\\|([^|\\\\]|\\\\.)*\\|"
+	 /* All other words should be delimited by spaces or parentheses. */
+	 "|([^][)(}{ \t])+"),
 PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
 	 "\\\\[a-zA-Z@]+|\\\\.|([a-zA-Z0-9]|[^\x01-\x7f])+"),
 { .name = "default", .binary = -1 },

gitgitgadget

Am 16.01.26 um 00:18 schrieb Scott L. Burson via GitGitGadget:

The commit message doesn't mention the changes regarding the word-diff pattern. I would have prefered to have them in their own patch; it would make the patch text less obscure about what it actually changes.

Okay, will do.

Of course I will fix the indentation, but I don't agree with your proposal for the text. There are many Lisp dialects in use, indeed probably thousands; lots of people write their own. As previously noted, matching an unindented open parenthesis is a very general heuristic that is likely to work for the vast majority of dialects. While we can't answer the question "is the dialect I'm using covered?" for everyone, I think the text should encourage them to give the driver a try.

So how about this:

- scheme suitable for source code in most Lisp dialects, including Scheme, Emacs Lisp, Common Lisp, and Clojure.

I've looked at some Clojure and I believe the proposed regexp will work for it. I think it's a good idea to mention it explicitly, because people might search the text for it.

Agreed, will do.

-- Scott

Am 17.01.26 um 03:09 schrieb Scott L. Burson:

OK, let's leave it at that.

-- Hannes

Common Lisp, Emacs Lisp, and other dialects have some top-level forms, most importantly 'defun', that are not matched by the current Scheme pattern. Also, it is common in these dialects, when defining user macros intended as top-level forms, to prefix their names with "def" instead of "define"; such forms are also not currently matched. Some such forms don't even begin with "def".

On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives.

This commit disjoins a regexp employing these heuristics to the existing Scheme regexp, so it will still match everything that it did previously.

Johannes Sixt (1):
userdiff: tighten word-diff test case of the scheme driver

Scott L. Burson (1):
userdiff: extend Scheme support to cover other Lisp dialects

 Documentation/gitattributes.adoc           |  3 ++-
 t/t4018/scheme-lisp-defun-a                |  4 ++++
 t/t4018/scheme-lisp-defun-b                |  4 ++++
 t/t4018/scheme-lisp-eval-when              |  4 ++++
 t/t4018/{scheme-module => scheme-module-a} |  0
 t/t4018/scheme-module-b                    |  6 ++++++
 t/t4034/scheme/expect                      |  5 +++--
 t/t4034/scheme/post                        |  3 ++-
 t/t4034/scheme/pre                         |  3 ++-
 userdiff.c                                 | 22 ++++++++++++++++------
 10 files changed, 43 insertions(+), 11 deletions(-)
 create mode 100644 t/t4018/scheme-lisp-defun-a
 create mode 100644 t/t4018/scheme-lisp-defun-b
 create mode 100644 t/t4018/scheme-lisp-eval-when
 rename t/t4018/{scheme-module => scheme-module-a} (100%)
 create mode 100644 t/t4018/scheme-module-b

base-commit: 9e8f4e9c04e3efa494e78b710e0c5f6cc77a0a5e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2000%2Fslburson%2Flisp-userdiff_driver-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2000/slburson/lisp-userdiff_driver-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/2000

Range-diff vs v3:

 1:  e20ac5b6a6 = 1:  8e0b1e3d01 userdiff: tighten word-diff test case of the scheme driver
 2:  fb4c8dc5d4 ! 2:  0bd51e02ba userdiff: extend Scheme support to cover other Lisp dialects

@@ Commit message

## Documentation/gitattributes.adoc ##
@@ Documentation/gitattributes.adoc: patterns are available:
+
- rust suitable for source code in the Rust language.

- - scheme suitable for source code in the Scheme language. -+Also handles Emacs Lisp, Common Lisp, and most other dialects. +-- scheme suitable for source code in the Scheme language. ++- scheme suitable for source code in most Lisp dialects, ++ including Scheme, Emacs Lisp, Common Lisp, and Clojure.

- tex suitable for source code for LaTeX documents.

@@ t/t4034/scheme/expect
; This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function. (<RED>this\place<RESET><GREEN>that\place<RESET> (+ 3 4))
- (define <RED>|the greeting|<RESET><GREEN>|a greeting|<RESET> "hello")
-+ (define <RED>|the \greeting|<RESET><GREEN>|a \greeting|<RESET> |hello there|)
++ (define <RED>|the \| \greeting|<RESET><GREEN>|a \greeting|<RESET> |hello there|)
({<RED>}<RESET>(([<RED>]<RESET>(func-n)<RED>[<RESET>]))<RED>{<RESET>})
(let ((c (<RED>+ a b<RESET><GREEN>add1 first<RESET>)))
(format "one more than the total is %d" (<RED>add1<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))
@@ t/t4034/scheme/pre
; This is a really cool function.
(this\place (+ 3 4))
- (define |the greeting| "hello")
-+ (define |the \greeting| |hello there|)
++ (define |the \| \greeting| |hello there|)
({}(([](func-n)[])){})
(let ((c (+ a b)))
(format "one more than the total is %d" (add1 c))))

gitgitgadget

From: Johannes Sixt <[email protected]>

The scheme driver separates identifiers only at parentheses of all sorts and whitespace, except that vertical bars act as brackets that enclose an identifier.

The test case attempts to demonstrate the vertical bars with a change from 'some-text' to '|a greeting|'. However, this misses the goal because the same word coloring would be applied if '|a greeting|' were parsed as two words.

Have an identifier between vertical bars with a space in both the pre- and the post-image and change only one side of the space to show that the single word exists between the vertical bars.

Also add cases that change parentheses of all kinds in a sequence of parentheses to show that they are their own word each.

---
 t/t4034/scheme/expect | 5 +++--
 t/t4034/scheme/post   | 1 +
 t/t4034/scheme/pre    | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

index 496cd5de8c..138abe9f56 100644
@@ -2,10 +2,11 @@
 <BOLD>index 74b6605..63b6ac4 100644<RESET>
 <BOLD>--- a/pre<RESET>
 <BOLD>+++ b/post<RESET>
-<CYAN>@@ -1,6 +1,6 @@<RESET>
+<CYAN>@@ -1,7 +1,7 @@<RESET>
 (define (<RED>myfunc a b<RESET><GREEN>my-func first second<RESET>)
   ; This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function.
   (<RED>this\place<RESET><GREEN>that\place<RESET> (+ 3 4))
-  (define <RED>some-text<RESET><GREEN>|a greeting|<RESET> "hello")
+  (define <RED>|the greeting|<RESET><GREEN>|a greeting|<RESET> "hello")
+  ({<RED>}<RESET>(([<RED>]<RESET>(func-n)<RED>[<RESET>]))<RED>{<RESET>})
   (let ((c (<RED>+ a b<RESET><GREEN>add1 first<RESET>)))
     (format "one more than the total is %d" (<RED>add1<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))

index 63b6ac4f87..0e3bab101d 100644
@@ -2,5 +2,6 @@
   ; This is a (moderately) cool function.
   (that\place (+ 3 4))
   (define |a greeting| "hello")
+  ({(([(func-n)]))})
   (let ((c (add1 first)))
     (format "one more than the total is %d" (+ c second))))

index 74b6605357..03d77c7c43 100644
@@ -1,6 +1,7 @@
 (define (myfunc a b)
   ; This is a really cool function.
   (this\place (+ 3 4))
-  (define some-text "hello")
+  (define |the greeting| "hello")
+  ({}(([](func-n)[])){})
   (let ((c (+ a b)))
     (format "one more than the total is %d" (add1 c))))

gitgitgadget

From: "Scott L. Burson" <[email protected]>

Common Lisp has top-level forms, such as 'defun' and 'defmacro', that are not matched by the current Scheme pattern. Also, it is more common in CL, when defining user macros intended as top-level forms, to prefix their names with "def" instead of "define"; such forms are also not matched. And some top-level forms don't even begin with "def".

On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare.

However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives.

---
 Documentation/gitattributes.adoc           |  3 ++-
 t/t4018/scheme-lisp-defun-a                |  4 ++++
 t/t4018/scheme-lisp-defun-b                |  4 ++++
 t/t4018/scheme-lisp-eval-when              |  4 ++++
 t/t4018/{scheme-module => scheme-module-a} |  0
 t/t4018/scheme-module-b                    |  6 ++++++
 t/t4034/scheme/expect                      |  2 +-
 t/t4034/scheme/post                        |  2 +-
 t/t4034/scheme/pre                         |  2 +-
 userdiff.c                                 | 22 ++++++++++++++++------
 10 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 t/t4018/scheme-lisp-defun-a
 create mode 100644 t/t4018/scheme-lisp-defun-b
 create mode 100644 t/t4018/scheme-lisp-eval-when
 rename t/t4018/{scheme-module => scheme-module-a} (100%)
 create mode 100644 t/t4018/scheme-module-b

index f20041a323..bd76167a45 100644
@@ -911,7 +911,8 @@ patterns are available:
 
 - `rust` suitable for source code in the Rust language.
 
-- `scheme` suitable for source code in the Scheme language.
+- `scheme` suitable for source code in most Lisp dialects,
+  including Scheme, Emacs Lisp, Common Lisp, and Clojure.
 
 - `tex` suitable for source code for LaTeX documents.
 

new file mode 100644
index 0000000000..c3c750f76d
@@ -0,0 +1,4 @@
+(defun some-func (x y z) RIGHT
+  (let ((a x)
+        (b y))
+        (ChangeMe a b)))

new file mode 100644
index 0000000000..21be305968
@@ -0,0 +1,4 @@
+(macrolet ((foo (x) `(bar ,x)))
+  (defun mumble (x) ; RIGHT
+    (when (> x 0)
+      (foo x)))) ; ChangeMe

new file mode 100644
index 0000000000..5d941d7e0e
@@ -0,0 +1,4 @@
+(eval-when (:compile-toplevel :load-toplevel :execute)  ; RIGHT
+  (set-macro-character #\?
+		       (lambda (stream char)
+			 `(make-pattern-variable ,(read stream)))))  ; ChangeMe

similarity index 100%
rename from t/t4018/scheme-module
rename to t/t4018/scheme-module-a

new file mode 100644
index 0000000000..77bc0c5eff
@@ -0,0 +1,6 @@
+(module A
+  (export with-display-exception)
+  (extern (display-exception display-exception))
+  (def (with-display-exception thunk) RIGHT
+    (with-catch (lambda (e) (display-exception e (current-error-port)) e)
+      thunk ChangeMe)))

index 138abe9f56..fb7f2616fe 100644
@@ -6,7 +6,7 @@
 (define (<RED>myfunc a b<RESET><GREEN>my-func first second<RESET>)
   ; This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function.
   (<RED>this\place<RESET><GREEN>that\place<RESET> (+ 3 4))
-  (define <RED>|the greeting|<RESET><GREEN>|a greeting|<RESET> "hello")
+  (define <RED>|the \| \greeting|<RESET><GREEN>|a \greeting|<RESET> |hello there|)
   ({<RED>}<RESET>(([<RED>]<RESET>(func-n)<RED>[<RESET>]))<RED>{<RESET>})
   (let ((c (<RED>+ a b<RESET><GREEN>add1 first<RESET>)))
     (format "one more than the total is %d" (<RED>add1<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))

index 0e3bab101d..450cc234f7 100644
@@ -1,7 +1,7 @@
 (define (my-func first second)
   ; This is a (moderately) cool function.
   (that\place (+ 3 4))
-  (define |a greeting| "hello")
+  (define |a \greeting| |hello there|)
   ({(([(func-n)]))})
   (let ((c (add1 first)))
     (format "one more than the total is %d" (+ c second))))

index 03d77c7c43..e16ee75849 100644
@@ -1,7 +1,7 @@
 (define (myfunc a b)
   ; This is a really cool function.
   (this\place (+ 3 4))
-  (define |the greeting| "hello")
+  (define |the \| \greeting| |hello there|)
   ({}(([](func-n)[])){})
   (let ((c (+ a b)))
     (format "one more than the total is %d" (add1 c))))

index fe710a68bf..b5412e6bc3 100644
@@ -344,14 +344,24 @@ PATTERNS("rust",
 	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"
 	 "|[-+*\\/<>%&^|=!:]=|<<=?|>>=?|&&|\\|\\||->|=>|\\.{2}=|\\.{3}|::"),
 PATTERNS("scheme",
-	 "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$",
 	 /*
-	  * R7RS valid identifiers include any sequence enclosed
-	  * within vertical lines having no backslashes
+	  * An unindented opening parenthesis identifies a top-level
+	  * expression in all Lisp dialects.
 	  */
-	 "\\|([^\\\\]*)\\|"
-	 /* All other words should be delimited by spaces or parentheses */
-	 "|([^][)(}{[ \t])+"),
+	 "^(\\(.*)$\n"
+	 /* For Scheme: a possibly indented left paren followed by a keyword. */
+	 "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$\n"
+	 /*
+	  * For all Lisp dialects: a slightly indented line starting with "(def".
+	  */
+	 "^  ?(\\([Dd][Ee][Ff].*)$",
+	 /*
+	  * The union of R7RS and Common Lisp symbol syntax: allows arbitrary
+	  * strings between vertical bars, including any escaped characters.
+	  */
+	 "\\|([^|\\\\]|\\\\.)*\\|"
+	 /* All other words should be delimited by spaces or parentheses. */
+	 "|([^][)(}{ \t])+"),
 PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
 	 "\\\\[a-zA-Z@]+|\\\\.|([a-zA-Z0-9]|[^\x01-\x7f])+"),
 { .name = "default", .binary = -1 },

gitgitgadget

Am 15.04.26 um 04:27 schrieb Scott L. Burson via GitGitGadget:

This implements the conclusion of our discussion from three months ago.

-- Hannes