diff mbox series

userdiff: add support for Emacs Lisp

Message ID 20210213192447.6114-1-git@adamspiers.org (mailing list archive)
State New, archived
Headers show
Series userdiff: add support for Emacs Lisp | expand

Commit Message

Adam Spiers Feb. 13, 2021, 7:24 p.m. UTC
Add a diff driver which recognises Elisp top-level forms and outline
headings for display in hunk headers, and which correctly renders word
diffs.

This approach was previously discussed on the emacs-devel mailing list:

   https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00705.html

* userdiff.c (builtin_drivers): Provide regexen for Elisp top level
  forms and outline headings, and for word diffs.
* t/t4018-diff-funcname.sh (diffpatterns): Add test for elisp driver
* t/t4018/elisp-outline-heading: Test fixture for outline headings
* t/t4018/elisp-top-level-form: Test fixture for top level forms
* t/t4034-diff-words.sh: Add test for elisp driver
* t/t4034/elisp/*: Test fixtures for word diffs

Signed-off-by: Protesilaos Stavrou <info@protesilaos.com>
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
 Documentation/gitattributes.txt | 2 ++
 t/t4018-diff-funcname.sh        | 1 +
 t/t4018/elisp-outline-heading   | 6 ++++++
 t/t4018/elisp-top-level-form    | 7 +++++++
 t/t4034-diff-words.sh           | 1 +
 t/t4034/elisp/expect            | 9 +++++++++
 t/t4034/elisp/post              | 4 ++++
 t/t4034/elisp/pre               | 4 ++++
 userdiff.c                      | 9 +++++++++
 9 files changed, 43 insertions(+)
 create mode 100644 t/t4018/elisp-outline-heading
 create mode 100644 t/t4018/elisp-top-level-form
 create mode 100644 t/t4034/elisp/expect
 create mode 100644 t/t4034/elisp/post
 create mode 100644 t/t4034/elisp/pre

Comments

Ævar Arnfjörð Bjarmason Feb. 14, 2021, 1:41 a.m. UTC | #1
On Sat, Feb 13 2021, Adam Spiers wrote:

> Add a diff driver which recognises Elisp top-level forms and outline
> headings for display in hunk headers, and which correctly renders word
> diffs.

Neat, I for one would find this useful.

> This approach was previously discussed on the emacs-devel mailing list:
>
>    https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00705.html
>
> * userdiff.c (builtin_drivers): Provide regexen for Elisp top level
>   forms and outline headings, and for word diffs.
> * t/t4018-diff-funcname.sh (diffpatterns): Add test for elisp driver
> * t/t4018/elisp-outline-heading: Test fixture for outline headings
> * t/t4018/elisp-top-level-form: Test fixture for top level forms
> * t/t4034-diff-words.sh: Add test for elisp driver
> * t/t4034/elisp/*: Test fixtures for word diffs

Please no on the overly verbose emacs.git convention of per-file
changelogs in commit messages :)

> diff --git a/t/t4018/elisp-outline-heading b/t/t4018/elisp-outline-heading
> new file mode 100644
> index 0000000000..c13bdafafe
> --- /dev/null
> +++ b/t/t4018/elisp-outline-heading
> @@ -0,0 +1,6 @@
> +;;; A top-level outline heading
> +;;;; A second-level outline heading RIGHT
> +
> +;; This is a ChangeMe comment outside top-level forms
> +(defun foo ()
> +  (bar 1 2 3)
> diff --git a/t/t4018/elisp-top-level-form b/t/t4018/elisp-top-level-form
> new file mode 100644
> index 0000000000..683f7ffcf1
> --- /dev/null
> +++ b/t/t4018/elisp-top-level-form
> @@ -0,0 +1,7 @@
> +;;; Outline heading
> +
> +;; This is a comment
> +(RIGHT
> +  (list 1 2 3)
> +  ChangeMe
> +  (list a b c))
> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
> index 0c8fb39ced..a546ee831a 100755
> --- a/t/t4034-diff-words.sh
> +++ b/t/t4034-diff-words.sh
> @@ -315,6 +315,7 @@ test_language_driver cpp
>  test_language_driver csharp
>  test_language_driver css
>  test_language_driver dts
> +test_language_driver elisp
>  test_language_driver fortran
>  test_language_driver html
>  test_language_driver java
> diff --git a/t/t4034/elisp/expect b/t/t4034/elisp/expect
> new file mode 100644
> index 0000000000..29a6ef2520
> --- /dev/null
> +++ b/t/t4034/elisp/expect
> @@ -0,0 +1,9 @@
> +<BOLD>diff --git a/pre b/post<RESET>
> +<BOLD>index 4a39df8..6619e96 100644<RESET>
> +<BOLD>--- a/pre<RESET>
> +<BOLD>+++ b/post<RESET>
> +<CYAN>@@ -1,4 +1,4 @@<RESET>
> +(defun <RED>myfunc<RESET><GREEN>my-func<RESET> (<RED>a b<RESET><GREEN>first second<RESET>)
> +  "This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function."
> +  (let ((c (<RED>+ a b<RESET><GREEN>1+ first<RESET>)))
> +    (format "one more than the total is %d" (<RED>1+<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))
> diff --git a/t/t4034/elisp/post b/t/t4034/elisp/post
> new file mode 100644
> index 0000000000..6619e96657
> --- /dev/null
> +++ b/t/t4034/elisp/post
> @@ -0,0 +1,4 @@
> +(defun my-func (first second)
> +  "This is a (moderately) cool function."
> +  (let ((c (1+ first)))
> +    (format "one more than the total is %d" (+ c second))))
> diff --git a/t/t4034/elisp/pre b/t/t4034/elisp/pre
> new file mode 100644
> index 0000000000..4a39df8ffb
> --- /dev/null
> +++ b/t/t4034/elisp/pre
> @@ -0,0 +1,4 @@
> +(defun myfunc (a b)
> +  "This is a really cool function."
> +  (let ((c (+ a b)))
> +    (format "one more than the total is %d" (1+ c))))
> diff --git a/userdiff.c b/userdiff.c
> index 3f81a2261c..292e51674a 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -53,6 +53,15 @@ PATTERNS("dts",
>  	 /* Property names and math operators */
>  	 "[a-zA-Z0-9,._+?#-]+"
>  	 "|[-+*/%&^|!~]|>>|<<|&&|\\|\\|"),
> +PATTERNS("elisp",
> +	 /* Top level forms and outline headings */
> +	 "^((\\(|;;;+ +).+)",
> +	 /*
> +	  * Emacs Lisp allows symbol names containing any characters.
> +	  * However spaces within the symbol must be escaped.
> +	  */
> +	 "(\\.|[^ ()])+"
> +	 ),
>  PATTERNS("elixir",
>  	 "^[ \t]*((def(macro|module|impl|protocol|p)?|test)[ \t].*)$",
>  	 /* -- */

I think this patch would benefit from first being dogfooded in the
emacs-devel community. There's an existing regex in emacs.git's
autoconf.sh which anyone developing emacs.git is using.

If you run:

    diff -u <(git -c diff.elisp.xfuncname='^\(def[^[:space:]]+[[:space:]]+([^()[:space:]]+)' log -p -- lisp | grep -m 100 @@) \
	    <(git -c diff.elisp.xfuncname='^((\(|;;;+ +).*)$' log -p -- lisp | grep -m 100 @@) \
    | less

You can see how it differs from yours, and that also neatly answers the
question[1] you had in the linked thread about why these patterns tend
to be explicitly scoped to how you define a function/package/whatever in
the language in question.

Just a cursory "git log -p -- lisp" in emacs.git with your patch shows
e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before
it selects the "defun", after with yours it's a "put" in the middle of
the function).

