bug#56110: 27+; switching from line-mode to char-mode

bug#56110: 27+; switching from line-mode to char-mode

[C. Michailidis]

I recently updated a few machines and noticed that term.el in Emacs
version 27+ forcibly submits my pasted commands to the shell sub-process
when switching to char-mode (from line-mode). In prior versions of Emacs
term.el didn't do this; I could further modify the command in char-mode
even if it was pasted from line-mode and submit it myself by pressing
enter.

See also:
https://lists.gnu.org/archive/html/help-gnu-emacs/2022-06/msg00452.html

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

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

"C. Michailidis" <signal3@gmail.com> writes:

> I recently updated a few machines and noticed that term.el in Emacs
> version 27+ forcibly submits my pasted commands to the shell sub-process
> when switching to char-mode (from line-mode). In prior versions of Emacs
> term.el didn't do this; I could further modify the command in char-mode
> even if it was pasted from line-mode and submit it myself by pressing
> enter.

Thanks for the report.

This issue is a side effect of this change:

5653b76d0b * lisp/term.el: Fix minor compilation issues with cl-lib and lexbind
Stefan Monnier <monnier@iro.umontreal.ca> 2019-02-24

When I revert the part that changes `term-char-mode':


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Fix-56110.patch --]
[-- Type: text/x-diff, Size: 1230 bytes --]

From 3ebb78f622453623a406d4413cbee421ae57ad84 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Tue, 21 Jun 2022 13:41:51 +0200
Subject: [PATCH] WIP: Fix 56110

---
 lisp/term.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 94bf13e973..eec78707e7 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1463,14 +1463,16 @@ term-char-mode
     (add-hook 'post-command-hook #'term-goto-process-mark-maybe nil t)

     ;; Send existing partial line to inferior (without newline).
-    (let ((pmark (process-mark (get-buffer-process (current-buffer)))))
+    (let ((pmark (process-mark (get-buffer-process (current-buffer))))
+	  (save-input-sender term-input-sender))
       (when (> (point) pmark)
 	(unwind-protect
 	    (progn
-	      (add-function :override term-input-sender #'term-send-string)
+	      (setq term-input-sender
+		    (symbol-function 'term-send-string))
 	      (end-of-line)
 	      (term-send-input))
-	  (remove-function term-input-sender #'term-send-string))))
+	  (setq term-input-sender save-input-sender))))
     (term-update-mode-line)))

 (defun term-line-mode  ()
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 113 bytes --]


the original behavior is restored.

Stefan, how would a fix look like that fits your intention?

TIA,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Stefan, how would a fix look like that fits your intention?

This maybe?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Fix-56110.patch --]
[-- Type: text/x-diff, Size: 916 bytes --]

From db062f00cbd4cdf5934237dee3350046a40f0482 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Tue, 21 Jun 2022 13:41:51 +0200
Subject: [PATCH] WIP: Fix 56110

---
 lisp/term.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 94bf13e973..a8e44b4c34 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1467,10 +1467,10 @@ term-char-mode
       (when (> (point) pmark)
 	(unwind-protect
 	    (progn
-	      (add-function :override term-input-sender #'term-send-string)
+	      (add-function :override (local 'term-input-sender) #'term-send-string)
 	      (end-of-line)
 	      (term-send-input))
-	  (remove-function term-input-sender #'term-send-string))))
+	  (remove-function (local 'term-input-sender) #'term-send-string))))
     (term-update-mode-line)))

 (defun term-line-mode  ()
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 204 bytes --]


[ I first failed to do this correctly because I had tried with
(local term-input-sender) instead of (local 'term-input-sender) - maybe
we can warn about that syntax missing that quote? ]

TIA,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> This maybe?

That works - but I fail to understand why a simple `let' doesn't suffice
(which works as well):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Fix-56110.patch --]
[-- Type: text/x-diff, Size: 1003 bytes --]

From 24a089360bfb8521b255d583e462dc19cac9179b Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Tue, 21 Jun 2022 13:41:51 +0200
Subject: [PATCH] WIP: Fix 56110

