Subject: Re: [PATCH] diff: "lisp" userdiff_driver

From: Johannes Sixt <[email-masked]>
Date: Sat, 15 Nov 2025 18:06:44 +0100

[Cc the author of the Scheme driver]

Am 15.11.25 um 11:17 schrieb Scott L. Burson via GitGitGadget:
> From: "Scott L. Burson" <[email-masked]>
> 
> 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.

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

> 
> Signed-off-by: Scott L. Burson <[email-masked]>
> ---
>     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(+)
> 
> diff --git a/userdiff.c b/userdiff.c
> index fe710a68bf..e127b4a1f1 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -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).*)$",

Compared to the Scheme driver, this regular expression is

- more restrictive because it does not permit arbitrary indentation;

- less restrictive because it permits everything that begins with "(def".

What would happen if this regular expression were added to the Scheme
driver? Would it pick up additional and unwanted hunk headers is typical
Scheme code?

Just so you know: The string literal for hunk headers can contain "\n"
to separate regular expressions. For a given line, they are attempted to
match in order until the first match is found.

> +	 /* Common Lisp symbol syntax allows arbitrary strings between vertical bars */
> +	 "\\|([^\\\\]|\\\\\\\\|\\\\\\|)*\\|"

The Scheme driver has an similar description of this word token, but it
has only half as many backslashes. Is the difference necessary? Isn't
actually one or the other incorrect? (I did not try to understand what
this version here does.)

> +	 /* All other words are delimited by spaces or parentheses/brackets/braces */
> +	 "|([^][(){} \t])+"),

This one looks identical to the same line in Scheme with a slightly
altered comment.

In summary, I feel like this could be unified with the Scheme driver,
except when awkward false positive hunk headers in existing Scheme code
are to be expected. That's a question that I cannot answer.

-- Hannes