Yours also changes it from e.g.:

    @@ -61,7 +61,7 @@ forward-thing

to:

    @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n)

Is this really desired in elisp? I also note how our tests in
t4018-diff-funcname.sh are really bad in not testing for this at
all. I.e. we just test that we match the right line, not how we extract
a match from it.

1. https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00739.html
Johannes Sixt Feb. 14, 2021, 8:12 a.m. UTC | #2
Am 14.02.21 um 02:41 schrieb Ævar Arnfjörð Bjarmason:
> Just a cursory "git log -p -- lisp" in emacs.git with your patch shows
> e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before
> it selects the "defun", after with yours it's a "put" in the middle of
> the function).

Note that negative matches can be specified. We use the feature in the 
cpp case to exclude public:/protected:/private: and label: that happen 
to be not indented. Perhaps that can be useful here?

Oh, and BTW, what the patterns treat as "function" must not match what 
the language treats as function. The purpose of the hunk header is to 
spot a place in the source file easily. So, it does not hurt if 
eval-and-compile forms are captured (as was mentioned in the linked 
thread) if desired.

> 
> Yours also changes it from e.g.:
> 
>      @@ -61,7 +61,7 @@ forward-thing
> 
> to:
> 
>      @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n)
> 
> Is this really desired in elisp?