---
 lisp/term.el | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 94bf13e973..1fde42aa7f 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1465,12 +1465,9 @@ term-char-mode
     ;; Send existing partial line to inferior (without newline).
     (let ((pmark (process-mark (get-buffer-process (current-buffer)))))
       (when (> (point) pmark)
-	(unwind-protect
-	    (progn
-	      (add-function :override term-input-sender #'term-send-string)
-	      (end-of-line)
-	      (term-send-input))
-	  (remove-function term-input-sender #'term-send-string))))
+	(let ((term-input-sender #'term-send-string))
+	  (end-of-line)
+	  (term-send-input))))
     (term-update-mode-line)))

 (defun term-line-mode  ()
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 36 bytes --]


Does anybody know?

TIA,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[signal3]

On Wed, Jun 22, 2022 at 5:23 PM Michael Heerdegen
<michael_heerdegen@web.de> wrote:
>
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
> > This maybe?
>
> That works - but I fail to understand why a simple `let' doesn't suffice
> (which works as well):
>
>
> Does anybody know?
>
> TIA,
>
> Michael.

I don't know. But, whenever I'm making changes I like to keep the
smallest hamming distance possible between revisions. Just a pet peeve
maybe?

So unless there is a reason to avoid using 'let', it seems preferable
to me. That way the snippet better resembles the version prior to 27+.

And in that vein, I'm not sure why the unwind-protect was removed.

bug#56110: 27+; switching from line-mode to char-mode

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

signal3 [2022-06-22 19:22 +0000] wrote:

> On Wed, Jun 22, 2022 at 5:23 PM Michael Heerdegen
> <michael_heerdegen@web.de> wrote:
>>
>> That works - but I fail to understand why a simple `let' doesn't suffice
>> (which works as well):
>>
>> Does anybody know?

FWIW, I also fail to see why let wouldn't be equivalent for most
intensive porpoises, but then I wouldn't trust myself to tell a porpoise
from a dolphin.

> I don't know. But, whenever I'm making changes I like to keep the
> smallest hamming distance possible between revisions. Just a pet peeve
> maybe?
>
> So unless there is a reason to avoid using 'let', it seems preferable
> to me. That way the snippet better resembles the version prior to 27+.

Surely rewriting in terms of let increases Hamming distance? ;)

In any case, my vote is for correct and simpler code over a smaller
diff.

> And in that vein, I'm not sure why the unwind-protect was removed.

AFAICT it's redundant because let always unwinds its temporary bindings,
in contrast with the previous add-function and setq which make permanent
modifications that require manual unwinding.

Thanks,

-- 
Basil

bug#56110: 27+; switching from line-mode to char-mode

[signal3]

On Wed, Jun 22, 2022 at 9:14 PM Basil L. Contovounesios <contovob@tcd.ie> wrote:
<snip>
> In any case, my vote is for correct and simpler code over a smaller
> diff.

Lol, correct is definitely better! Smaller diff might be considered if
there's doubt regarding correctness.

> > And in that vein, I'm not sure why the unwind-protect was removed.
>
> AFAICT it's redundant because let always unwinds its temporary bindings,
> in contrast with the previous add-function and setq which make permanent
> modifications that require manual unwinding.

Ah... makes sense to me now, thanks for the clarification..

bug#56110: 27+; switching from line-mode to char-mode

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> This maybe?
>
> That works - but I fail to understand why a simple `let' doesn't suffice
> (which works as well):

Depends if you want to have to think about what other code does or not.
If you don't, then `let` is not the same: e.g. if some other code uses
`add/remove-function` on that variable within your `let`, their changes
will be lost when your `let` ends.


        Stefan

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > That works - but I fail to understand why a simple `let' doesn't suffice
> > (which works as well):
>
> Depends if you want to have to think about what other code does or
> not.

I want.  The initial revision by Richard already looks like

#+begin_src emacs-lisp
(unwind-protect
    (progn
      (setq term-input-sender (symbol-function 'term-send-string))
      (end-of-line)
      (term-send-input))
  (setq term-input-sender save-input-sender))
#+end_src

I checked (using variable watchers) that when I replace
unwind-protect+setq with let, the executed code doesn't leave the scope
of the let.  So why was it written like that?  `term-input-sender' has a
buffer local binding (it already had in the initial revision from 1994)
- but the current buffer is not changed in between.

Did `let' back then not work with buffer-local bindings - or what could
have been the intention to avoid `let'?

> If you don't, then `let` is not the same: e.g. if some other code uses
> `add/remove-function` on that variable within your `let`, their changes
> will be lost when your `let` ends.

Yeah, such things - but I don't think anything like this is crucial
here.


Thanks,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I want.  The initial revision by Richard already looks like
>
> #+begin_src emacs-lisp
> (unwind-protect
>     (progn
>       (setq term-input-sender (symbol-function 'term-send-string))
>       (end-of-line)
>       (term-send-input))
>   (setq term-input-sender save-input-sender))
> #+end_src

I have no idea why it was written that way.  There's been all kinds of
bugs linked to `let` bindings interacting with buffer-local bindings,
but that doesn't seem to explain it either.
It's probably just an accident of history.

>> If you don't, then `let` is not the same: e.g. if some other code uses
>> `add/remove-function` on that variable within your `let`, their changes
>> will be lost when your `let` ends.
> Yeah, such things - but I don't think anything like this is crucial
> here.

So you prefer taking the risk that such a thing happens at some point in
the future, or that someone copies this code without knowing what were
your assumptions?

Personally, I prefer using `add/remove-function` and stop worrying about
those risks.  After all, that's part of the reason why I developed them.


        Stefan

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So you prefer taking the risk that such a thing happens at some point in
> the future, or that someone copies this code without knowing what were
> your assumptions?
>
> Personally, I prefer using `add/remove-function` and stop worrying about
> those risks.  After all, that's part of the reason why I developed them.

I must admit I don't understand.

First: Don't buffer local variables come with the same problem?

Second: When somebody changes a binding using `add/remove-function` with
a scope limited to the scope of my `let', we have no problem.

So let's assume a global scope is wanted.  Then that somebody still has
to figure out what exactly to add-function to, and if the currently seen
value is not the global one, or what the scope of the currently seen
binding is.  Bindings can be local or buffer local.

Then one has to figure out if the binding (variable), or the value is to
be modified.  You yourself got it wrong in this case.  So using
`add/remove-function` is still absolutely nontrivial if I used
`add-function' instead of `let'.

Finally, what about variables that can be bound to functions but also
other types like strings?  You can't use `add-function' on them, right?

TIA,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> First: Don't buffer local variables come with the same problem?

The problem is somewhat orthogonal to buffer-local vs global, yes.

> Second: When somebody changes a binding using `add/remove-function` with
> a scope limited to the scope of my `let', we have no problem.

Indeed.

> So let's assume a global scope is wanted.  Then that somebody still has
> to figure out what exactly to add-function to, and if the currently seen
> value is not the global one, or what the scope of the currently seen
> binding is.  Bindings can be local or buffer local.

The same holds if they want to use `let` (except that it's a lot more
difficult to let-bind the global value of a variable when there is
a buffer-local value).

> Then one has to figure out if the binding (variable), or the value is to
> be modified.

I don't understand what you mean by that.

> You yourself got it wrong in this case.  So using
> `add/remove-function` is still absolutely nontrivial if I used
> `add-function' instead of `let'.

Yup.  The purpose is not to protect you from your own errors, but to let
you write code whose semantics is more precisely the ones you need such
that the interaction with other code out there will be correct even for
"unexpected" code.

> Finally, what about variables that can be bound to functions but also
> other types like strings?  You can't use `add-function' on them, right?

That's right (well, you still can if the variable currently holds
a function, but it's less convenient in any case).


        Stefan

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > Then one has to figure out if the binding (variable), or the value is to
> > be modified.
>
> I don't understand what you mean by that.

Simply speaking I meant one has to figure out whether one wants to
`add-function' to var or to 'var.

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Simply speaking I meant one has to figure out whether one wants to
> `add-function' to var or to 'var.

`add-function` expects a "place", like `setf`.


        Stefan

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > Simply speaking I meant one has to figure out whether one wants to
> > `add-function' to var or to 'var.
>
> `add-function` expects a "place", like `setf`.

Thanks, I had an error in my mental model, partly induced by the SYMBOL
without quote vs. (local 'SYMBOL) with quote thing.

Do we want a let-like environment (`let-function'?) that does the same
as `add-function' temporarily?

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Personally, I prefer using `add/remove-function` and stop worrying about
> those risks.  After all, that's part of the reason why I developed them.

Ok - I have now installed this fix:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-Bug-56110-switching-from-line-mode-to-char-mode.patch --]
[-- Type: text/x-diff, Size: 1086 bytes --]

From edf6f5d0cae97de10c914c6e94dc5b35f06ec33c Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Tue, 21 Jun 2022 13:41:51 +0200
Subject: [PATCH] Fix Bug#56110 (switching from line-mode to char-mode)

* lisp/term.el (term-char-mode): Make `add-function' override the
correct place (the buffer local variable `term-input-sender').
---
 lisp/term.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 94bf13e973..a8e44b4c34 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1467,10 +1467,10 @@ term-char-mode
       (when (> (point) pmark)
 	(unwind-protect
 	    (progn
-	      (add-function :override term-input-sender #'term-send-string)
+	      (add-function :override (local 'term-input-sender) #'term-send-string)
 	      (end-of-line)
 	      (term-send-input))
-	  (remove-function term-input-sender #'term-send-string))))
+	  (remove-function (local 'term-input-sender) #'term-send-string))))
     (term-update-mode-line)))

 (defun term-line-mode  ()
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 19 bytes --]


Thanks,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Andreas Schwab]

On Jun 26 2022, Michael Heerdegen wrote:

> diff --git a/lisp/term.el b/lisp/term.el
> index 94bf13e973..a8e44b4c34 100644
> --- a/lisp/term.el
> +++ b/lisp/term.el
> @@ -1467,10 +1467,10 @@ term-char-mode
>        (when (> (point) pmark)
>  	(unwind-protect
>  	    (progn
> -	      (add-function :override term-input-sender #'term-send-string)
> +	      (add-function :override (local 'term-input-sender) #'term-send-string)
>  	      (end-of-line)
>  	      (term-send-input))
> -	  (remove-function term-input-sender #'term-send-string))))
> +	  (remove-function (local 'term-input-sender) #'term-send-string))))
>      (term-update-mode-line)))

Why can't that simply use let?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#56110: 27+; switching from line-mode to char-mode

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> > Simply speaking I meant one has to figure out whether one wants to
>> > `add-function' to var or to 'var.
>> `add-function` expects a "place", like `setf`.
> Thanks, I had an error in my mental model, partly induced by the SYMBOL
> without quote vs. (local 'SYMBOL) with quote thing.

Right, that's because for the argument to `local`, we can actually
accept any expression (i.e. an rvalue) as long as it returns a symbol
(just like for `(setf (car EXP) ..)` where can accept an expression EXP
as long as it returns a cons-cell).

I did hesitate to restrict the syntax to (local SYMBOL) because of the
asymmetry it between "SYMBOL" for the global case and "(local 'SYMBOL)"
for the local case, but there is no technical reason to restrict it
this way.

> Do we want a let-like environment (`let-function'?) that does the same
> as `add-function' temporarily?

I'm not sure it's common enough to be worthwhile.  But if we do, then
we'd also want to add something for the `add/remove-hook` version of
the problem.


        Stefan

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Why can't that simply use let?

I asked the same question in <877d58obnn.fsf@web.de> above in this bug's
thread.

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[signal3]

I may literally die from laughter when someone changes it back! See:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49186
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44087

On Sun, Jun 26, 2022 at 2:49 PM Michael Heerdegen
<michael_heerdegen@web.de> wrote:
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
> > Personally, I prefer using `add/remove-function` and stop worrying about
> > those risks.  After all, that's part of the reason why I developed them.
>
> Ok - I have now installed this fix:
>
>
> Thanks,
>
> Michael.



-- 
Software is written for people to read; the machine's execution of it
is ancillary.

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

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

signal3 <signal3@gmail.com> writes:

> I may literally die from laughter when someone changes it back! See:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49186
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44087

Oh no - not literally!  Anyway - thanks for this important hint.

Is installing this ok for everyone (Lars?)?  I don't think anyone will
miss the never intended behavior.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-term-char-mode-doc-string-correction.patch --]
[-- Type: text/x-diff, Size: 1806 bytes --]

From bf8663415e31abb6b7b96baff2947d9b74b37b1a Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Sun, 26 Jun 2022 19:14:41 +0200
Subject: [PATCH] `term-char-mode' doc string correction

This reverts 16860f6c5f "`term-char-mode' doc string clarification".
Making switching to `term-char-mode' send partially given input
obviously was changed by accident (in 5653b76d0b "Fix minor
compilation issues with cl-lib and lexbind").

See Bug#44087, Bug#49186 and Bug#56110.

* lisp/term.el (term-char-mode): Restore the version of the docstring
describing the originally intended behavior.  Break an overlong line.
---
 lisp/term.el | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index a8e44b4c34..2a7eb72363 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1445,10 +1445,7 @@ term-send-function-key
 (defun term-char-mode ()
   "Switch to char (\"raw\") sub-mode of term mode.
 Each character you type is sent directly to the inferior without
-intervention from Emacs, except for the escape character (usually C-c).
-
-This command will send existing partial lines to the terminal
-process."
+intervention from Emacs, except for the escape character (usually C-c)."
   (interactive)
   ;; FIXME: Emit message? Cfr ilisp-raw-message
   (when (term-in-line-mode)
@@ -1467,7 +1464,8 @@ term-char-mode
       (when (> (point) pmark)
 	(unwind-protect
 	    (progn
-	      (add-function :override (local 'term-input-sender) #'term-send-string)
+	      (add-function :override (local 'term-input-sender)
+                            #'term-send-string)
 	      (end-of-line)
 	      (term-send-input))
 	  (remove-function (local 'term-input-sender) #'term-send-string))))
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 16 bytes --]


TIA,

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Is installing this ok for everyone (Lars?)?  I don't think anyone will
> miss the never intended behavior.

Lars was faster, he had already updated the docstring.

Michael.

bug#56110: 27+; switching from line-mode to char-mode

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Is installing this ok for everyone (Lars?)?  I don't think anyone will
> miss the never intended behavior.

[...]

> This reverts 16860f6c5f "`term-char-mode' doc string clarification".

I've already reverted that, though?

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

bug#56110: 27+; switching from line-mode to char-mode

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > This reverts 16860f6c5f "`term-char-mode' doc string clarification".
>
> I've already reverted that, though?

Yes.  Meanwhile I have noticed, too :-)

Thanks,

Michael.