bug#64185: proposal for new function: copy-line

bug#64185: proposal for new function: copy-line

[Zachary Kanfer]

[-- Attachment #1.1: Type: text/plain, Size: 201 bytes --]

This proposal is a function that copies the line point is on, duplicating
it in the buffer.

I find it useful for both text and code. It makes writing certain types of
repetitive things a lot simpler.

[-- Attachment #1.2: Type: text/html, Size: 243 bytes --]

[-- Attachment #2: 0001-Add-copy-line-function.patch --]
[-- Type: text/x-patch, Size: 3883 bytes --]

From 883f4dac31f4c78cdc5dd4622a0b110045840bd9 Mon Sep 17 00:00:00 2001
From: Zachary Kanfer <zkanfer@gmail.com>
Date: Tue, 20 Jun 2023 01:04:47 -0400
Subject: [PATCH] Add copy-line function

* lisp/simple.el (copy-line): Add the copy-line function itself.
* test/lisp/simple-tests.el (simple-tests-zap-to-char): Tests for this
new function.
* etc/NEWS: Update
---
 etc/NEWS                  |  4 ++++
 lisp/simple.el            | 20 ++++++++++++++++++
 test/lisp/simple-tests.el | 44 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 61e6e161665..fe3d68b1b9e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -665,6 +665,10 @@ Their 'noerror' arguments have no effect and are therefore obsolete.
 This function evaluates a command's interactive form and returns the
 resultant list.
 
+** New function 'copy-line' in the simple library.
+This function copies the current line, inserting it before (or, with
+prefix argument, after) point.
+
 \f
 * Changes in Emacs 30.1 on Non-Free Operating Systems
 
diff --git a/lisp/simple.el b/lisp/simple.el
index e08bf4fdd64..0c2cd2ba93a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10977,6 +10977,26 @@ lax-plist-put
   (plist-put plist prop val #'equal))
 \f
 
+(defun copy-line (&optional copy-above)
+  "Copy the line point is on, placing the copy above the current line.
+
+With prefix argument COPY-ABOVE, put the copy below the current line."
+  (interactive "*P")
+  (let ((line (buffer-substring-no-properties (line-beginning-position) (line-end-position)))
+        (where-in-line (- (point) (line-beginning-position))))
+    (if copy-above
+        (progn (beginning-of-line)
+               (open-line 1))
+      (progn (end-of-line)
+             (insert "\n")))
+    (insert line)
+    (goto-char (line-beginning-position))
+    (forward-char where-in-line))
+  (set-transient-map
+   (define-keymap "c" (lambda () (interactive) (copy-line copy-above))))
+  (message "press c to copy again?"))
+\f
+
 (provide 'simple)
 
 ;;; simple.el ends here
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index 7dabb735522..d419e3493ab 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -1046,5 +1046,49 @@ simple-tests-zap-to-char
     (with-zap-to-char-test "abcdeCXYZ" "XYZ"
       (zap-to-char 1 ?C 'interactive))))
 
+;;; Tests for `copy-line'
+
+(ert-deftest copy-test--copy-down--check-text ()
+  (should (equal (with-temp-buffer
+                   (insert "a\nb\nc")
+                   (previous-line)
+                   (copy-line)
+                   (buffer-string))
+                 "a\nb\nb\nc")))
+
+(ert-deftest copy-test--copy-down--check-point ()
+  (should (equal (with-temp-buffer
+                   (insert "a\nb\nc")
+                   (previous-line)
+                   (copy-line)
+                   (point))
+                 6)))
+
+(ert-deftest copy-test--copy-up--check-point ()
+  (should (equal (with-temp-buffer
+                   (insert "a\nb\nc")
+                   (previous-line)
+                   (copy-line t)
+                   (point))
+                 4)))
+
+(ert-deftest copy-test--copy-down--middle-of-line--check-text ()
+  (should (equal (with-temp-buffer
+                   (insert "a\nb c d e\nf")
+                   (beginning-of-line -1)
+                   (forward-char 4)
+                   (copy-line)
+                   (buffer-string))
+                 "a\nb c d e\nb c d e\nf")))
+
+(ert-deftest copy-test--copy-down--middle-of-line--check-point ()
+  (should (equal (with-temp-buffer
+                   (insert "a\nb c d e\nf")
+                   (beginning-of-line 0)
+                   (forward-char 4)
+                   (copy-line)
+                   (point))
+                 15)))
+
 (provide 'simple-test)
 ;;; simple-tests.el ends here
-- 
2.40.1

bug#64185: proposal for new function: copy-line

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

Just a couple of thoughts:

Zachary Kanfer <zkanfer@gmail.com> writes:

> This proposal is a function that copies the line point is on, duplicating it in the buffer.
>
> I find it useful for both text and code. It makes writing certain types of repetitive things a lot simpler.
>

How does this relate to the existing `duplicate-line`?

> +(defun copy-line (&optional copy-above)
> +  "Copy the line point is on, placing the copy above the current line.
> +
> +With prefix argument COPY-ABOVE, put the copy below the current line."
> +  (interactive "*P")
> +  (let ((line (buffer-substring-no-properties (line-beginning-position) (line-end-position)))
> +        (where-in-line (- (point) (line-beginning-position))))
> +    (if copy-above
> +        (progn (beginning-of-line)
> +               (open-line 1))
> +      (progn (end-of-line)
> +             (insert "\n")))
> +    (insert line)
> +    (goto-char (line-beginning-position))
> +    (forward-char where-in-line))
> +  (set-transient-map
> +   (define-keymap "c" (lambda () (interactive) (copy-line copy-above))))
> +  (message "press c to copy again?"))

I think you can get the same behavior by using `duplicate-line` as follows:

(defun esy/copy-line (&optional copy-above)
  (interactive "*P")
  (duplicate-line)
  (unless copy-above (line-move 1)))

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> Cc: 64185@debbugs.gnu.org
> Date: Tue, 20 Jun 2023 09:15:51 +0300
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> How does this relate to the existing `duplicate-line`?

There's also copy-from-above-command.

bug#64185: proposal for new function: copy-line

[Zachary Kanfer]

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

Hrm, this is relatively similar to duplicate-line. I was running 28.2, and
duplicate-line was added since that release. Thank you for pointing those
functions out.

I do think it's more useful to put point on the second line; I almost
always find myself wanting to edit the "new" line, not the old one. Yes,
there isn't really a new vs old line, but I write the first thing I want to
see first, then want to duplicate it to make a changed second line.

I suspect that although it hasn't been in a released version yet, it's
slightly late to have a discussion about changing the behavior to leave
point on the second line instead of the first.

On Tue, Jun 20, 2023 at 7:44 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > Cc: 64185@debbugs.gnu.org
> > Date: Tue, 20 Jun 2023 09:15:51 +0300
> > From:  Eshel Yaron via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >
> > How does this relate to the existing `duplicate-line`?
>
> There's also copy-from-above-command.
>

[-- Attachment #2: Type: text/html, Size: 1563 bytes --]

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Zachary Kanfer <zkanfer@gmail.com>
> Date: Wed, 21 Jun 2023 23:33:15 -0400
> Cc: Eshel Yaron <me@eshelyaron.com>, 64185@debbugs.gnu.org
> 
> I do think it's more useful to put point on the second line; I almost always find myself wanting to edit the
> "new" line, not the old one. Yes, there isn't really a new vs old line, but I write the first thing I want to see
> first, then want to duplicate it to make a changed second line.

We could have the prefix arg of - (i.e., "C-u - M-x ...") produce that
behavior from duplicate-line.  Or we could have a user option to do
that regardless of the argument, but then we should decide on which of
the N new lines to put point: the first one copied or the last one.

> I suspect that although it hasn't been in a released version yet, it's slightly late to have a discussion
> about changing the behavior to leave point on the second line instead of the first.

Not the default behavior, but we could have it optionally behave
differently.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> I do think it's more useful to put point on the second line; I almost
>> always find myself wanting to edit the "new" line, not the old
>> one. Yes, there isn't really a new vs old line, but I write the first
>> thing I want to see first, then want to duplicate it to make
>> a changed second line.

I agree that it's more useful to put point on the second line.
This is the reason why I can't use the new command 'duplicate-line',
so forced to continue using my own implementation in ~/.emacs.

> We could have the prefix arg of - (i.e., "C-u - M-x ...") produce that
> behavior from duplicate-line.

The problem is how to define this behavior with the prefix arg C-u in case
when a prefix numeric argument is used.

> Or we could have a user option to do that regardless of the argument,
> but then we should decide on which of the N new lines to put point:
> the first one copied or the last one.

A user option looks preferable.  In case of a prefix numeric argument,
it also makes sense to have 3 choices:

1. leave point on the old line
2. move point to the first new line
3. move point to the last new line

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Zachary Kanfer <zkanfer@gmail.com>,  64185@debbugs.gnu.org,
>   me@eshelyaron.com
> Date: Thu, 22 Jun 2023 09:57:31 +0300
> 
> > We could have the prefix arg of - (i.e., "C-u - M-x ...") produce that
> > behavior from duplicate-line.
> 
> The problem is how to define this behavior with the prefix arg C-u in case
> when a prefix numeric argument is used.
> 
> > Or we could have a user option to do that regardless of the argument,
> > but then we should decide on which of the N new lines to put point:
> > the first one copied or the last one.
> 
> A user option looks preferable.  In case of a prefix numeric argument,
> it also makes sense to have 3 choices:
> 
> 1. leave point on the old line
> 2. move point to the first new line
> 3. move point to the last new line

Patches welcome, but if you want to have this in Emacs 29, please
hurry up with the patches.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

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

>> > Or we could have a user option to do that regardless of the argument,
>> > but then we should decide on which of the N new lines to put point:
>> > the first one copied or the last one.
>>
>> A user option looks preferable.  In case of a prefix numeric argument,
>> it also makes sense to have 3 choices:
>>
>> 1. leave point on the old line
>> 2. move point to the first new line
>> 3. move point to the last new line
>
> Patches welcome, but if you want to have this in Emacs 29, please
> hurry up with the patches.

This is both simple and general allowing to specify any number
for the new line position counting from the first/last line,
but only values 0, 1, and -1 are really useful:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: duplicate-line-pos.patch --]
[-- Type: text/x-diff, Size: 1421 bytes --]

diff --git a/lisp/misc.el b/lisp/misc.el
index ca013d5f72f..a0c8a1f620e 100644
--- a/lisp/misc.el
+++ b/lisp/misc.el
@@ -63,6 +63,14 @@ copy-from-above-command
 				 (+ n (point)))))))
     (insert string)))

+(defcustom duplicate-line-pos 0
+  "Where to put point after copying the line.
+When 0, leave point on the original line.
+When 1, move point to the first new line.
+When -1, move point to the last new line."
+  :type 'integer
+  :version "29.1")
+
 ;;;###autoload
 (defun duplicate-line (&optional n)
   "Duplicate the current line N times.
@@ -71,13 +79,19 @@ duplicate-line
   (interactive "p")
   (unless n
     (setq n 1))
-  (let ((line (buffer-substring (line-beginning-position) (line-end-position))))
-    (save-excursion
-      (forward-line 1)
-      (unless (bolp)
-        (insert "\n"))
-      (dotimes (_ n)
-        (insert line "\n")))))
+  (let ((line (buffer-substring (line-beginning-position) (line-end-position)))
+        (pos (unless (< duplicate-line-pos 0) (point)))
+        (column (unless (eq duplicate-line-pos 0) (current-column))))
+    (forward-line 1)
+    (unless (bolp)
+      (insert "\n"))
+    (dotimes (_ n)
+      (insert line "\n"))
+    (unless (< duplicate-line-pos 0)
+      (goto-char pos))
+    (unless (eq duplicate-line-pos 0)
+      (forward-line duplicate-line-pos)
+      (move-to-column column))))

 (declare-function rectangle--duplicate-right "rect" (n))

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: zkanfer@gmail.com,  64185@debbugs.gnu.org,  me@eshelyaron.com
> Date: Thu, 22 Jun 2023 20:27:04 +0300
> 
> > Patches welcome, but if you want to have this in Emacs 29, please
> > hurry up with the patches.
> 
> This is both simple and general allowing to specify any number
> for the new line position counting from the first/last line,
> but only values 0, 1, and -1 are really useful:

Thanks, this LGTM, with one comment:

> +(defcustom duplicate-line-pos 0
> +  "Where to put point after copying the line.

A better name for this would be duplicate-line-final-position or maybe
duplicate-line-point-position.

Also, please mention this variable in NEWS, where duplicate-line is
announced.

bug#64185: proposal for new function: copy-line

[Drew Adams]

> > +(defcustom duplicate-line-pos 0
> > +  "Where to put point after copying the line.
> 
> A better name for this would be duplicate-line-final-position or maybe
> duplicate-line-point-position.
> 
> Also, please mention this variable in NEWS, where duplicate-line is
> announced.

FWIW -

Point is the cursor position.  So I'd suggest
just "point" (or "cursor-position", if you
like), and not "point-position".

We do use `pos' in various names to mean a
buffer position, but in those cases it
doesn't stand for point (the cursor position).
So I agree that `pos' shouldn't be used here.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

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

>> +(defcustom duplicate-line-pos 0
>> +  "Where to put point after copying the line.
>
> A better name for this would be duplicate-line-final-position or maybe
> duplicate-line-point-position.

Unless someone has an idea for a shorter name that is still descriptive,
here is the final patch for emacs-29:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: duplicate-line-final-position.patch --]
[-- Type: text/x-diff, Size: 3526 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index ca0d602e9ad..aa3b758a815 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -696,7 +696,9 @@ between these modes while the user is inputting a command by hitting
 'duplicate-line' duplicates the current line the specified number of times.
 'duplicate-dwim' duplicates the region if it is active.  If not, it
 works like 'duplicate-line'.  An active rectangular region is
-duplicated on its right-hand side.
+duplicated on its right-hand side.  The new user option
+'duplicate-line-final-position' specifies where to move point
+after duplicating the line.
 
 ---
 ** Files with the ".eld" extension are now visited in 'lisp-data-mode'.
diff --git a/lisp/misc.el b/lisp/misc.el
index ca013d5f72f..67048d4ec34 100644
--- a/lisp/misc.el
+++ b/lisp/misc.el
@@ -63,21 +63,41 @@ copy-from-above-command
 				 (+ n (point)))))))
     (insert string)))
 
+(defcustom duplicate-line-final-position 0
+  "Where to put point after duplicating the line with `duplicate-line'.
+When 0, leave point on the original line.
+When 1, move point to the first new line.
+When -1, move point to the last new line."
+  :type '(choice (const :tag "Leave point on old line" 0)
+                 (const :tag "Move point to first new line" 1)
+                 (const :tag "Move point to last new line" -1)
+                 (integer))
+  :group 'editing
+  :version "29.1")
+
 ;;;###autoload
 (defun duplicate-line (&optional n)
   "Duplicate the current line N times.
 Interactively, N is the prefix numeric argument, and defaults to 1.
+The user option `duplicate-line-final-position' specifies where to
+move point after duplicating the line.
 Also see the `copy-from-above-command' command."
   (interactive "p")
   (unless n
     (setq n 1))
-  (let ((line (buffer-substring (line-beginning-position) (line-end-position))))
-    (save-excursion
-      (forward-line 1)
-      (unless (bolp)
-        (insert "\n"))
-      (dotimes (_ n)
-        (insert line "\n")))))
+  (let ((line (buffer-substring (line-beginning-position) (line-end-position)))
+        (pos (point))
+        (col (current-column)))
+    (forward-line 1)
+    (unless (bolp)
+      (insert "\n"))
+    (dotimes (_ n)
+      (insert line "\n"))
+    (unless (< duplicate-line-final-position 0)
+      (goto-char pos))
+    (unless (eq duplicate-line-final-position 0)
+      (forward-line duplicate-line-final-position)
+      (move-to-column col))))
 
 (declare-function rectangle--duplicate-right "rect" (n))
 
diff --git a/test/lisp/misc-tests.el b/test/lisp/misc-tests.el
index f1d22e099b9..ea27ea1653b 100644
--- a/test/lisp/misc-tests.el
+++ b/test/lisp/misc-tests.el
@@ -88,6 +88,20 @@ misc--duplicate-line
     (duplicate-line 2)
     (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
     (should (equal (point) 7)))
+  ;; Duplicate a line (twice) and move point to the first duplicated line.
+  (with-temp-buffer
+    (insert "abc\ndefg\nh\n")
+    (goto-char 7)
+    (let ((duplicate-line-final-position 1)) (duplicate-line 2))
+    (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
+    (should (equal (point) 12)))
+  ;; Duplicate a line (twice) and move point to the last duplicated line.
+  (with-temp-buffer
+    (insert "abc\ndefg\nh\n")
+    (goto-char 7)
+    (let ((duplicate-line-final-position -1)) (duplicate-line 2))
+    (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
+    (should (equal (point) 17)))
   ;; Duplicate a non-terminated line.
   (with-temp-buffer
     (insert "abc")

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> > +(defcustom duplicate-line-pos 0
>> > +  "Where to put point after copying the line.
>> 
>> A better name for this would be duplicate-line-final-position or maybe
>> duplicate-line-point-position.
>> 
>> Also, please mention this variable in NEWS, where duplicate-line is
>> announced.
>
> FWIW -
>
> Point is the cursor position.  So I'd suggest
> just "point" (or "cursor-position", if you
> like), and not "point-position".
>
> We do use `pos' in various names to mean a
> buffer position, but in those cases it
> doesn't stand for point (the cursor position).
> So I agree that `pos' shouldn't be used here.

In the patch a new position is counted by lines,
so maybe a better name would be 'duplicate-line-final-line'.
But this double "line-line" doesn't look nice.

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: zkanfer@gmail.com,  64185@debbugs.gnu.org,  me@eshelyaron.com
> Date: Thu, 22 Jun 2023 21:17:51 +0300
> 
> Unless someone has an idea for a shorter name that is still descriptive,
> here is the final patch for emacs-29:

LGTM, thanks.  But let's wait for a day or two to give others a chance
to comment.

bug#64185: proposal for new function: copy-line

[Drew Adams]

> > FWIW -
> >
> > Point is the cursor position.  So I'd suggest
> > just "point" (or "cursor-position", if you
> > like), and not "point-position".
> >
> > We do use `pos' in various names to mean a
> > buffer position, but in those cases it
> > doesn't stand for point (the cursor position).
> > So I agree that `pos' shouldn't be used here.
> 
> In the patch a new position is counted by lines,
> so maybe a better name would be 'duplicate-line-final-line'.
> But this double "line-line" doesn't look nice.

Is it about absolute line number, relative line
number, cursor position (point), some other buffer
position,...?

A guess, from what you just wrote, is that it's
about the relative line number, i.e., relative
to the number of the line of the cursor initially.
(Just a guess; haven't followed this thread.)

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> > FWIW -
>> >
>> > Point is the cursor position.  So I'd suggest
>> > just "point" (or "cursor-position", if you
>> > like), and not "point-position".
>> >
>> > We do use `pos' in various names to mean a
>> > buffer position, but in those cases it
>> > doesn't stand for point (the cursor position).
>> > So I agree that `pos' shouldn't be used here.
>>
>> In the patch a new position is counted by lines,
>> so maybe a better name would be 'duplicate-line-final-line'.
>> But this double "line-line" doesn't look nice.
>
> Is it about absolute line number, relative line
> number, cursor position (point), some other buffer
> position,...?
>
> A guess, from what you just wrote, is that it's
> about the relative line number, i.e., relative
> to the number of the line of the cursor initially.
> (Just a guess; haven't followed this thread.)

It's a relative line number - an offset from the
original line number where the number is in line units.

bug#64185: proposal for new function: copy-line

[Drew Adams]

Sorry, but I can't comment further, as I don't
understand what this does.  I don't see the
code for it in the bug thread, and I can't
guess from the descriptions there.

E.g., what does this mean?

> we could have a user option to do that
> regardless of the argument, but then we
> should decide on which of the N new lines
> to put point: the first one copied or the last one.

And this doesn't help (me) either:

> In case of a prefix numeric argument,
> it also makes sense to have 3 choices:
>
> 1. leave point on the old line
> 2. move point to the first new line
> 3. move point to the last new line

The "old line" is presumably the current
line before the command is invoked.  No
idea what "first" and "last" new line
might mean.

Is this about duplicating multiple lines?
Is it about doing that and then also
moving them down or up?

Without a full description or seeing the
code, I can't suggest anything helpful
about the name; sorry.

bug#64185: proposal for new function: copy-line

[Zachary Kanfer]

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

The option looks really nice to me!

Is there a standard to use numbers as the values, rather than, say,
symbols? It seems more readable to look at an option of '(old new last),
for example, rather than '(-1 0 1).

I also like the added tests!

On Thu, Jun 22, 2023 at 2:30 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Juri Linkov <juri@linkov.net>
> > Cc: zkanfer@gmail.com,  64185@debbugs.gnu.org,  me@eshelyaron.com
> > Date: Thu, 22 Jun 2023 21:17:51 +0300
> >
> > Unless someone has an idea for a shorter name that is still descriptive,
> > here is the final patch for emacs-29:
>
> LGTM, thanks.  But let's wait for a day or two to give others a chance
> to comment.
>

[-- Attachment #2: Type: text/html, Size: 1321 bytes --]

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Zachary Kanfer <zkanfer@gmail.com>
> Date: Fri, 23 Jun 2023 01:46:27 -0400
> Cc: Juri Linkov <juri@linkov.net>, 64185@debbugs.gnu.org, me@eshelyaron.com
> 
> Is there a standard to use numbers as the values, rather than, say, symbols? It seems more readable
> to look at an option of '(old new last), for example, rather than '(-1 0 1).

Numbers are much easier for users to use as values of options, so we
should prefer them when they do the job.  A number can also express
more than just those 3 possibilities, so it's more flexible in this
case.

> I also like the added tests!

That's always welcome, TIA.

bug#64185: proposal for new function: copy-line

[Robert Pluim]

>>>>> On Thu, 22 Jun 2023 21:17:51 +0300, Juri Linkov <juri@linkov.net> said:

    Juri> diff --git a/etc/NEWS b/etc/NEWS
    Juri> index ca0d602e9ad..aa3b758a815 100644
    Juri> --- a/etc/NEWS
    Juri> +++ b/etc/NEWS
    Juri> @@ -696,7 +696,9 @@ between these modes while the user is inputting a command by hitting
    Juri>  'duplicate-line' duplicates the current line the specified number of times.
    Juri>  'duplicate-dwim' duplicates the region if it is active.  If not, it
    Juri>  works like 'duplicate-line'.  An active rectangular region is
    Juri> -duplicated on its right-hand side.
    Juri> +duplicated on its right-hand side.  The new user option
    Juri> +'duplicate-line-final-position' specifies where to move point
    Juri> +after duplicating the line.

The first thing that springs to mind here is "where on the line is
point placed?". At the beginning, end, same column as when
duplicate-line was invoked? (I know I could just read the code, but
the caffeine hasnʼt kicked in yet :-))

Robert
-- 

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  64185@debbugs.gnu.org,
>   me@eshelyaron.com,  zkanfer@gmail.com
> Date: Fri, 23 Jun 2023 09:08:56 +0200
> 
> >>>>> On Thu, 22 Jun 2023 21:17:51 +0300, Juri Linkov <juri@linkov.net> said:
> 
>     Juri> diff --git a/etc/NEWS b/etc/NEWS
>     Juri> index ca0d602e9ad..aa3b758a815 100644
>     Juri> --- a/etc/NEWS
>     Juri> +++ b/etc/NEWS
>     Juri> @@ -696,7 +696,9 @@ between these modes while the user is inputting a command by hitting
>     Juri>  'duplicate-line' duplicates the current line the specified number of times.
>     Juri>  'duplicate-dwim' duplicates the region if it is active.  If not, it
>     Juri>  works like 'duplicate-line'.  An active rectangular region is
>     Juri> -duplicated on its right-hand side.
>     Juri> +duplicated on its right-hand side.  The new user option
>     Juri> +'duplicate-line-final-position' specifies where to move point
>     Juri> +after duplicating the line.
> 
> The first thing that springs to mind here is "where on the line is
> point placed?". At the beginning, end, same column as when
> duplicate-line was invoked? (I know I could just read the code, but
> the caffeine hasnʼt kicked in yet :-))

This is a NEWS entry, it doesn't have to provide such details.  The
doc string should, so if these details are missing from the doc
string, it should be improved to include them.

bug#64185: proposal for new function: copy-line

[Robert Pluim]

>>>>> On Fri, 23 Jun 2023 10:19:18 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> The first thing that springs to mind here is "where on the line is
    >> point placed?". At the beginning, end, same column as when
    >> duplicate-line was invoked? (I know I could just read the code, but
    >> the caffeine hasnʼt kicked in yet :-))

    Eli> This is a NEWS entry, it doesn't have to provide such details.  The
    Eli> doc string should, so if these details are missing from the doc
    Eli> string, it should be improved to include them.

The doc string doesnʼt provide those details

Robert
-- 

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

>  'duplicate-dwim' duplicates the region if it is active.  If not, it
>  works like 'duplicate-line'.  An active rectangular region is
> -duplicated on its right-hand side.
> +duplicated on its right-hand side.  The new user option
> +'duplicate-line-final-position' specifies where to move point
> +after duplicating the line.

This makes it unclear to what extent the variable affects `duplicate-dwim`. (For some reason it only does so when the region is inactive, which doesn't seem right.)

And I agree that allowing any integer as value isn't necessary the best choice here. It's a false flexibility; nobody will ever set it to something other than 0, 1 or -1. Symbols would probably be better, for several reasons.

Finally, it's a bit surprising that this is even discussed for inclusion in Emacs 29 at this stage, given the raw state of the design. What about we do a proper job on master instead, instead of rushing a half-baked new feature into a branch that is already deep into pre-release?

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 23 Jun 2023 11:07:58 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  64185@debbugs.gnu.org,
>  me@eshelyaron.com,
>  zkanfer@gmail.com
> 
> Finally, it's a bit surprising that this is even discussed for inclusion in Emacs 29 at this stage, given the raw state of the design. What about we do a proper job on master instead, instead of rushing a half-baked new feature into a branch that is already deep into pre-release?

This is a new feature in Emacs 29, so adding customization
opportunities to it that allow fine control of its behavior is fine at
this stage.  Of course, if the proposal is controversial, we shouldn't
rush.  My impression till now was that it wasn't controversial.  If
the only issue is better documentation and/or whether the value should
be a number or a symbol, then I see no problem with installing this on
the release branch.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

23 juni 2023 kl. 12.28 skrev Eli Zaretskii <eliz@gnu.org>:

> This is a new feature in Emacs 29, so adding customization
> opportunities to it that allow fine control of its behavior is fine at
> this stage.  Of course, if the proposal is controversial, we shouldn't
> rush.  My impression till now was that it wasn't controversial.  If
> the only issue is better documentation and/or whether the value should
> be a number or a symbol, then I see no problem with installing this on
> the release branch.

The new setting would be quite incomplete if it doesn't apply to the more general and useful duplicate-dwim as well. I can help with that but obviously want it done properly.

It's not as straightforward as when duplicating lines because the region would need to move, and it's a lot more complicated for rectangles. This may take some time to get right.

In particular, the full generality of allowing an arbitrary number to be specified adds quite some complexity here, and it's all useless gold-plating -- nobody will ever set that knob to 3, say.

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 23 Jun 2023 12:50:52 +0200
> Cc: juri@linkov.net,
>  64185@debbugs.gnu.org,
>  me@eshelyaron.com,
>  zkanfer@gmail.com
> 
> 23 juni 2023 kl. 12.28 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > This is a new feature in Emacs 29, so adding customization
> > opportunities to it that allow fine control of its behavior is fine at
> > this stage.  Of course, if the proposal is controversial, we shouldn't
> > rush.  My impression till now was that it wasn't controversial.  If
> > the only issue is better documentation and/or whether the value should
> > be a number or a symbol, then I see no problem with installing this on
> > the release branch.
> 
> The new setting would be quite incomplete if it doesn't apply to the
> more general and useful duplicate-dwim as well.

It is? why?  The difference between these two commands is that
duplicate-dwim is sensitive to the active region, but in that case how
is it useful to talk about lines, given that region could be a part of
a line or several lines plus a part of the first and the last one?

So it sounds to me that we could easily leave duplicate-dwim as it is
now, i.e. disregard this option in that command.

Why isn't that "good enough" for Emacs 29?

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>>  'duplicate-dwim' duplicates the region if it is active.  If not, it
>>  works like 'duplicate-line'.  An active rectangular region is
>> -duplicated on its right-hand side.
>> +duplicated on its right-hand side.  The new user option
>> +'duplicate-line-final-position' specifies where to move point
>> +after duplicating the line.
>
> This makes it unclear to what extent the variable affects
> `duplicate-dwim`.  (For some reason it only does so when the region is
> inactive, which doesn't seem right.)

The new command `duplicate-line` would be useless for many users
without that option.  There is no such problem for `duplicate-dwim`
that is more complicated.  It would be a major undertaking to design
a similar option for `duplicate-dwim` for consistency with `duplicate-line`
because of more decisions necessary to make: whether to move point
to the beginning or the end of the copied regions, whether to select
all the copied regions or only one of them, etc.

> And I agree that allowing any integer as value isn't necessary the
> best choice here.  It's a false flexibility; nobody will ever set it
> to something other than 0, 1 or -1.  Symbols would probably be better,
> for several reasons.

For users of the Customization UI the choices are explained clearly:

  :type '(choice (const :tag "Leave point on old line" 0)
                 (const :tag "Move point to first new line" 1)
                 (const :tag "Move point to last new line" -1)
                 (integer))

For users who know Emacs Lisp and like to write plain values in ~/.emacs,
the values 0, 1, -1 are more intuitive and familiar due to functions
like `substring`, etc.

BTW, to my surprise the shortdoc of `substring` has no examples
of a negative argument.  I don't know whether this omission
is intentional to make the string section shorter.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

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

>> The first thing that springs to mind here is "where on the line is
>> point placed?". At the beginning, end, same column as when
>> duplicate-line was invoked? (I know I could just read the code, but
>> the caffeine hasnʼt kicked in yet :-))
>
> This is a NEWS entry, it doesn't have to provide such details.  The
> doc string should, so if these details are missing from the doc
> string, it should be improved to include them.

Now the docstring is updated:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: duplicate-line-final-position.patch --]
[-- Type: text/x-diff, Size: 3584 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index ca0d602e9ad..aa3b758a815 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -696,7 +696,9 @@ between these modes while the user is inputting a command by hitting
 'duplicate-line' duplicates the current line the specified number of times.
 'duplicate-dwim' duplicates the region if it is active.  If not, it
 works like 'duplicate-line'.  An active rectangular region is
-duplicated on its right-hand side.
+duplicated on its right-hand side.  The new user option
+'duplicate-line-final-position' specifies where to move point
+after duplicating the line.
 
 ---
 ** Files with the ".eld" extension are now visited in 'lisp-data-mode'.
diff --git a/lisp/misc.el b/lisp/misc.el
index ca013d5f72f..898fe9dd168 100644
--- a/lisp/misc.el
+++ b/lisp/misc.el
@@ -63,21 +63,42 @@ copy-from-above-command
 				 (+ n (point)))))))
     (insert string)))
 
+(defcustom duplicate-line-final-position 0
+  "Where to put point after duplicating the line with `duplicate-line'.
+When 0, leave point on the original line.
+When 1, move point to the first new line.
+When -1, move point to the last new line.
+The same column is preserved after moving to a new line."
+  :type '(choice (const :tag "Leave point on old line" 0)
+                 (const :tag "Move point to first new line" 1)
+                 (const :tag "Move point to last new line" -1)
+                 (integer))
+  :group 'editing
+  :version "29.1")
+
 ;;;###autoload
 (defun duplicate-line (&optional n)
   "Duplicate the current line N times.
 Interactively, N is the prefix numeric argument, and defaults to 1.
+The user option `duplicate-line-final-position' specifies where to
+move point after duplicating the line.
 Also see the `copy-from-above-command' command."
   (interactive "p")
   (unless n
     (setq n 1))
-  (let ((line (buffer-substring (line-beginning-position) (line-end-position))))
-    (save-excursion
-      (forward-line 1)
-      (unless (bolp)
-        (insert "\n"))
-      (dotimes (_ n)
-        (insert line "\n")))))
+  (let ((line (buffer-substring (line-beginning-position) (line-end-position)))
+        (pos (point))
+        (col (current-column)))
+    (forward-line 1)
+    (unless (bolp)
+      (insert "\n"))
+    (dotimes (_ n)
+      (insert line "\n"))
+    (unless (< duplicate-line-final-position 0)
+      (goto-char pos))
+    (unless (eq duplicate-line-final-position 0)
+      (forward-line duplicate-line-final-position)
+      (move-to-column col))))
 
 (declare-function rectangle--duplicate-right "rect" (n))
 
