GNU bug report logs - #33975
[PATCH] inhibit read-only text properties in comint-interrupt-subjob

Previous Next

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

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

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

From: Alex Branham <alex.branham <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] inhibit read-only text properties in comint-interrupt-subjob
Date: Fri, 04 Jan 2019 10:35:05 -0600

[Message part 1 (text/plain, inline)]

Hi -

This patch inhibits read-only properties during comint-interrupt-subjob.
I ran across this while using ESS and (setq comint-prompt-read-only t).
There's a little more info (including a reproducible example) on ESS's
bugtracker.[1]

Thanks,
Alex

From 8e3885c5b9747987cacd3b17b9de29975e7691e3 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham <at> gmail.com>
Date: Fri, 4 Jan 2019 10:28:09 -0600
Subject: [PATCH] * lisp/comint.el (comint-interrupt-subjob): Inhibit read only

Otherwise with comint-prompt-read-only set to t users can be incapable
of interrupting running busy processes. See ESS's issue tracker for
details: https://github.com/emacs-ess/ESS/issues/792
---
 lisp/comint.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 0a6aff2e73..2ed65c1c1c 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -2500,8 +2500,9 @@ comint-clear-buffer
 (defun comint-interrupt-subjob ()
   "Interrupt the current subjob."
   (interactive)
-  (comint-skip-input)
-  (interrupt-process nil comint-ptyp)
+  (let ((inhibit-read-only t))
+    (comint-skip-input)
+    (interrupt-process nil comint-ptyp))
   ;; (process-send-string nil "\n")
   )

--
2.19.2