It's common practice to extract the entire line (sans indentation if 
applicable), not just the function name. Why would somebody want the 
latter? It doesn't carry as much information as could be possible.

> I also note how our tests in
> t4018-diff-funcname.sh are really bad in not testing for this at
> all. I.e. we just test that we match the right line, not how we extract
> a match from it.

Oh, well. These are "semi-automated" tests cases. If you have an idea 
how to achieve the goal without burdening test writers with lots of 
subtleties, be my guest. ;)

> 
> 1. https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00739.html
> 

-- Hannes
Johannes Sixt Feb. 14, 2021, 11:10 a.m. UTC | #3
Am 14.02.21 um 09:12 schrieb Johannes Sixt:
> Am 14.02.21 um 02:41 schrieb Ævar Arnfjörð Bjarmason:
>> Just a cursory "git log -p -- lisp" in emacs.git with your patch shows
>> e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before
>> it selects the "defun", after with yours it's a "put" in the middle of
>> the function).
> 
> Note that negative matches can be specified. We use the feature in the 
> cpp case to exclude public:/protected:/private: and label: that happen 
> to be not indented. Perhaps that can be useful here?
> 
> Oh, and BTW, what the patterns treat as "function" must not match what 

I meant to say "need not match".

> the language treats as function. The purpose of the hunk header is to 
> spot a place in the source file easily. So, it does not hurt if 
> eval-and-compile forms are captured (as was mentioned in the linked 
> thread) if desired.
> 
>>
>> Yours also changes it from e.g.:
>>
>>      @@ -61,7 +61,7 @@ forward-thing
>>
>> to:
>>
>>      @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n)
>>
>> Is this really desired in elisp?
> 
> It's common practice to extract the entire line (sans indentation if 
> applicable), not just the function name. Why would somebody want the 
> latter? It doesn't carry as much information as could be possible.
> 
>> I also note how our tests in
>> t4018-diff-funcname.sh are really bad in not testing for this at
>> all. I.e. we just test that we match the right line, not how we extract
>> a match from it.
> 
> Oh, well. These are "semi-automated" tests cases. If you have an idea 
> how to achieve the goal without burdening test writers with lots of 
> subtleties, be my guest. ;)
> 
>>
>> 1. https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00739.html
>>
> 
> -- Hannes
Ævar Arnfjörð Bjarmason Feb. 14, 2021, 6:25 p.m. UTC | #4
On Sun, Feb 14 2021, Johannes Sixt wrote:

> Am 14.02.21 um 02:41 schrieb Ævar Arnfjörð Bjarmason:
>> Just a cursory "git log -p -- lisp" in emacs.git with your patch shows
>> e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before
>> it selects the "defun", after with yours it's a "put" in the middle of
>> the function).
>
> Note that negative matches can be specified. We use the feature in the
> cpp case to exclude public:/protected:/private: and label: that happen 
> to be not indented. Perhaps that can be useful here?
>
> Oh, and BTW, what the patterns treat as "function" must not match what
> the language treats as function. The purpose of the hunk header is to 
> spot a place in the source file easily. So, it does not hurt if
> eval-and-compile forms are captured (as was mentioned in the linked 
> thread) if desired.

Right, so having lots of test-case is helpful, e.g. for elisp maybe you
have a top-level defun, maybe not, maybe the top-level is a "(progn" so
you'd like a second-level more meaningful context, or maybe not...

Obviously these userdiff patterns aren't a general parser and will
always be hit-and-miss, it's just useful to at least eyeball them
against in-the-wild test data to check their sanity & add some tests.

My cursory glance at the emacs.git version v.s. what's being submitted
here is that this one does a worse job in *very common* cases.

>> Yours also changes it from e.g.:
>>      @@ -61,7 +61,7 @@ forward-thing
>> to:
>>      @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n)
>> Is this really desired in elisp?
>
> It's common practice to extract the entire line (sans indentation if
> applicable), not just the function name. Why would somebody want the 
> latter? It doesn't carry as much information as could be possible.

Because I'm familiar with the codebase I'm editing and I just need to
know that the diff I'm viewing is on the "main" function, not that it's
"main()", "int main(int argv, char **argv)", "int main(const int argv,
const char *argv[])", or to either have a " {" or not at the end
depending on the project's coding style.

I know our own userdiff builtin patterns don't do this, but it would be
very useful to retrofit this capability / maybe make it a configurable
feature, i.e. have them capture the meaningful part of the line, and you
could either print it all, or just that part.

>> I also note how our tests in
>> t4018-diff-funcname.sh are really bad in not testing for this at
>> all. I.e. we just test that we match the right line, not how we extract
>> a match from it.
>
> Oh, well. These are "semi-automated" tests cases. If you have an idea
> how to achieve the goal without burdening test writers with lots of 
> subtleties, be my guest. ;)

I'll do that soon.

>> 1. https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00739.html
>> 
>
> -- Hannes
Protesilaos Stavrou Feb. 16, 2021, 8:26 a.m. UTC | #5
On 2021-02-14, 19:25 +0100, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> On Sun, Feb 14 2021, Johannes Sixt wrote:
>
>> Am 14.02.21 um 02:41 schrieb Ævar Arnfjörð Bjarmason:
>>> Just a cursory "git log -p -- lisp" in emacs.git with your patch shows
>>> e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before
>>> it selects the "defun", after with yours it's a "put" in the middle of
>>> the function).
>>
>> Note that negative matches can be specified. We use the feature in the
>> cpp case to exclude public:/protected:/private: and label: that happen 
>> to be not indented. Perhaps that can be useful here?
>>
>> Oh, and BTW, what the patterns treat as "function" must not match what
>> the language treats as function. The purpose of the hunk header is to 
>> spot a place in the source file easily. So, it does not hurt if
>> eval-and-compile forms are captured (as was mentioned in the linked 
>> thread) if desired.
>
> Right, so having lots of test-case is helpful, e.g. for elisp maybe you
> have a top-level defun, maybe not, maybe the top-level is a "(progn" so
> you'd like a second-level more meaningful context, or maybe not...
>
> Obviously these userdiff patterns aren't a general parser and will
> always be hit-and-miss, it's just useful to at least eyeball them
> against in-the-wild test data to check their sanity & add some tests.

Our intent with this patch is to rely on a heuristic that works for most
cases.  There will always be some case that is not covered.  This point
was made explicit in the relevant emacs-devel thread.

> My cursory glance at the emacs.git version v.s. what's being submitted
> here is that this one does a worse job in *very common* cases.
>
>>> Yours also changes it from e.g.:
>>>      @@ -61,7 +61,7 @@ forward-thing
>>> to:
>>>      @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n)
>>> Is this really desired in elisp?

I think this is a matter of perspective and indeed a valid reason for
doing this in emacs.git first.

For my part, I find the verbose style more informative, especially when
it captures the previous form instead of the one directly around the
diffed lines.

Perhaps one could add to that Emacs' ability to use the same name for
different things, e.g. a function and a variable, where verbosity of
this sort can help with disambiguation.  Granted, we should not really
on such a heading style to aid us in that task, though it may be
something to consider.

>> It's common practice to extract the entire line (sans indentation if
>> applicable), not just the function name. Why would somebody want the 
>> latter? It doesn't carry as much information as could be possible.
>
> Because I'm familiar with the codebase I'm editing and I just need to
> know that the diff I'm viewing is on the "main" function, not that it's
> "main()", "int main(int argv, char **argv)", "int main(const int argv,
> const char *argv[])", or to either have a " {" or not at the end
> depending on the project's coding style.

Oftentimes we produce/read patches for projects that we are not
necessarily acquainted with.  This includes emacs.git and the multitude
of Elisp packages in that milieu.  An extra element of contextuality can
do us good.

It is true that familiarity with the code base will always benefit from
succinct headings, though I feel that whenever we are in doubt we should
err on the side of caution: which means that we must not introduce such
an assumption to the workings of this piece of functionality.

> I know our own userdiff builtin patterns don't do this, but it would be
> very useful to retrofit this capability / maybe make it a configurable
> feature, i.e. have them capture the meaningful part of the line, and you
> could either print it all, or just that part.

It would be nice to have an option that toggles verbosity.  Though I
guess this lies outside the scope of the patch in question.

In conclusion, I think we should decide on the next step forward: if you
think this should be applied to emacs.git before making its way to git
itself, then we can move the discussion there.

Thanks for your time and efforts!
Protesilaos or "Prot"
Ævar Arnfjörð Bjarmason Feb. 16, 2021, 11:13 a.m. UTC | #6
On Tue, Feb 16 2021, Protesilaos Stavrou wrote:

> On 2021-02-14, 19:25 +0100, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
>> On Sun, Feb 14 2021, Johannes Sixt wrote:
>>
>>> Am 14.02.21 um 02:41 schrieb Ævar Arnfjörð Bjarmason:
>>>> Just a cursory "git log -p -- lisp" in emacs.git with your patch shows
>>>> e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before
>>>> it selects the "defun", after with yours it's a "put" in the middle of
>>>> the function).
>>>
>>> Note that negative matches can be specified. We use the feature in the
>>> cpp case to exclude public:/protected:/private: and label: that happen 
>>> to be not indented. Perhaps that can be useful here?
>>>
>>> Oh, and BTW, what the patterns treat as "function" must not match what
>>> the language treats as function. The purpose of the hunk header is to 
>>> spot a place in the source file easily. So, it does not hurt if
>>> eval-and-compile forms are captured (as was mentioned in the linked 
>>> thread) if desired.
>>
>> Right, so having lots of test-case is helpful, e.g. for elisp maybe you
>> have a top-level defun, maybe not, maybe the top-level is a "(progn" so
>> you'd like a second-level more meaningful context, or maybe not...
>>
>> Obviously these userdiff patterns aren't a general parser and will
>> always be hit-and-miss, it's just useful to at least eyeball them
>> against in-the-wild test data to check their sanity & add some tests.
>
> Our intent with this patch is to rely on a heuristic that works for most
> cases.  There will always be some case that is not covered.  This point
> was made explicit in the relevant emacs-devel thread.

Yes these heuristics will always be bad in some cases. I'm just
encouraging you to find some of the more common cases, make test-cases
out of them, to say "this is what I want".

The most common difference I've seen is that by finding comments it
means for changes like this:
    
    (def foo ()
         "..."
         (some-code)
    ;; comments inside functions are often not indented
       (blah))
    
    (def bar ()
         "..."
         CHANGE_ME)

Before we'd find "foo" as the context, now we find ";; comments inside
functions are often not indented".

There's other cases where that comment rule does the right
thing. E.g. finding the description of the file at the top, but we could
also capture that with:

    ^(;;; [^.]+\.el ---.*)'

