bug#46339: 28.0.50; ERC likely bug with `erc-previous-command'

["J.P." <jp@neverwas.me>]

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

Emanuel Berg <moasenwood@zoho.eu> writes:

> what does that mean? `erc-next-command' works so a bug is
> perhaps likely.

I think it's expecting an erc-input struct object, as passed by the hook
erc-pre-send-functions.

Would something like the following work?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Accept-string-argument-in-erc-add-to-input-ring.patch --]
[-- Type: text/x-patch, Size: 5396 bytes --]

From a805a4e507e19eb6c68fdd57ea7d8972b270aeb9 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 8 Feb 2021 02:34:18 -0800
Subject: [PATCH] Accept string argument in erc-add-to-input-ring

* lisp/erc/erc-ring.el: (erc-add-to-input-ring)
(erc-previous-command): Use existing API to grab input.
* test/lisp/erc/erc-tests.el: (erc-ring-previous-command)
See (bug#46339).
---
 lisp/erc/erc-ring.el       | 18 ++++++-----
 test/lisp/erc/erc-tests.el | 64 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/lisp/erc/erc-ring.el b/lisp/erc/erc-ring.el
index 71a9f8ef3da..028ab1eead8 100644
--- a/lisp/erc/erc-ring.el
+++ b/lisp/erc/erc-ring.el
@@ -69,10 +69,13 @@ erc-input-ring-setup
     (setq erc-input-ring (make-ring comint-input-ring-size)))
   (setq erc-input-ring-index nil))
 
-(defun erc-add-to-input-ring (state)
-  "Add string S to the input ring and reset history position."
+(defun erc-add-to-input-ring (state-or-string)
+  "Add STATE-OR-STRING to input ring and reset history position.
+STATE-OR-STRING should be a string or an erc-input object."
   (unless erc-input-ring (erc-input-ring-setup))
-  (ring-insert erc-input-ring (erc-input-string state))
+  (ring-insert erc-input-ring (if (erc-input-p state-or-string)
+                                  (erc-input-string state-or-string)
+                                state-or-string)) ; string
   (setq erc-input-ring-index nil))
 
 (defun erc-clear-input-ring ()
@@ -101,11 +104,10 @@ erc-previous-command
       ;; area, push it on the history ring before moving back through
       ;; the input history, so it will be there when we return to the
       ;; front.
-      (if (null erc-input-ring-index)
-          (when (> (point-max) erc-input-marker)
-            (erc-add-to-input-ring (buffer-substring erc-input-marker
-                                                     (point-max)))
-            (setq erc-input-ring-index 0)))
+      (when (and (null erc-input-ring-index)
+                 (> (point-max) erc-input-marker))
+        (erc-add-to-input-ring (erc-user-input))
+        (setq erc-input-ring-index 0))
 
       (setq erc-input-ring-index (if erc-input-ring-index
                                      (ring-plus1 erc-input-ring-index
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 26e14b98e91..d13397274aa 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -23,6 +23,7 @@
 
 (require 'ert)
 (require 'erc)
+(require 'erc-ring)
 
 (ert-deftest erc--read-time-period ()
   (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "")))
@@ -45,3 +46,66 @@ erc--read-time-period
 
   (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "1d")))
     (should (equal (erc--read-time-period "foo: ") 86400))))
+
+(ert-deftest erc-ring-previous-command-base-case ()
+  (ert-info ("Create ring when nonexistent and do nothing")
+    (let (erc-input-ring
+          erc-input-ring-index)
+      (erc-previous-command)
+      (should (ring-p erc-input-ring))
+      (should (zerop (ring-length erc-input-ring)))
+      (should-not erc-input-ring-index)))
+  (should-not erc-input-ring))
+
+(ert-deftest erc-ring-previous-command ()
+  (with-current-buffer (get-buffer-create "*#fake*")
+    (erc-mode)
+    (insert "\n\n")
+    (setq erc-input-marker (make-marker) ; these are all local
+          erc-insert-marker (make-marker)
+          erc-send-completed-hook nil)
+    (set-marker erc-insert-marker (point-max))
+    (erc-display-prompt)
+    (should (= (point) erc-input-marker))
+    (add-hook 'erc-pre-send-functions #'erc-add-to-input-ring nil t)
+    ;;
+    (cl-letf (((symbol-function 'erc-process-input-line)
+               (lambda (&rest _)
+                 (insert-before-markers
+                  (erc-display-message-highlight 'notice "echo: one\n"))))
+              ((symbol-function 'erc-command-no-process-p)
+               (lambda (&rest _) t)))
+      (ert-info ("Create ring, populate, recall")
+        (insert "/one")
+        (erc-send-current-line)
+        (should (ring-p erc-input-ring))
+        (should (zerop (ring-member erc-input-ring "/one"))) ; equal
+        (should (save-excursion (forward-line -1) (goto-char (point-at-bol))
+                                (looking-at-p "[*]+ echo: one")))
+        (should-not erc-input-ring-index)
+        (erc-bol)
+        (should (looking-at "$"))
+        (erc-previous-command)
+        (erc-bol)
+        (should (looking-at "/one"))
+        (should (zerop erc-input-ring-index)))
+      (ert-info ("Back to one")
+        (should (= (ring-length erc-input-ring) (1+ erc-input-ring-index)))
+        (erc-previous-command)
+        (should-not erc-input-ring-index)
+        (erc-bol)
+        (should (looking-at "$"))
+        (should (equal (ring-ref erc-input-ring 0) "/one")))
+      (ert-info ("Swap input after prompt with previous (#bug46339)")
+        (insert "abc")
+        (erc-previous-command)
+        (should (= 1 erc-input-ring-index))
+        (erc-bol)
+        (should (looking-at "/one"))
+        (should (equal (ring-ref erc-input-ring 0) "abc"))
+        (should (equal (ring-ref erc-input-ring 1) "/one"))
+        (erc-next-command)
+        (erc-bol)
+        (should (looking-at "abc")))))
+  (when noninteractive
+    (kill-buffer "*#fake*")))
-- 
2.29.2