diff --git a/test/lisp/misc-tests.el b/test/lisp/misc-tests.el
index f1d22e099b9..ea27ea1653b 100644
--- a/test/lisp/misc-tests.el
+++ b/test/lisp/misc-tests.el
@@ -88,6 +88,20 @@ misc--duplicate-line
     (duplicate-line 2)
     (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
     (should (equal (point) 7)))
+  ;; Duplicate a line (twice) and move point to the first duplicated line.
+  (with-temp-buffer
+    (insert "abc\ndefg\nh\n")
+    (goto-char 7)
+    (let ((duplicate-line-final-position 1)) (duplicate-line 2))
+    (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
+    (should (equal (point) 12)))
+  ;; Duplicate a line (twice) and move point to the last duplicated line.
+  (with-temp-buffer
+    (insert "abc\ndefg\nh\n")
+    (goto-char 7)
+    (let ((duplicate-line-final-position -1)) (duplicate-line 2))
+    (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
+    (should (equal (point) 17)))
   ;; Duplicate a non-terminated line.
   (with-temp-buffer
     (insert "abc")

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

23 juni 2023 kl. 18.45 skrev Juri Linkov <juri@linkov.net>:

> It would be a major undertaking to design
> a similar option for `duplicate-dwim` for consistency with `duplicate-line`
> because of more decisions necessary to make: whether to move point
> to the beginning or the end of the copied regions, whether to select
> all the copied regions or only one of them, etc.

Actually we are helped by some constraints: duplicate-dwim must keep the region active afterwards, so that the user can run the same command immediately and get another copy. This is important for usability.

It seems reasonable to assume that a user wants the same behaviour in `duplicate-dwim` whether it is lines or columns being duplicated, and have a single variable controlling both. This assumption may be wrong: perhaps a user prefers to stay when copying a line, but move when copying columns?

If we use separate settings they should naturally work the same way, or users will wonder who is running the asylum.

A perhaps more intuitive way of describing the behaviour is not where to put point afterwards, but whether copies are inserted before or after the original. That will generalise to duplication of regions, rectangular or contiguous, a little better.

It's also easy to describe:

(defcustom duplicate-direction 'after
  "Where to insert the copies made by `duplicate-line' and `duplicate-dwim'.
Valid values are `after' and `before' (the original text).
  :group 'editing
  :type '(choice (const :tag "After original" after)
                 (const :tag "Before original" before)))

What do you think? Should we have a single setting for lines and regions, or individual settings for lines and regions, or for lines, rectangles and contiguous regions?

> For users who know Emacs Lisp and like to write plain values in ~/.emacs,
> the values 0, 1, -1 are more intuitive and familiar due to functions
> like `substring`, etc.

I suppose I'm one of those who don't know Emacs Lisp then!

> to my surprise the shortdoc of `substring` has no examples
> of a negative argument.  I don't know whether this omission
> is intentional to make the string section shorter.

Thank you for noticing this. I doubt it was intentional.
The examples have now been extended a bit.
I'm sure we could do better with examples in general if we were to make an effort.

bug#64185: proposal for new function: copy-line

[Zachary Kanfer]

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

>  And I agree that allowing any integer as value isn't necessary the best
choice here. It's a false flexibility; nobody will ever set it to something
other than 0, 1 or -1. Symbols would probably be better, for several
reasons.

I agree. The reasons I can think of are as follows:

1. These values will be read far more often than they are written. A symbol
like 'last is somewhat self-documenting. On the other hand, what is 1? Is
it the old line, because it's highest up on the screen? Is it the last new
line, because the last new line has the greatest point, and 1 is the
greatest of the options? (In the current implementation, it's the first new
line. Did you know that until I mentioned it?) The values have to be
recalled each time they are used.
2. These settings aren't really integers. We won't want to add, multiply,
subtract or divide them.
3. Using integers leads to more complicated code. The current patch has a
line (unless (< duplicate-line-final-position 0) ...). I believe this would
be easier to read as (unless (equal duplicate-line-final-position 'last)
...).

On Fri, Jun 23, 2023 at 5:08 AM Mattias Engdegård <
mattias.engdegard@gmail.com> wrote:

> >  'duplicate-dwim' duplicates the region if it is active.  If not, it
> >  works like 'duplicate-line'.  An active rectangular region is
> > -duplicated on its right-hand side.
> > +duplicated on its right-hand side.  The new user option
> > +'duplicate-line-final-position' specifies where to move point
> > +after duplicating the line.
>
> This makes it unclear to what extent the variable affects
> `duplicate-dwim`. (For some reason it only does so when the region is
> inactive, which doesn't seem right.)
>
> And I agree that allowing any integer as value isn't necessary the best
> choice here. It's a false flexibility; nobody will ever set it to something
> other than 0, 1 or -1. Symbols would probably be better, for several
> reasons.
>
> Finally, it's a bit surprising that this is even discussed for inclusion
> in Emacs 29 at this stage, given the raw state of the design. What about we
> do a proper job on master instead, instead of rushing a half-baked new
> feature into a branch that is already deep into pre-release?
>
>

[-- Attachment #2: Type: text/html, Size: 2711 bytes --]

bug#64185: proposal for new function: copy-line

[Juri Linkov]

> 1. These values will be read far more often than they are written. A symbol
> like 'last is somewhat self-documenting. On the other hand, what is 1? Is
> it the old line, because it's highest up on the screen? Is it the last new
> line, because the last new line has the greatest point, and 1 is the
> greatest of the options? (In the current implementation, it's the first new
> line. Did you know that until I mentioned it?) The values have to be
> recalled each time they are used.
> 2. These settings aren't really integers. We won't want to add, multiply,
> subtract or divide them.
> 3. Using integers leads to more complicated code. The current patch has a
> line (unless (< duplicate-line-final-position 0) ...). I believe this would
> be easier to read as (unless (equal duplicate-line-final-position 'last) .
> ..).

This might surprise you, but code would be more complicated with symbols.
Instead of

  (unless (eq duplicate-line-final-position 0)
    (forward-line duplicate-line-final-position)

it will be

  (unless (eq duplicate-line-final-position 'old-original-line)
    (when (eq duplicate-line-final-position 'first-copied-duplicate-line)
      (forward-line 1))
    (when (eq duplicate-line-final-position 'last-copied-duplicate-line)
      (forward-line -1))

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> It would be a major undertaking to design
>> a similar option for `duplicate-dwim` for consistency with `duplicate-line`
>> because of more decisions necessary to make: whether to move point
>> to the beginning or the end of the copied regions, whether to select
>> all the copied regions or only one of them, etc.
>
> Actually we are helped by some constraints: duplicate-dwim must keep
> the region active afterwards, so that the user can run the same
> command immediately and get another copy.  This is important for
> usability.

I don't see from where we can get such assumption - e.g. extending the region
to the copied text also makes sense.

> It seems reasonable to assume that a user wants the same behaviour in
> `duplicate-dwim` whether it is lines or columns being duplicated, and
> have a single variable controlling both.  This assumption may be
> wrong: perhaps a user prefers to stay when copying a line, but move
> when copying columns?

When the users will request a setting for columns, then we could
add a new option e.g. `duplicate-dwim-final-column`.

> If we use separate settings they should naturally work the same way,
> or users will wonder who is running the asylum.

It would be crazy to mix apples and oranges.  `duplicate-line-final-line`
should be applied only to `duplicate-line`.  And if the users need to
do the same with regions, then a separate option could be added
e.g. `duplicate-dwim-final-region`.

> A perhaps more intuitive way of describing the behaviour is not where
> to put point afterwards, but whether copies are inserted before or
> after the original.  That will generalise to duplication of regions,
> rectangular or contiguous, a little better.
>
> It's also easy to describe:
>
> (defcustom duplicate-direction 'after
>   "Where to insert the copies made by `duplicate-line' and `duplicate-dwim'.
> Valid values are `after' and `before' (the original text).
>   :group 'editing
>   :type '(choice (const :tag "After original" after)
>                  (const :tag "Before original" before)))
>
> What do you think?  Should we have a single setting for lines and
> regions, or individual settings for lines and regions, or for lines,
> rectangles and contiguous regions?

My first implementation of `duplicate-line` prepended the copied line
before the original line, and that implementation was flawed: it had
no visual feedback when copying the first line at the top of the window.

>> to my surprise the shortdoc of `substring` has no examples
>> of a negative argument.  I don't know whether this omission
>> is intentional to make the string section shorter.
>
> Thank you for noticing this. I doubt it was intentional.
> The examples have now been extended a bit.
> I'm sure we could do better with examples in general if we were to make an effort.

Thanks.

(Not sure why now one example uses boring "abcde"
instead of the overall style with "foo", "bar", "zot").

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

25 juni 2023 kl. 19.24 skrev Juri Linkov <juri@linkov.net>:

> I don't see from where we can get such assumption - e.g. extending the region
> to the copied text also makes sense.

No, it's actually a firm requirement. Extending the region would only allow making exponentially more copies which is useless in practice. Try it yourself and you'll see.

> When the users will request a setting for columns, then we could
> add a new option e.g. `duplicate-dwim-final-column`.

Request submitted then!

>> If we use separate settings they should naturally work the same way,
>> or users will wonder who is running the asylum.
> 
> It would be crazy to mix apples and oranges.  `duplicate-line-final-line`
> should be applied only to `duplicate-line`.

No, what I meant is that it would make little sense to use different types and values for settings affecting the exact same operation on different pieces of text.

>  And if the users need to
> do the same with regions, then a separate option could be added
> e.g. `duplicate-dwim-final-region`.

How about we, for a change, attempt this thing called design, instead of the customary Emacs piecemeal hodgepodge?

> (Not sure why now one example uses boring "abcde"
> instead of the overall style with "foo", "bar", "zot").

Yes there are oodles of those so I just fix the ones in examples I'm changing for other reasons.
Maybe we should take a day off and do them all in one batch.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> I don't see from where we can get such assumption - e.g. extending
>> the region to the copied text also makes sense.
>
> No, it's actually a firm requirement.  Extending the region would only
> allow making exponentially more copies which is useless in practice.
> Try it yourself and you'll see.

I remember already the need of doing this several times: to create
initial content for testing first I select one line then duplicate
its region, then select the whole buffer and duplicate it again
and so on, so the duplicated region grows exponentially.

>> When the users will request a setting for columns, then we could
>> add a new option e.g. `duplicate-dwim-final-column`.
>
> Request submitted then!

`duplicate-dwim-final-column` should be applied to both cases?
To rectangular regions and to lines?  In case of lines it could
define whether to keep the original column, or move to the
beginning of the line, or to the end of the line?

>> And if the users need to do the same with regions, then a separate
>> option could be added e.g. `duplicate-dwim-final-region`.
>
> How about we, for a change, attempt this thing called design, instead
> of the customary Emacs piecemeal hodgepodge?

Then I think `duplicate-dwim` is a misnomer, and it was misdesigned.
A proper name that matches the design of `thing-at-point`, would be
`duplicate-thing`.  thingatpt.el contains the definition of the region
among many other things that are candidates for duplication.

Since the design of a general thing-duplication command
is still under development, maybe it would be appropriate
to remove `duplicate-dwim` from emacs-29, and continue
its development in master?

bug#64185: proposal for new function: copy-line

[Drew Adams]

> Then I think `duplicate-dwim` is a misnomer, and it was misdesigned.

FWIW/OT:

This is pretty much the case for all commands named `*-dwim'.

In practice, DWIM doesn't mean "do what I mean".  It means something more like "do what its author wants" or even "do something its author couldn't describe well, let alone name well".

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  64185@debbugs.gnu.org,
>   me@eshelyaron.com,  zkanfer@gmail.com
> Date: Mon, 26 Jun 2023 20:37:10 +0300
> 
> Since the design of a general thing-duplication command
> is still under development, maybe it would be appropriate
> to remove `duplicate-dwim` from emacs-29, and continue
> its development in master?

No, we are not removing any commands from emacs-29 at this time.  If
you plan incompatible changes in duplicate-dwim in the future, I'm
prepared to consider changes to it to make those future changes less
incompatible, but I won't agree to removing it.  This discussion
should have happened a year ago, and now it's too late for such
proposals.  Sorry.

bug#64185: proposal for new function: copy-line

[Zachary Kanfer]

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

 > > The code becomes slightly more verbose this way, but is far easier to
read
> > and comprehend. It seems an extremely worthwhile tradeoff to me.
>
> Not sure how often users will read the code of duplicate-line.

Developers will. And it's easier to read with symbols.

On Mon, Jun 26, 2023 at 1:41 PM Juri Linkov <juri@linkov.net> wrote:

> >> This might surprise you, but code would be more complicated with
> symbols.
> >> Instead of
> >>
> >>   (unless (eq duplicate-line-final-position 0)
> >>     (forward-line duplicate-line-final-position)
> >>
> >> it will be
> >>
> >>   (unless (eq duplicate-line-final-position 'old-original-line)
> >>     (when (eq duplicate-line-final-position
> 'first-copied-duplicate-line)
> >>       (forward-line 1))
> >>     (when (eq duplicate-line-final-position 'last-copied-duplicate-line)
> >>       (forward-line -1))
> >
> > The code doesn't need the (unless (eq duplicate-line-final-position
> > 'old-original-line) ... check. It can be only the two when cases. But
> > that's a secondary matter.
>
> I agree.
>
> > The code becomes slightly more verbose this way, but is far easier to
> read
> > and comprehend. It seems an extremely worthwhile tradeoff to me.
>
> Not sure how often users will read the code of duplicate-line.
>

[-- Attachment #2: Type: text/html, Size: 1873 bytes --]

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> Cc: 64185@debbugs.gnu.org
> From: Zachary Kanfer <zkanfer@gmail.com>
> Date: Mon, 26 Jun 2023 15:18:19 -0400
> 
> > > The code becomes slightly more verbose this way, but is far easier to read
> > > and comprehend. It seems an extremely worthwhile tradeoff to me.
> > 
> > Not sure how often users will read the code of duplicate-line.
> 
> Developers will. And it's easier to read with symbols.

The advantage of symbols for reading the code is very minor, so the
other considerations, in particular the ease of changing the value
interactively, prevail.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

26 juni 2023 kl. 19.37 skrev Juri Linkov <juri@linkov.net>:

> I remember already the need of doing this several times: to create
> initial content for testing first I select one line then duplicate
> its region, then select the whole buffer and duplicate it again
> and so on, so the duplicated region grows exponentially.

Yes, but there is no reason to do that now. Just use duplicate-dwim (or duplicate-line).
Both benefit a lot from immediate repeatability.

> `duplicate-dwim-final-column` should be applied to both cases?
> To rectangular regions and to lines?  In case of lines it could
> define whether to keep the original column, or move to the
> beginning of the line, or to the end of the line?

No, I meant the same as you were proposing for lines: whether to insert copies before or after the original (or where point and the region should be afterwards, but that's pretty much the same thing as long as we preserve window-start, as you correctly pointed out).

> Then I think `duplicate-dwim` is a misnomer, and it was misdesigned.
> A proper name that matches the design of `thing-at-point`, would be
> `duplicate-thing`.  thingatpt.el contains the definition of the region
> among many other things that are candidates for duplication.

That is a very different operation. `duplicate-dwim` acts on a buffer area designated by the user regardless of its content. The 'thing' functions (there seem to be no commands) parse buffer contents to derive a value or extent.

I don't like the 'dwim' name part either but it has some precedence. A single function should suffice but since this is Emacs, we have two of them for historical reasons before anything is even released.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> I remember already the need of doing this several times: to create
>> initial content for testing first I select one line then duplicate
>> its region, then select the whole buffer and duplicate it again
>> and so on, so the duplicated region grows exponentially.
>
> Yes, but there is no reason to do that now.  Just use duplicate-dwim
> (or duplicate-line).  Both benefit a lot from immediate repeatability.

I agree it's much easier to do this with the new commands now.
(The only consideration is performance, but probably not worth
making the code more complex by increasing the size of copied region
exponentially to avoid inserting a large amount of small chunks one by one.)

> I don't like the 'dwim' name part either but it has some precedence.
> A single function should suffice but since this is Emacs, we have two
> of them for historical reasons before anything is even released.

I see no problem in having two commands, since most users expect a standard
command to duplicate lines, and a more general dwim command is a bonus
for more advanced uses.  So AFAIC, a separate option for duplicate-line
is a must, and an option for duplicate-dwim is a nice-to-have.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

27 juni 2023 kl. 20.28 skrev Juri Linkov <juri@linkov.net>:

> (The only consideration is performance, but probably not worth
> making the code more complex by increasing the size of copied region
> exponentially to avoid inserting a large amount of small chunks one by one.)

We can certainly do better. I just pushed another 10× improvement, on top of the previous performance doubling for duplicate-line.

> I see no problem in having two commands, since most users expect a standard
> command to duplicate lines, and a more general dwim command is a bonus
> for more advanced uses.  So AFAIC, a separate option for duplicate-line
> is a must, and an option for duplicate-dwim is a nice-to-have.

That duplicate-dwim is somehow 'advanced' is a misconception: it's just more useful, and definitely not harder to use.
I don't believe in hiding useful functionality from people by intentionally making them difficult to access.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> (The only consideration is performance, but probably not worth
>> making the code more complex by increasing the size of copied region
>> exponentially to avoid inserting a large amount of small chunks one by one.)
>
> We can certainly do better.  I just pushed another 10× improvement, on
> top of the previous performance doubling for duplicate-line.

Thanks, this is a nice improvement.  Too bad you did this on master.
This means I can't push the patch that Eli approved to emacs-29 anymore,
because whoever will merge emacs-29 to master next time will be forced
to resolve the merge conflict, and this is not a trivial task.

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  64185@debbugs.gnu.org,
>   me@eshelyaron.com,  zkanfer@gmail.com
> Date: Wed, 28 Jun 2023 20:42:10 +0300
> 
> > We can certainly do better.  I just pushed another 10× improvement, on
> > top of the previous performance doubling for duplicate-line.
> 
> Thanks, this is a nice improvement.  Too bad you did this on master.
> This means I can't push the patch that Eli approved to emacs-29 anymore,
> because whoever will merge emacs-29 to master next time will be forced
> to resolve the merge conflict, and this is not a trivial task.

You can install on emacs-29 with a commit log message which says
"don't merge to master".  Then Someone™ will have to do the equivalent
fix on master.

bug#64185: proposal for new function: copy-line

[Juri Linkov]

>> > We can certainly do better.  I just pushed another 10× improvement, on
>> > top of the previous performance doubling for duplicate-line.
>>
>> Thanks, this is a nice improvement.  Too bad you did this on master.
>> This means I can't push the patch that Eli approved to emacs-29 anymore,
>> because whoever will merge emacs-29 to master next time will be forced
>> to resolve the merge conflict, and this is not a trivial task.
>
> You can install on emacs-29 with a commit log message which says
> "don't merge to master".  Then Someone™ will have to do the equivalent
> fix on master.

This means more work.  Ok, done.

I don't know if this can be closed now, since a new option for
duplicate-dwim could be created here as well.

Then e.g. duplicate-dwim-final-position could inherit its default value
from duplicate-line-final-position.  And here more general values 0, 1, -1
become an advantage since inherited symbol values such as 'last-new-line'
would not make sense for the region part of duplicate-dwim.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

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

29 juni 2023 kl. 09.13 skrev Juri Linkov <juri@linkov.net>:

> I don't know if this can be closed now, since a new option for
> duplicate-dwim could be created here as well.

Indeed, and here it is. It also fixes a bug when duplicating some rectangles, which arose from changes in Emacs 29.

There is only a single variable for both contiguous and rectangular regions; maybe we should have them separate -- not sure.

Since this is intended for emacs-29, I'm posting it here for approval first.


[-- Attachment #2: 0001-Add-duplicate-region-final-position-bug-64185.patch --]
[-- Type: application/octet-stream, Size: 9289 bytes --]

From c7051500cbfda75a2a218c323da3715cbf7263cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Fri, 30 Jun 2023 18:34:10 +0200
Subject: [PATCH] Add duplicate-region-final-position (bug#64185)

* lisp/misc.el (duplicate-region-final-position): New defcustom.
(duplicate-dwim): Use it.
* lisp/rect.el (rectangle--duplicate-right): Add displacement
argument.  Adjust for changes in Emacs 29.
* test/lisp/misc-tests.el (misc--duplicate-dwim):
Extend test.
---
 lisp/misc.el            | 37 +++++++++++++---
 lisp/rect.el            | 25 +++++++----
 test/lisp/misc-tests.el | 97 +++++++++++++++++++++++++++--------------
 3 files changed, 112 insertions(+), 47 deletions(-)

diff --git a/lisp/misc.el b/lisp/misc.el
index 52df33911f7..c50eec9156e 100644
--- a/lisp/misc.el
+++ b/lisp/misc.el
@@ -100,7 +100,18 @@ duplicate-line
       (forward-line duplicate-line-final-position)
       (move-to-column col))))
 
-(declare-function rectangle--duplicate-right "rect" (n))
+(defcustom duplicate-region-final-position 0
+  "Where the region ends up after duplicating a region with `duplicate-dwim'.
+When 0, leave the region in place.
+When 1, put the region around the first copy.
+When -1, put the region around the last copy."
+  :type '(choice (const :tag "Leave region in place" 0)
+                 (const :tag "Put region around first copy" 1)
+                 (const :tag "Put region around last copy" -1))
+  :group 'editing
+  :version "29.1")
+
+(declare-function rectangle--duplicate-right "rect" (n displacement))
 
 ;; `duplicate-dwim' preserves an active region and changes the buffer
 ;; outside of it: disregard the region when immediately undoing the
@@ -113,25 +124,41 @@ duplicate-dwim
 If the region is inactive, duplicate the current line (like `duplicate-line').
 Otherwise, duplicate the region, which remains active afterwards.
 If the region is rectangular, duplicate on its right-hand side.
-Interactively, N is the prefix numeric argument, and defaults to 1."
+Interactively, N is the prefix numeric argument, and defaults to 1.
+The variables `duplicate-line-final-position' and
+`duplicate-region-final-position' control the position of point
+and the region after the duplication."
   (interactive "p")
   (unless n
     (setq n 1))
   (cond
+   ((<= n 0) nil)
    ;; Duplicate rectangle.
    ((bound-and-true-p rectangle-mark-mode)
-    (rectangle--duplicate-right n)
+    (rectangle--duplicate-right n
+                                (if (< duplicate-region-final-position 0)
+                                    n
+                                  duplicate-region-final-position))
     (setq deactivate-mark nil))
 
    ;; Duplicate (contiguous) region.
    ((use-region-p)
     (let* ((beg (region-beginning))
            (end (region-end))
-           (text (buffer-substring beg end)))
+           (text (buffer-substring beg end))
+           (pt (point))
+           (mk (mark)))
       (save-excursion
         (goto-char end)
         (dotimes (_ n)
-          (insert text))))
+          (insert text)))
+      (let* ((displace (if (< duplicate-region-final-position 0)
+                           n
+                         duplicate-region-final-position))
+             (d (* displace (- end beg))))
+        (unless (zerop d)
+          (push-mark (+ mk d))
+          (goto-char (+ pt d)))))
     (setq deactivate-mark nil))
 
    ;; Duplicate line.
diff --git a/lisp/rect.el b/lisp/rect.el
index 5ff821abb3f..8dc188b1de0 100644
--- a/lisp/rect.el
+++ b/lisp/rect.el
@@ -930,8 +930,9 @@ rectangle--unhighlight-for-redisplay
     (mapc #'delete-overlay (nthcdr 5 rol))
     (setcar (cdr rol) nil)))
 
-(defun rectangle--duplicate-right (n)
-  "Duplicate the rectangular region N times on the right-hand side."
+(defun rectangle--duplicate-right (n displacement)
+  "Duplicate the rectangular region N times on the right-hand side.
+Leave the region moved DISPLACEMENT region-wide steps to the right."
   (let ((cols (rectangle--pos-cols (point) (mark))))
     (apply-on-rectangle
      (lambda (startcol endcol)
@@ -940,16 +941,22 @@ rectangle--duplicate-right
          (move-to-column endcol t)
          (dotimes (_ n)
            (insert (cadr lines)))))
-     (region-beginning) (region-end))
-    ;; Recompute the rectangle state; no crutches should be needed now.
-    (let ((p (point))
-          (m (mark)))
+     (min (point) (mark))
+     (max (point) (mark)))
+    ;; Recompute the rectangle state.
+    (let* ((p (point))
+           (m (mark))
+           (point-col (car cols))
+           (mark-col (cdr cols))
+           (d (* displacement (abs (- point-col mark-col)))))
       (rectangle--reset-crutches)
       (goto-char m)
-      (move-to-column (cdr cols) t)
-      (set-mark (point))
+      (move-to-column (+ mark-col d) t)
+      (if (= d 0)
+          (set-mark (point))
+        (push-mark (point)))
       (goto-char p)
-      (move-to-column (car cols) t))))
+      (move-to-column (+ point-col d) t))))
 
 (provide 'rect)
 
diff --git a/test/lisp/misc-tests.el b/test/lisp/misc-tests.el
index ea27ea1653b..b9bafe4bd11 100644
--- a/test/lisp/misc-tests.el
+++ b/test/lisp/misc-tests.el
@@ -24,6 +24,7 @@
 ;;; Code:
 
 (require 'ert)
+(require 'misc)
 
 (defmacro with-misc-test (original result &rest body)
   (declare (indent 2))
@@ -113,40 +114,70 @@ misc--duplicate-line
 (require 'rect)
 
 (ert-deftest misc--duplicate-dwim ()
-  ;; Duplicate a line.
-  (with-temp-buffer
-    (insert "abc\ndefg\nh\n")
-    (goto-char 7)
-    (duplicate-dwim 2)
-    (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\nh\n"))
-    (should (equal (point) 7)))
+  (let ((duplicate-line-final-position 0)
+        (duplicate-region-final-position 0))
+    ;; Duplicate a line.
+    (dolist (final-pos '(0 -1 1))
+      (ert-info ((prin1-to-string final-pos) :prefix "final-pos: ")
+        (with-temp-buffer
+          (insert "abc\ndefg\nh\n")
+          (goto-char 7)
+          (let ((duplicate-line-final-position final-pos))
+            (duplicate-dwim 3))
+          (should (equal (buffer-string) "abc\ndefg\ndefg\ndefg\ndefg\nh\n"))
+          (let ((delta (* 5 (if (< final-pos 0) 3 final-pos))))
+            (should (equal (point) (+ 7 delta)))))))
+
+    ;; Duplicate a region.
+    (dolist (final-pos '(0 -1 1))
+      (ert-info ((prin1-to-string final-pos) :prefix "final-pos: ")
+        (with-temp-buffer
+          (insert "abCDEFghi")
+          (set-mark 3)
+          (goto-char 7)
+          (transient-mark-mode)
+          (should (use-region-p))
+          (let ((duplicate-region-final-position final-pos))
+            (duplicate-dwim 3))
+          (should (equal (buffer-string) "abCDEFCDEFCDEFCDEFghi"))
+          (should (region-active-p))
+          (let ((delta (* 4 (if (< final-pos 0) 3 final-pos))))
+            (should (equal (point) (+ 7 delta)))
+            (should (equal (mark) (+ 3 delta)))))))
+
+    ;; Duplicate a rectangular region (sparse).
+    (with-temp-buffer
+      (insert "x\n>a\n>bcde\n>fg\nyz\n")
+      (goto-char 4)
+      (rectangle-mark-mode)
+      (goto-char 15)
+      (rectangle-forward-char 1)
+      (duplicate-dwim)
+      (should (equal (buffer-string) "x\n>a  a  \n>bcdbcde\n>fg fg \nyz\n"))
+      (should (equal (point) 24))
+      (should (region-active-p))
+      (should rectangle-mark-mode)
+      (should (equal (mark) 4)))
+
+    ;; Idem (dense).
+    (dolist (final-pos '(0 -1 1))
+      (ert-info ((prin1-to-string final-pos) :prefix "final-pos: ")
+        (with-temp-buffer
+          (insert "aBCd\neFGh\niJKl\n")
+          (goto-char 2)
+          (rectangle-mark-mode)
+          (goto-char 14)
+          (let ((duplicate-region-final-position final-pos))
+            (duplicate-dwim 3))
+          (should (equal (buffer-string)
+                         "aBCBCBCBCd\neFGFGFGFGh\niJKJKJKJKl\n"))
+          (should (region-active-p))
+          (should rectangle-mark-mode)
+          (let ((hdelta (* 2 (if (< final-pos 0) 3 final-pos)))
+                (vdelta 12))
+            (should (equal (point) (+ 14 vdelta hdelta)))
+            (should (equal (mark) (+ 2 hdelta)))))))))
 
-  ;; Duplicate a region.
-  (with-temp-buffer
-    (insert "abc\ndef\n")
-    (set-mark 2)
-    (goto-char 7)
-    (transient-mark-mode)
-    (should (use-region-p))
-    (duplicate-dwim)
-    (should (equal (buffer-string) "abc\ndebc\ndef\n"))
-    (should (equal (point) 7))
-    (should (region-active-p))
-    (should (equal (mark) 2)))
-
-  ;; Duplicate a rectangular region.
-  (with-temp-buffer
-    (insert "x\n>a\n>bcde\n>fg\nyz\n")
-    (goto-char 4)
-    (rectangle-mark-mode)
-    (goto-char 15)
-    (rectangle-forward-char 1)
-    (duplicate-dwim)
-    (should (equal (buffer-string) "x\n>a  a  \n>bcdbcde\n>fg fg \nyz\n"))
-    (should (equal (point) 24))
-    (should (region-active-p))
-    (should rectangle-mark-mode)
-    (should (equal (mark) 4))))
 
 (provide 'misc-tests)
 ;;; misc-tests.el ends here
-- 
2.32.0 (Apple Git-132)

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 30 Jun 2023 19:13:26 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  64185@debbugs.gnu.org,
>  me@eshelyaron.com,
>  zkanfer@gmail.com
> 
> 29 juni 2023 kl. 09.13 skrev Juri Linkov <juri@linkov.net>:
> 
> > I don't know if this can be closed now, since a new option for
> > duplicate-dwim could be created here as well.
> 
> Indeed, and here it is. It also fixes a bug when duplicating some rectangles, which arose from changes in Emacs 29.
> 
> There is only a single variable for both contiguous and rectangular regions; maybe we should have them separate -- not sure.
> 
> Since this is intended for emacs-29, I'm posting it here for approval first.

Sorry, this is too much for emacs-29.  Please either find a simpler
change for Emacs 29, or install on master.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

30 juni 2023 kl. 21.03 skrev Eli Zaretskii <eliz@gnu.org>:

> Sorry, this is too much for emacs-29.  Please either find a simpler
> change for Emacs 29, or install on master.

Simpler in what way, exactly? The bulk of the change is in tests.

It is the exact equivalent to Juri's change to duplicate-line, but that one actually had a bigger footprint; this change only affects duplicate-dwim and nothing else.

If you like, I could try splitting the change into smaller parts.

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Sat, 1 Jul 2023 10:45:35 +0200
> Cc: juri@linkov.net,
>  64185@debbugs.gnu.org,
>  me@eshelyaron.com,
>  zkanfer@gmail.com
> 
> 30 juni 2023 kl. 21.03 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > Sorry, this is too much for emacs-29.  Please either find a simpler
> > change for Emacs 29, or install on master.
> 
> Simpler in what way, exactly?

In a way that doesn't make any significant changes in
rectangle--duplicate-right, for example.

> The bulk of the change is in tests.

That's not the part which bothers me, obviously.

> It is the exact equivalent to Juri's change to duplicate-line, but that one actually had a bigger footprint; this change only affects duplicate-dwim and nothing else.

The difference is that (a) duplicate-line was a feature which was
explicitly requested by the bug report which caused its
implementation, and (b) the changes for duplicate-line were much
simpler and easier to reason about.

> If you like, I could try splitting the change into smaller parts.

I'm not sure I understand what would that accomplish.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

1 juli 2023 kl. 11.53 skrev Eli Zaretskii <eliz@gnu.org>:

> In a way that doesn't make any significant changes in
> rectangle--duplicate-right, for example.

That's just the part of duplicate-dwim that lives in rect.el.

> The difference is that (a) duplicate-line was a feature which was
> explicitly requested by the bug report which caused its
> implementation, and (b) the changes for duplicate-line were much
> simpler and easier to reason about.

They weren't really simpler, but that's not the point. It's about finishing a job properly.
I certainly understand the motivation behind the bug and see that it applies to duplicate-dwim as well.
We usually don't respond to requests by narrowly implementing just what the reporter asked about and nothing else.

bug#64185: proposal for new function: copy-line

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Sat, 1 Jul 2023 12:07:15 +0200
> Cc: juri@linkov.net,
>  64185@debbugs.gnu.org,
>  me@eshelyaron.com,
>  zkanfer@gmail.com
> 
> 1 juli 2023 kl. 11.53 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > In a way that doesn't make any significant changes in
> > rectangle--duplicate-right, for example.
> 
> That's just the part of duplicate-dwim that lives in rect.el.

I understand, but the changes are non-trivial, and I cannot see any
urgency to doing that on emacs-29.  We lived with the current
implementation for a year.

> > The difference is that (a) duplicate-line was a feature which was
> > explicitly requested by the bug report which caused its
> > implementation, and (b) the changes for duplicate-line were much
> > simpler and easier to reason about.
> 
> They weren't really simpler, but that's not the point. It's about finishing a job properly.
> I certainly understand the motivation behind the bug and see that it applies to duplicate-dwim as well.
> We usually don't respond to requests by narrowly implementing just what the reporter asked about and nothing else.

I don't object to installing your changes, I just don't want to risk
any more unnecessary breakage on the release branch.  My point about
duplicate-dwim is that it was added at our initiative, so extending it
could wait till Emacs 30.

bug#64185: proposal for new function: copy-line

[Mattias Engdegård]

1 juli 2023 kl. 12.22 skrev Eli Zaretskii <eliz@gnu.org>:

> I don't object to installing your changes, I just don't want to risk
> any more unnecessary breakage on the release branch.

There is no particular risk for that, but I'll make the changes to master instead.
(Except for two tiny fixes pushed to emacs-29, assuming they would qualify.)