From: Johannes Sixt <j6t@kdbg•org>
To: "Scott L. Burson" <Scott@sympoiesis•com>
Cc: "Junio C Hamano" <gitster@pobox•com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail•com>,
"Jaydeep P Das" <jaydeepjd.8914@gmail•com>,
"Atharva Raykar" <raykar.ath@gmail•com>,
git@vger•kernel.org,
"Scott L. Burson via GitGitGadget" <gitgitgadget@gmail•com>
Subject: Re: [PATCH] diff: "lisp" userdiff_driver
Date: Sat, 15 Nov 2025 18:06:44 +0100 [thread overview]
Message-ID: <773d3233-c890-4df9-8f7e-32ff8a48651e@kdbg.org> (raw)
In-Reply-To: <pull.2000.git.1763201865025.gitgitgadget@gmail.com>
[Cc the author of the Scheme driver]
Am 15.11.25 um 11:17 schrieb Scott L. Burson via GitGitGadget:
> From: "Scott L. Burson" <Scott@sympoiesis•com>
>
> 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 <Scott@sympoiesis•com>
> ---
> 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
next prev parent reply other threads:[~2025-11-15 17:40 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-15 10:17 [PATCH] diff: "lisp" userdiff_driver Scott L. Burson via GitGitGadget
2025-11-15 17:06 ` Johannes Sixt [this message]
2025-11-15 23:32 ` Scott L. Burson
2025-11-20 16:47 ` D. Ben Knoble
2025-11-27 2:10 ` Scott L. Burson
2025-11-16 5:30 ` Junio C Hamano
2025-11-17 23:23 ` Scott L. Burson
2025-11-18 4:38 ` Junio C Hamano
2025-11-27 2:38 ` [PATCH v2 0/2] userdiff: extend Scheme support to cover other Lisp dialects Scott L. Burson via GitGitGadget
2025-11-27 2:38 ` [PATCH v2 1/2] diff: "lisp" userdiff_driver Scott L. Burson via GitGitGadget
2025-11-27 10:32 ` Scott L. Burson
2025-11-27 10:51 ` Johannes Sixt
2025-11-27 2:38 ` [PATCH v2 2/2] merge with Scheme regexp; fix bugs Scott L. Burson via GitGitGadget
2025-11-27 16:09 ` Johannes Sixt
2025-12-02 10:27 ` Johannes Sixt
2026-01-14 6:18 ` Scott L. Burson
2026-01-14 8:40 ` Johannes Sixt
2026-01-15 23:18 ` [PATCH v3 0/2] userdiff: extend Scheme support to cover other Lisp dialects Scott L. Burson via GitGitGadget
2026-01-15 23:18 ` [PATCH v3 1/2] userdiff: tighten word-diff test case of the scheme driver Johannes Sixt via GitGitGadget
2026-01-15 23:18 ` [PATCH v3 2/2] userdiff: extend Scheme support to cover other Lisp dialects Scott L. Burson via GitGitGadget
2026-01-16 8:49 ` Johannes Sixt
2026-01-17 2:09 ` Scott L. Burson
2026-01-17 8:15 ` Johannes Sixt
2026-04-15 2:27 ` [PATCH v4 0/2] " Scott L. Burson via GitGitGadget
2026-04-15 2:27 ` [PATCH v4 1/2] userdiff: tighten word-diff test case of the scheme driver Johannes Sixt via GitGitGadget
2026-04-15 2:27 ` [PATCH v4 2/2] userdiff: extend Scheme support to cover other Lisp dialects Scott L. Burson via GitGitGadget
2026-04-15 6:54 ` [PATCH v4 0/2] " 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=773d3233-c890-4df9-8f7e-32ff8a48651e@kdbg.org \
--to=j6t@kdbg$(echo .)org \
--cc=Scott@sympoiesis$(echo .)com \
--cc=avarab@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=gitster@pobox$(echo .)com \
--cc=jaydeepjd.8914@gmail$(echo .)com \
--cc=raykar.ath@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox