bug#47879: [PATCH] Ignore mode hooks when analysing diffs

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]


This fixes a bug related to log-edit-generate-changelog-from-diff.

When I tried to invoke this command in a commit buffer, the error

     signal: Unmatched bracket or quote

was raised. The reason was that I added paredit-mode to
emacs-lisp-mode-hook, that when invoked calls check-parens. This raises
an error when the diff contains a partial s-expression, which is almost
always the case.

The least invasive approach I could come up with was to defer the mode
hooks. Most hooks should not have any effect on what
diff-add-log-current-defuns is doing, but I'm not sure if there are any
exceptions that I didn't think of.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Avoid-calling-mode-hook-when-analysing-diff.patch --]
[-- Type: text/x-diff, Size: 1083 bytes --]

From 6ce025a1c61912ee20083271e821878f049ac801 Mon Sep 17 00:00:00 2001
From: Philip K <philipk@posteo.net>
Date: Sun, 18 Apr 2021 20:40:16 +0200
Subject: [PATCH 3/4] Avoid calling mode hook when analysing diff

Author:
---
 lisp/vc/diff-mode.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 2c72c45f4b..0b77ed11bc 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2334,7 +2334,8 @@ diff-add-log-current-defuns
                         (if other-buf (set-buffer other-buf)
                           (set-buffer (generate-new-buffer " *diff-other-text*"))
                           (insert (if applied old-text new-text))
-                          (funcall (buffer-local-value 'major-mode buf))
+                          (delay-mode-vhooks
+                            (funcall (buffer-local-value 'major-mode buf)))
                           (setq other-buf (current-buffer)))
                         (goto-char (point-min))
                         (forward-line (+ =lines -1
-- 
2.30.2

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Date: Sun, 18 Apr 2021 19:08:01 +0000
> 
> This fixes a bug related to log-edit-generate-changelog-from-diff.
> 
> When I tried to invoke this command in a commit buffer, the error
> 
>      signal: Unmatched bracket or quote
> 
> was raised. The reason was that I added paredit-mode to
> emacs-lisp-mode-hook, that when invoked calls check-parens. This raises
> an error when the diff contains a partial s-expression, which is almost
> always the case.
> 
> The least invasive approach I could come up with was to defer the mode
> hooks.

I rather think that the bug is either in the hook you added or in
paredit-mode, and should be solved there.  Otherwise, we'd need to
defer hooks every time we turn on a mode.  And who known what
deferring all the hooks could do to other hooks, which are entirely
legitimate and don't cause any problems.

IOW, I think what you suggest is too blunt a weapon, and could do more
harm than help.  Let's try to find a cleaner way of fixing the
problem, preferably where it happens, i.e. in the code that is run by
the hook.

Thanks.

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Sun, 18 Apr 2021 19:08:01 +0000
>> 
>> This fixes a bug related to log-edit-generate-changelog-from-diff.
>> 
>> When I tried to invoke this command in a commit buffer, the error
>> 
>>      signal: Unmatched bracket or quote
>> 
>> was raised. The reason was that I added paredit-mode to
>> emacs-lisp-mode-hook, that when invoked calls check-parens. This raises
>> an error when the diff contains a partial s-expression, which is almost
>> always the case.
>> 
>> The least invasive approach I could come up with was to defer the mode
>> hooks.
>
> I rather think that the bug is either in the hook you added or in
> paredit-mode, and should be solved there.

Ok, I'll try that too.

-- 
	Philip K.

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Stefan Kangas]

Philip Kaludercic <philipk@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Philip Kaludercic <philipk@posteo.net>
>>> Date: Sun, 18 Apr 2021 19:08:01 +0000
>>>
>>> This fixes a bug related to log-edit-generate-changelog-from-diff.
>>>
>>> When I tried to invoke this command in a commit buffer, the error
>>>
>>>      signal: Unmatched bracket or quote
>>>
>>> was raised. The reason was that I added paredit-mode to
>>> emacs-lisp-mode-hook, that when invoked calls check-parens. This raises
>>> an error when the diff contains a partial s-expression, which is almost
>>> always the case.
>>>
>>> The least invasive approach I could come up with was to defer the mode
>>> hooks.
>>
>> I rather think that the bug is either in the hook you added or in
>> paredit-mode, and should be solved there.
>
> Ok, I'll try that too.

So is this a notabug or a wontfix, or is there more to do here?

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 21 Apr 2021 09:12:45 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 47879@debbugs.gnu.org
> 
> >> I rather think that the bug is either in the hook you added or in
> >> paredit-mode, and should be solved there.
> >
> > Ok, I'll try that too.
> 
> So is this a notabug or a wontfix, or is there more to do here?

Too early to say, I think.  Let's give Philip some time to try to fix
it as I suggested.

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Wed, 21 Apr 2021 09:12:45 -0500
>> Cc: Eli Zaretskii <eliz@gnu.org>, 47879@debbugs.gnu.org
>> 
>> >> I rather think that the bug is either in the hook you added or in
>> >> paredit-mode, and should be solved there.
>> >
>> > Ok, I'll try that too.
>> 
>> So is this a notabug or a wontfix, or is there more to do here?
>
> Too early to say, I think.  Let's give Philip some time to try to fix
> it as I suggested.

I looked into the code, and Philipp Stephani's suggestion to set
paredit-override-check-parens-function to (lambda (_) t) seems to be the
cleanest solution. I'd say this is not really a bug, but an issue in my
configuration. It might be worth considering sending paredit a patch to
only check the syntax if invoked interactively, but my suggestion was
certainly too broad.

-- 
	Philip K.

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Stefan Kangas]

Philip Kaludercic <philipk@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Stefan Kangas <stefan@marxist.se>
>>> Date: Wed, 21 Apr 2021 09:12:45 -0500
>>> Cc: Eli Zaretskii <eliz@gnu.org>, 47879@debbugs.gnu.org
>>>
>>> >> I rather think that the bug is either in the hook you added or in
>>> >> paredit-mode, and should be solved there.
>>> >
>>> > Ok, I'll try that too.
>>>
>>> So is this a notabug or a wontfix, or is there more to do here?
>>
>> Too early to say, I think.  Let's give Philip some time to try to fix
>> it as I suggested.
>
> I looked into the code, and Philipp Stephani's suggestion to set
> paredit-override-check-parens-function to (lambda (_) t) seems to be the
> cleanest solution. I'd say this is not really a bug, but an issue in my
> configuration. It might be worth considering sending paredit a patch to
> only check the syntax if invoked interactively, but my suggestion was
> certainly too broad.

Thanks.  Should this bug remain open, or can it therefore be closed?

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Philip Kaludercic]

Stefan Kangas <stefan@marxist.se> writes:

> Thanks.  Should this bug remain open, or can it therefore be closed?

I would close it, because the issue is not tied to Emacs itself.

-- 
	Philip K.

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> I would close it, because the issue is not tied to Emacs itself.

OK; closing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47879: [PATCH] Ignore mode hooks when analysing diffs

[Philipp Stephani]

Am So., 18. Apr. 2021 um 21:09 Uhr schrieb Philip Kaludercic
<philipk@posteo.net>:
>
>
> This fixes a bug related to log-edit-generate-changelog-from-diff.
>
> When I tried to invoke this command in a commit buffer, the error
>
>      signal: Unmatched bracket or quote
>
> was raised. The reason was that I added paredit-mode to
> emacs-lisp-mode-hook, that when invoked calls check-parens. This raises
> an error when the diff contains a partial s-expression, which is almost
> always the case.

I have the following in my emacs-lisp-mode-hook:
   (with-demoted-errors "Error enabling Paredit: %s" (paredit-mode))
Enabling Paredit unconditionally is too invasive; it will also break
file visiting if the file has a syntax error.
Alternatively, bind paredit-override-check-parens-function to e.g.
(lambda (_) t).