GNU bug report logs - #47879
[PATCH] Ignore mode hooks when analysing diffs

Previous Next

Package: emacs;

Reported by: Philip Kaludercic <philipk <at> posteo.net>

Date: Sun, 18 Apr 2021 19:09:02 UTC

Severity: normal

Tags: patch

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47879 in the body.
You can then email your comments to 47879 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Sun, 18 Apr 2021 19:09:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Philip Kaludercic <philipk <at> posteo.net>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 18 Apr 2021 19:09:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Ignore mode hooks when analysing diffs
Date: Sun, 18 Apr 2021 19:08:01 +0000
[Message part 1 (text/plain, inline)]
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.

[0003-Avoid-calling-mode-hook-when-analysing-diff.patch (text/x-diff, inline)]
From 6ce025a1c61912ee20083271e821878f049ac801 Mon Sep 17 00:00:00 2001
From: Philip K <philipk <at> 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


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Sun, 18 Apr 2021 19:26:01 GMT) Full text and rfc822 format available.

Message #8 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Sun, 18 Apr 2021 22:25:11 +0300
> From: Philip Kaludercic <philipk <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Sun, 18 Apr 2021 20:09:02 GMT) Full text and rfc822 format available.

Message #11 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Sun, 18 Apr 2021 20:08:35 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Sun, 18 Apr 2021 20:19:02 GMT) Full text and rfc822 format available.

Message #14 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Sun, 18 Apr 2021 22:18:29 +0200
Am So., 18. Apr. 2021 um 21:09 Uhr schrieb Philip Kaludercic
<philipk <at> 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).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Wed, 21 Apr 2021 14:13:01 GMT) Full text and rfc822 format available.

Message #17 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Wed, 21 Apr 2021 09:12:45 -0500
Philip Kaludercic <philipk <at> posteo.net> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Philip Kaludercic <philipk <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Wed, 21 Apr 2021 14:25:01 GMT) Full text and rfc822 format available.

Message #20 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: philipk <at> posteo.net, 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Wed, 21 Apr 2021 17:23:37 +0300
> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Wed, 21 Apr 2021 09:12:45 -0500
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 47879 <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Wed, 21 Apr 2021 14:37:01 GMT) Full text and rfc822 format available.

Message #23 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 47879 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Wed, 21 Apr 2021 14:36:22 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefan <at> marxist.se>
>> Date: Wed, 21 Apr 2021 09:12:45 -0500
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 47879 <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Wed, 21 Apr 2021 14:47:02 GMT) Full text and rfc822 format available.

Message #26 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Wed, 21 Apr 2021 09:45:52 -0500
Philip Kaludercic <philipk <at> posteo.net> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Stefan Kangas <stefan <at> marxist.se>
>>> Date: Wed, 21 Apr 2021 09:12:45 -0500
>>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 47879 <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Wed, 21 Apr 2021 14:53:02 GMT) Full text and rfc822 format available.

Message #29 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Wed, 21 Apr 2021 14:52:32 +0000
Stefan Kangas <stefan <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47879; Package emacs. (Mon, 03 May 2021 10:29:02 GMT) Full text and rfc822 format available.

Message #32 received at 47879 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefan <at> marxist.se>,
 47879 <at> debbugs.gnu.org
Subject: Re: bug#47879: [PATCH] Ignore mode hooks when analysing diffs
Date: Mon, 03 May 2021 12:28:20 +0200
Philip Kaludercic <philipk <at> 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 closed, send any further explanations to 47879 <at> debbugs.gnu.org and Philip Kaludercic <philipk <at> posteo.net> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Mon, 03 May 2021 10:29:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 31 May 2021 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 302 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.