Then you'd get things like:

    ;;; button.el --- clickable buttons

Without overmatching any and all comments. Or maybe it isn't
overmatching and is better in the common case. I don't know.

Another example is now we'll match "(progn", sometimes that's "right",
sometimes "wrong". Perhaps even in cases where that's "right" the common
"(progn" pattern doesn't provide much/any extra information, and we'd be
better to skip past it with a negative rule to the next "(def" ?

I'm typing this in mu4e, but I wouldn't call myself deeply familiar with
Elisp. My reading of the linked thread / blog posts it links to is that
some other people came up with more narrow matching rules, presumably
because they found some of these cases.

So all I'm suggesting is maybe pro-actively find those cases & loop
those people in.

>> My cursory glance at the emacs.git version v.s. what's being submitted
>> here is that this one does a worse job in *very common* cases.
>>
>>>> Yours also changes it from e.g.:
>>>>      @@ -61,7 +61,7 @@ forward-thing
>>>> to:
>>>>      @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n)
>>>> Is this really desired in elisp?
>
> I think this is a matter of perspective and indeed a valid reason for
> doing this in emacs.git first.
>
> For my part, I find the verbose style more informative, especially when
> it captures the previous form instead of the one directly around the
> diffed lines.
>
> Perhaps one could add to that Emacs' ability to use the same name for
> different things, e.g. a function and a variable, where verbosity of
> this sort can help with disambiguation.  Granted, we should not really
> on such a heading style to aid us in that task, though it may be
> something to consider.

I'm not saying it needs to be done in emacs.git first, just maybe
coordinate with people who wrote/use that rule.

I think matching the whole line makes sense (sans leading whitespace),
we do it for the rest of the git.git rules.

>>> It's common practice to extract the entire line (sans indentation if
>>> applicable), not just the function name. Why would somebody want the 
>>> latter? It doesn't carry as much information as could be possible.
>>
>> Because I'm familiar with the codebase I'm editing and I just need to
>> know that the diff I'm viewing is on the "main" function, not that it's
>> "main()", "int main(int argv, char **argv)", "int main(const int argv,
>> const char *argv[])", or to either have a " {" or not at the end
>> depending on the project's coding style.
>
> Oftentimes we produce/read patches for projects that we are not
> necessarily acquainted with.  This includes emacs.git and the multitude
> of Elisp packages in that milieu.  An extra element of contextuality can
> do us good.
>
> It is true that familiarity with the code base will always benefit from
> succinct headings, though I feel that whenever we are in doubt we should
> err on the side of caution: which means that we must not introduce such
> an assumption to the workings of this piece of functionality.
>
>> I know our own userdiff builtin patterns don't do this, but it would be
>> very useful to retrofit this capability / maybe make it a configurable
>> feature, i.e. have them capture the meaningful part of the line, and you
>> could either print it all, or just that part.
>
> It would be nice to have an option that toggles verbosity.  Though I
> guess this lies outside the scope of the patch in question.

Yes I agree that this is completely outside the scope of adding elisp
support to userdiff.

I was just replying generally to Johaness on the topic of why someone
would want a pattern to match $1 over $0, as most of our git.git
patterns do (or if it's $1, it's the whole line anyway, sans leading
whitespace).

> In conclusion, I think we should decide on the next step forward: if you
> think this should be applied to emacs.git before making its way to git
> itself, then we can move the discussion there.

I think just applying a version to this to git.git is fine, and it
should not wait for whatever 20-some patches test mechanism rewrite I
started. I can rebase on top of this.

B.t.w. emacs.git also has a rule for *.texi, perhaps you'd be interested
in hacking that up too? :)
diff mbox series

Patch

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e84e104f93..0026055f99 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -815,6 +815,8 @@  patterns are available:
 
 - `dts` suitable for devicetree (DTS) files.
 
+- `elisp` suitable for source code in the Emacs Lisp language.
+
 - `elixir` suitable for source code in the Elixir language.
 
 - `fortran` suitable for source code in the Fortran language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 9675bc17db..a9ea2d3cd5 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -33,6 +33,7 @@  diffpatterns="
 	csharp
 	css
 	dts