[0001-lisp-comint.el-comint-interrupt-subjob-Inhibit-read-.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

Footnotes:
[1]  https://github.com/emacs-ess/ESS/issues/792

[signature.asc (application/pgp-signature, inline)]

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

From: Alex Branham <alex.branham <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Thu, 21 Feb 2019 10:32:16 -0600

Hi all - 

Is this patch OK for Emacs? Or should we figure it out downstream?

Thanks,
Alex

On Fri 04 Jan 2019 at 10:35, Alex Branham <alex.branham <at> gmail.com> wrote:

> Hi -
>
> This patch inhibits read-only properties during comint-interrupt-subjob.
> I ran across this while using ESS and (setq comint-prompt-read-only t).
> There's a little more info (including a reproducible example) on ESS's
> bugtracker.[1]
>
> Thanks,
> Alex
>
> From 8e3885c5b9747987cacd3b17b9de29975e7691e3 Mon Sep 17 00:00:00 2001
> From: Alex Branham <alex.branham <at> gmail.com>
> Date: Fri, 4 Jan 2019 10:28:09 -0600
> Subject: [PATCH] * lisp/comint.el (comint-interrupt-subjob): Inhibit read only
>
> Otherwise with comint-prompt-read-only set to t users can be incapable
> of interrupting running busy processes. See ESS's issue tracker for
> details: https://github.com/emacs-ess/ESS/issues/792
> ---
>  lisp/comint.el | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/comint.el b/lisp/comint.el
> index 0a6aff2e73..2ed65c1c1c 100644
> --- a/lisp/comint.el
> +++ b/lisp/comint.el
> @@ -2500,8 +2500,9 @@ comint-clear-buffer
>  (defun comint-interrupt-subjob ()
>    "Interrupt the current subjob."
>    (interactive)
> -  (comint-skip-input)
> -  (interrupt-process nil comint-ptyp)
> +  (let ((inhibit-read-only t))
> +    (comint-skip-input)
> +    (interrupt-process nil comint-ptyp))
>    ;; (process-send-string nil "\n")
>    )
>
> --
> 2.19.2
>
>
> From 8e3885c5b9747987cacd3b17b9de29975e7691e3 Mon Sep 17 00:00:00 2001
> From: Alex Branham <alex.branham <at> gmail.com>
> Date: Fri, 4 Jan 2019 10:28:09 -0600
> Subject: [PATCH] * lisp/comint.el (comint-interrupt-subjob): Inhibit read only
>
> Otherwise with comint-prompt-read-only set to t users can be incapable
> of interrupting running busy processes. See ESS's issue tracker for
> details: https://github.com/emacs-ess/ESS/issues/792
> ---
>  lisp/comint.el | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/comint.el b/lisp/comint.el
> index 0a6aff2e73..2ed65c1c1c 100644
> --- a/lisp/comint.el
> +++ b/lisp/comint.el
> @@ -2500,8 +2500,9 @@ comint-clear-buffer
>  (defun comint-interrupt-subjob ()
>    "Interrupt the current subjob."
>    (interactive)
> -  (comint-skip-input)
> -  (interrupt-process nil comint-ptyp)
> +  (let ((inhibit-read-only t))
> +    (comint-skip-input)
> +    (interrupt-process nil comint-ptyp))
>    ;; (process-send-string nil "\n")
>    )

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 33975 <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Fri, 22 Feb 2019 10:37:00 +0200

> From: Alex Branham <alex.branham <at> gmail.com>
> Date: Thu, 21 Feb 2019 10:32:16 -0600
> 
> Is this patch OK for Emacs? Or should we figure it out downstream?

Can you explain how having comint-prompt-read-only interferes with
interrupt-process in this case?  I don't think I understand the
connection, and neither this bug report nor the ESS issue do, AFAICT.

Thanks.

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

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33975 <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Fri, 22 Feb 2019 15:26:00 -0600

[Message part 1 (text/plain, inline)]

On Fri 22 Feb 2019 at 02:37, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Alex Branham <alex.branham <at> gmail.com>
>> Date: Thu, 21 Feb 2019 10:32:16 -0600
>>
>> Is this patch OK for Emacs? Or should we figure it out downstream?
>
> Can you explain how having comint-prompt-read-only interferes with
> interrupt-process in this case?  I don't think I understand the
> connection, and neither this bug report nor the ESS issue do, AFAICT.

Sure thing. `comint-interrupt-subjob' calls `comint-skip-input', which
tries to do this:

(insert "  " (key-description (this-command-keys)))

Which fails if the text at point is read only.

Thanks,
Alex

[signature.asc (application/pgp-signature, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 33975 <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Sat, 23 Feb 2019 10:51:28 +0200

> From: Alex Branham <alex.branham <at> gmail.com>
> Cc: 33975 <at> debbugs.gnu.org
> Date: Fri, 22 Feb 2019 15:26:00 -0600
> 
> > Can you explain how having comint-prompt-read-only interferes with
> > interrupt-process in this case?  I don't think I understand the
> > connection, and neither this bug report nor the ESS issue do, AFAICT.
> 
> Sure thing. `comint-interrupt-subjob' calls `comint-skip-input', which
> tries to do this:
> 
> (insert "  " (key-description (this-command-keys)))
> 
> Which fails if the text at point is read only.

Then shouldn't the change be inside comint-skip-input instead?  I
mean, the same problem will happen also in all other callers of
comint-skip-input, no?

(I'm not sure I understand why that function inserts the description
of this-command-keys -- is that to insert "C-c C-c" into the buffer?
This is not in the doc string, perhaps we should add that.)

Thanks.

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

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33975 <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Sat, 23 Feb 2019 07:36:43 -0600

[Message part 1 (text/plain, inline)]

On Sat 23 Feb 2019 at 02:51, Eli Zaretskii <eliz <at> gnu.org> wrote:

> Then shouldn't the change be inside comint-skip-input instead?  I
> mean, the same problem will happen also in all other callers of
> comint-skip-input, no?

Probably, yes. Changed in the attached patch.

> (I'm not sure I understand why that function inserts the description
> of this-command-keys -- is that to insert "C-c C-c" into the buffer?
> This is not in the doc string, perhaps we should add that.)

The attached patch adds it. WDYT?

From 0eba90f40c4ffdd020b86ffa2e19815c0edfe2f0 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham <at> gmail.com>
Date: Sat, 23 Feb 2019 07:35:01 -0600
Subject: [PATCH] * lisp/comint.el (comint-skip-input): Set inhibit-read-only
 to t

Bug#33975
---
 lisp/comint.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 0a6aff2e73..ab6297a709 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -2536,13 +2536,16 @@ Useful if you accidentally suspend the top-level process."
 
 (defun comint-skip-input ()
   "Skip all pending input, from last stuff output by interpreter to point.
-This means mark it as if it had been sent as input, without sending it."
+This means mark it as if it had been sent as input, without
+sending it.  The command keys used to trigger this command are
+inserted into the buffer."
   (let ((comint-input-sender 'ignore)
 	(comint-input-filter-functions nil))
     (comint-send-input t t))
   (end-of-line)
   (let ((pos (point))
-	(marker (process-mark (get-buffer-process (current-buffer)))))
+	(marker (process-mark (get-buffer-process (current-buffer))))
+        (inhibit-read-only t))
     (insert "  " (key-description (this-command-keys)))
     (if (= marker pos)
 	(set-marker marker (point)))))
-- 
2.19.2

[0001-lisp-comint.el-comint-skip-input-Set-inhibit-read-on.patch (text/x-patch, inline)]

From 0eba90f40c4ffdd020b86ffa2e19815c0edfe2f0 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham <at> gmail.com>
Date: Sat, 23 Feb 2019 07:35:01 -0600
Subject: [PATCH] * lisp/comint.el (comint-skip-input): Set inhibit-read-only
 to t

Bug#33975
---
 lisp/comint.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 0a6aff2e73..ab6297a709 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -2536,13 +2536,16 @@ Useful if you accidentally suspend the top-level process."
 
 (defun comint-skip-input ()
   "Skip all pending input, from last stuff output by interpreter to point.
-This means mark it as if it had been sent as input, without sending it."
+This means mark it as if it had been sent as input, without
+sending it.  The command keys used to trigger this command are
+inserted into the buffer."
   (let ((comint-input-sender 'ignore)
 	(comint-input-filter-functions nil))
     (comint-send-input t t))
   (end-of-line)
   (let ((pos (point))
-	(marker (process-mark (get-buffer-process (current-buffer)))))
+	(marker (process-mark (get-buffer-process (current-buffer))))
+        (inhibit-read-only t))
     (insert "  " (key-description (this-command-keys)))
     (if (= marker pos)
 	(set-marker marker (point)))))
-- 
2.19.2

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 33975 <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Sat, 23 Feb 2019 19:01:46 +0200

> From: Alex Branham <alex.branham <at> gmail.com>
> Cc: 33975 <at> debbugs.gnu.org
> Date: Sat, 23 Feb 2019 07:36:43 -0600
> 
> > Then shouldn't the change be inside comint-skip-input instead?  I
> > mean, the same problem will happen also in all other callers of
> > comint-skip-input, no?
> 
> Probably, yes. Changed in the attached patch.
> 
> > (I'm not sure I understand why that function inserts the description
> > of this-command-keys -- is that to insert "C-c C-c" into the buffer?
> > This is not in the doc string, perhaps we should add that.)
> 
> The attached patch adds it. WDYT?
> 
>  (defun comint-skip-input ()
>    "Skip all pending input, from last stuff output by interpreter to point.
> -This means mark it as if it had been sent as input, without sending it."
> +This means mark it as if it had been sent as input, without
> +sending it.  The command keys used to trigger this command are
> +inserted into the buffer."

"this command" is inaccurate here, as comint-skip-input is not a
command.  I'd say "the command which called this function" instead.

Other than that, LGTM.  Thanks.

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

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33975 <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Sun, 24 Feb 2019 08:47:25 -0600

[Message part 1 (text/plain, inline)]

On Sat 23 Feb 2019 at 11:01, Eli Zaretskii <eliz <at> gnu.org> wrote:

>>  (defun comint-skip-input ()
>>    "Skip all pending input, from last stuff output by interpreter to point.
>> -This means mark it as if it had been sent as input, without sending it."
>> +This means mark it as if it had been sent as input, without
>> +sending it.  The command keys used to trigger this command are
>> +inserted into the buffer."
>
> "this command" is inaccurate here, as comint-skip-input is not a
> command.  I'd say "the command which called this function" instead.
>
> Other than that, LGTM.  Thanks.

Great, thanks I'll make that change and push to master later today or
tomorrow, unless I hear back otherwise.

Alex

[signature.asc (application/pgp-signature, inline)]

Message #31 received at 33975-done <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33975-done <at> debbugs.gnu.org
Subject: Re: bug#33975: [PATCH] inhibit read-only text properties in
 comint-interrupt-subjob
Date: Sun, 24 Feb 2019 19:11:40 -0600

[Message part 1 (text/plain, inline)]

On Sat 23 Feb 2019 at 11:01, Eli Zaretskii <eliz <at> gnu.org> wrote:

> "this command" is inaccurate here, as comint-skip-input is not a
> command.  I'd say "the command which called this function" instead.
>
> Other than that, LGTM.  Thanks.

Thanks, fixed and pushed to master as
6a3b1aaa066dac28355ca5d09550947250108950 to appear in Emacs 27

[signature.asc (application/pgp-signature, inline)]

Previous Next

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