bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

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

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


Hi Stefan and Eli!

As a part of my investigation into transposing things using tree-sitter
I promised to also introduce some improvements to both the code and
documentation, as the latter is a little sparse.  I'll add some patches
while I work on the tree-sitter stuff.

This commit simplifies the transpose-chars.  The bug described in the
commit history is no longer reproducible, so I think its complicated
nature can be removed.  What do you think?

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-p-in-interactive-form-in-transpose-chars.patch --]
[-- Type: text/x-diff, Size: 1470 bytes --]

From f7d98eb478b9d10c15e6b2d168049e65fcd3c448 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sat, 17 Dec 2022 13:14:22 +0100
Subject: [PATCH] Use *p in interactive form in transpose-chars

There are two changes in this commit.  The first one is removal of an
obsolete guard against moving into read-only text, as described in
bug#17829.  The second is a simplification of the code to make it more
similar to its sibling commands.  We can convert the prefix into a
number immediately, rather than later.

* lisp/simple.el (transpose-chars): Use '*p' instead of '*P' like the
other transpose-* functions.
---
 lisp/simple.el | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index dcc2242e49..bde65d96f1 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8421,11 +8421,8 @@ transpose-chars
 With prefix arg ARG, effect is to take character before point
 and drag it forward past ARG other characters (backward if ARG negative).
 If no argument and at end of line, the previous two chars are exchanged."
-  (interactive "*P")
-  (when (and (null arg) (eolp) (not (bobp))
-	     (not (get-text-property (1- (point)) 'read-only)))
-    (forward-char -1))
-  (transpose-subr 'forward-char (prefix-numeric-value arg)))
+  (interactive "*p")
+  (transpose-subr 'forward-char arg))
 
 (defun transpose-words (arg)
   "Interchange words around point, leaving point at end of them.
-- 
2.34.1

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, eliz@gnu.org
> From: Theodor Thornhill <theo@thornhill.no>
> Date: Sat, 17 Dec 2022 13:23:53 +0100
> 
> Hi Stefan and Eli!
> 
> As a part of my investigation into transposing things using tree-sitter
> I promised to also introduce some improvements to both the code and
> documentation, as the latter is a little sparse.  I'll add some patches
> while I work on the tree-sitter stuff.
> 
> This commit simplifies the transpose-chars.  The bug described in the
> commit history is no longer reproducible, so I think its complicated
> nature can be removed.  What do you think?
> 
> Theo
> 
> >From f7d98eb478b9d10c15e6b2d168049e65fcd3c448 Mon Sep 17 00:00:00 2001
> From: Theodor Thornhill <theo@thornhill.no>
> Date: Sat, 17 Dec 2022 13:14:22 +0100
> Subject: [PATCH] Use *p in interactive form in transpose-chars
> 
> There are two changes in this commit.  The first one is removal of an
> obsolete guard against moving into read-only text, as described in
> bug#17829.  The second is a simplification of the code to make it more
> similar to its sibling commands.  We can convert the prefix into a
> number immediately, rather than later.
> 
> * lisp/simple.el (transpose-chars): Use '*p' instead of '*P' like the
> other transpose-* functions.
> ---
>  lisp/simple.el | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/lisp/simple.el b/lisp/simple.el
> index dcc2242e49..bde65d96f1 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -8421,11 +8421,8 @@ transpose-chars
>  With prefix arg ARG, effect is to take character before point
>  and drag it forward past ARG other characters (backward if ARG negative).
>  If no argument and at end of line, the previous two chars are exchanged."
> -  (interactive "*P")
> -  (when (and (null arg) (eolp) (not (bobp))
> -	     (not (get-text-property (1- (point)) 'read-only)))
> -    (forward-char -1))
> -  (transpose-subr 'forward-char (prefix-numeric-value arg)))
> +  (interactive "*p")
> +  (transpose-subr 'forward-char arg))
>  
>  (defun transpose-words (arg)
>    "Interchange words around point, leaving point at end of them.
> -- 
> 2.34.1

Stefan, any further comments, or should I install this?

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

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

>>  If no argument and at end of line, the previous two chars are exchanged."
>> -  (interactive "*P")
>> -  (when (and (null arg) (eolp) (not (bobp))
>> -	     (not (get-text-property (1- (point)) 'read-only)))
>> -    (forward-char -1))

Hmm... If you remove this, does it still behave as described on the last
line of the docstring?

>> +  (transpose-subr 'forward-char arg))

Please use #' to quote function names.


        Stefan

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

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

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

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

>>>  If no argument and at end of line, the previous two chars are exchanged."
>>> -  (interactive "*P")
>>> -  (when (and (null arg) (eolp) (not (bobp))
>>> -	     (not (get-text-property (1- (point)) 'read-only)))
>>> -    (forward-char -1))
>
> Hmm... If you remove this, does it still behave as described on the last
> line of the docstring?
>

You're right.  It does not.  I fixed that in the latest patch.

>>> +  (transpose-subr 'forward-char arg))
>
> Please use #' to quote function names.
>

Sure!

Is this okay?

Theo



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-p-in-interactive-form-in-transpose-chars.patch --]
[-- Type: text/x-diff, Size: 1322 bytes --]

From 21cabdb2779caa55a53c972150d9bee8a4ca14e5 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sat, 17 Dec 2022 13:14:22 +0100
Subject: [PATCH] Use *p in interactive form in transpose-chars

* lisp/simple.el (transpose-chars): Use '*p' instead of '*P' like the
other transpose-* functions.
---
 lisp/simple.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index b3174eef7d..69a7189a35 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8423,12 +8423,12 @@ transpose-chars
   "Interchange characters around point, moving forward one character.
 With prefix arg ARG, effect is to take character before point
 and drag it forward past ARG other characters (backward if ARG negative).
-If no argument and at end of line, the previous two chars are exchanged."
-  (interactive "*P")
-  (when (and (null arg) (eolp) (not (bobp))
+If at end of line, the previous two chars are exchanged."
+  (interactive "*p")
+  (when (and (eolp) (not (bobp))
 	     (not (get-text-property (1- (point)) 'read-only)))
     (forward-char -1))
-  (transpose-subr 'forward-char (prefix-numeric-value arg)))
+  (transpose-subr #'forward-char arg))
 
 (defun transpose-words (arg)
   "Interchange words around point, leaving point at end of them.
-- 
2.34.1

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

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

> Is this okay?

The code is OK.
I don't understand the motivation behind the change, tho.


        Stefan

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

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

On 25 December 2022 02:11:31 CET, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> Is this okay?
>
>The code is OK.
>I don't understand the motivation behind the change, tho.
>
>
>        Stefan
>

Mostly uniformity with surrounding code and housekeeping, but if such changes are unwanted, I'm fine with that too.

Theo

bug#60157: 30.0.50; [PATCH]: Change interactive form in transpose-chars

[Stefan Kangas]

Version: 30.1

Theodor Thornhill <theo@thornhill.no> writes:

> On 25 December 2022 02:11:31 CET, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> Is this okay?
>>
>>The code is OK.
>>I don't understand the motivation behind the change, tho.
>>
>
> Mostly uniformity with surrounding code and housekeeping, but if such changes are unwanted, I'm fine with that too.

No further comments within 9 months.

Pushed to master as commit 4ac83ee105a.  Closing.