+	elisp
 	elixir
 	fortran
 	fountain
diff --git a/t/t4018/elisp-outline-heading b/t/t4018/elisp-outline-heading
new file mode 100644
index 0000000000..c13bdafafe
--- /dev/null
+++ b/t/t4018/elisp-outline-heading
@@ -0,0 +1,6 @@ 
+;;; A top-level outline heading
+;;;; A second-level outline heading RIGHT
+
+;; This is a ChangeMe comment outside top-level forms
+(defun foo ()
+  (bar 1 2 3)
diff --git a/t/t4018/elisp-top-level-form b/t/t4018/elisp-top-level-form
new file mode 100644
index 0000000000..683f7ffcf1
--- /dev/null
+++ b/t/t4018/elisp-top-level-form
@@ -0,0 +1,7 @@ 
+;;; Outline heading
+
+;; This is a comment
+(RIGHT
+  (list 1 2 3)
+  ChangeMe
+  (list a b c))
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 0c8fb39ced..a546ee831a 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -315,6 +315,7 @@  test_language_driver cpp
 test_language_driver csharp
 test_language_driver css
 test_language_driver dts
+test_language_driver elisp
 test_language_driver fortran
 test_language_driver html
 test_language_driver java
diff --git a/t/t4034/elisp/expect b/t/t4034/elisp/expect
new file mode 100644
index 0000000000..29a6ef2520
--- /dev/null
+++ b/t/t4034/elisp/expect
@@ -0,0 +1,9 @@ 
+<BOLD>diff --git a/pre b/post<RESET>
+<BOLD>index 4a39df8..6619e96 100644<RESET>
+<BOLD>--- a/pre<RESET>
+<BOLD>+++ b/post<RESET>
+<CYAN>@@ -1,4 +1,4 @@<RESET>
+(defun <RED>myfunc<RESET><GREEN>my-func<RESET> (<RED>a b<RESET><GREEN>first second<RESET>)
+  "This is a <RED>really<RESET><GREEN>(moderately)<RESET> cool function."
+  (let ((c (<RED>+ a b<RESET><GREEN>1+ first<RESET>)))
+    (format "one more than the total is %d" (<RED>1+<RESET><GREEN>+<RESET> c <GREEN>second<RESET>))))
diff --git a/t/t4034/elisp/post b/t/t4034/elisp/post
new file mode 100644
index 0000000000..6619e96657
--- /dev/null
+++ b/t/t4034/elisp/post
@@ -0,0 +1,4 @@ 
+(defun my-func (first second)
+  "This is a (moderately) cool function."
+  (let ((c (1+ first)))
+    (format "one more than the total is %d" (+ c second))))
diff --git a/t/t4034/elisp/pre b/t/t4034/elisp/pre
new file mode 100644
index 0000000000..4a39df8ffb
--- /dev/null
+++ b/t/t4034/elisp/pre
@@ -0,0 +1,4 @@ 
+(defun myfunc (a b)
+  "This is a really cool function."
+  (let ((c (+ a b)))
+    (format "one more than the total is %d" (1+ c))))
diff --git a/userdiff.c b/userdiff.c
index 3f81a2261c..292e51674a 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -53,6 +53,15 @@  PATTERNS("dts",
 	 /* Property names and math operators */
 	 "[a-zA-Z0-9,._+?#-]+"
 	 "|[-+*/%&^|!~]|>>|<<|&&|\\|\\|"),
+PATTERNS("elisp",
+	 /* Top level forms and outline headings */
+	 "^((\\(|;;;+ +).+)",
+	 /*
+	  * Emacs Lisp allows symbol names containing any characters.
+	  * However spaces within the symbol must be escaped.
+	  */
+	 "(\\.|[^ ()])+"
+	 ),
 PATTERNS("elixir",
 	 "^[ \t]*((def(macro|module|impl|protocol|p)?|test)[ \t].*)$",
 	 /* -- */