unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
@ 2021-04-18 19:08 Philip Kaludercic
  2021-04-18 19:25 ` Eli Zaretskii
  2021-04-18 20:18 ` Philipp Stephani
  0 siblings, 2 replies; 10+ messages in thread
From: Philip Kaludercic @ 2021-04-18 19:08 UTC (permalink / raw)
  To: 47879

[-- 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-18 19:08 bug#47879: [PATCH] Ignore mode hooks when analysing diffs Philip Kaludercic
@ 2021-04-18 19:25 ` Eli Zaretskii
  2021-04-18 20:08   ` Philip Kaludercic
  2021-04-18 20:18 ` Philipp Stephani
  1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-04-18 19:25 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 47879

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-18 19:25 ` Eli Zaretskii
@ 2021-04-18 20:08   ` Philip Kaludercic
  2021-04-21 14:12     ` Stefan Kangas
  0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2021-04-18 20:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47879

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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-18 19:08 bug#47879: [PATCH] Ignore mode hooks when analysing diffs Philip Kaludercic
  2021-04-18 19:25 ` Eli Zaretskii
@ 2021-04-18 20:18 ` Philipp Stephani
  1 sibling, 0 replies; 10+ messages in thread
From: Philipp Stephani @ 2021-04-18 20:18 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 47879

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-18 20:08   ` Philip Kaludercic
@ 2021-04-21 14:12     ` Stefan Kangas
  2021-04-21 14:23       ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Kangas @ 2021-04-21 14:12 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 47879

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?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-21 14:12     ` Stefan Kangas
@ 2021-04-21 14:23       ` Eli Zaretskii
  2021-04-21 14:36         ` Philip Kaludercic
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-04-21 14:23 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: philipk, 47879

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-21 14:23       ` Eli Zaretskii
@ 2021-04-21 14:36         ` Philip Kaludercic
  2021-04-21 14:45           ` Stefan Kangas
  0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2021-04-21 14:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47879, Stefan Kangas

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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-21 14:36         ` Philip Kaludercic
@ 2021-04-21 14:45           ` Stefan Kangas
  2021-04-21 14:52             ` Philip Kaludercic
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Kangas @ 2021-04-21 14:45 UTC (permalink / raw)
  To: Philip Kaludercic, Eli Zaretskii; +Cc: 47879

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?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-21 14:45           ` Stefan Kangas
@ 2021-04-21 14:52             ` Philip Kaludercic
  2021-05-03 10:28               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2021-04-21 14:52 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 47879

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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#47879: [PATCH] Ignore mode hooks when analysing diffs
  2021-04-21 14:52             ` Philip Kaludercic
@ 2021-05-03 10:28               ` Lars Ingebrigtsen
  0 siblings, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-03 10:28 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Stefan Kangas, 47879

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-05-03 10:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-18 19:08 bug#47879: [PATCH] Ignore mode hooks when analysing diffs Philip Kaludercic
2021-04-18 19:25 ` Eli Zaretskii
2021-04-18 20:08   ` Philip Kaludercic
2021-04-21 14:12     ` Stefan Kangas
2021-04-21 14:23       ` Eli Zaretskii
2021-04-21 14:36         ` Philip Kaludercic
2021-04-21 14:45           ` Stefan Kangas
2021-04-21 14:52             ` Philip Kaludercic
2021-05-03 10:28               ` Lars Ingebrigtsen
2021-04-18 20:18 ` Philipp Stephani

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).