unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 71355@debbugs.gnu.org, stefankangas@gmail.com
Subject: bug#71355: 30.0.50; [PATCH] Improve performance of buffered output in Eshell
Date: Fri, 7 Jun 2024 21:25:16 -0700	[thread overview]
Message-ID: <dd14cfe2-0f15-70ae-9e70-b75873544f9d@gmail.com> (raw)
In-Reply-To: <066f89aa-4026-b5fd-e529-12e70360bed6@gmail.com>

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

Whoops, one of the intermediate changes I'd made while working on these 
patches broke a bunch of the regression tests, so here's the fixed 
version. I also renamed a few of the new variables to be clearer that 
they're for internal use only.

Since it sounds like all the other concerns have been addressed, I'll 
probably merge this in the next day or so. (This is the last non-bugfix 
change I'd like to land for Eshell in Emacs 30.)

[-- Attachment #2: 0001-Be-more-efficient-when-buffering-output-in-Eshell.patch --]
[-- Type: text/plain, Size: 17643 bytes --]

From 8da40485bab3131577cbacd6f560c8d831e247d3 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 3 Jun 2024 22:01:48 -0700
Subject: [PATCH 1/2] Be more efficient when buffering output in Eshell

This makes the built-in 'eshell/cat' 5-10x faster on large files in my
(somewhat limited) tests.  In addition, this change periodically
redisplays when using the Eshell buffered output so that users can see
some progress.

* lisp/eshell/esh-io.el (eshell-print-queue-size, eshell-print-queue,
eshell-print-queue-count): Make obsolete in favor of...
(eshell-buffered-print-size, eshell--buffered-print-queue)
(eshell--buffered-print-current-size): ... these.
(eshell-buffered-print-redisplay-throttle): New user option.
(eshell--buffered-print-next-redisplay): New variable.
(eshell-init-print-buffer): Make obsolete.
(eshell-flush): Add new REDISPLAY-NOW argument in favor of CLEAR (which
only 'eshell-init-print-buffer' should have used).
(eshell-buffered-print): Compare queued output length to
'eshell--buffered-print-current-size'.
(eshell-with-buffered-print): New macro.

* lisp/eshell/esh-var.el (eshell/env):
* lisp/eshell/em-dirs.el (eshell/cd):
* lisp/eshell/em-hist.el (eshell/history):
* lisp/eshell/em-unix.el (eshell/cat):
* lisp/eshell/em-ls.el (eshell/ls): Use 'eshell-with-buffered-print'.
(flush-func): Remove.
(eshell-ls--insert-directory, eshell-do-ls): Remove 'flush-func'.

* test/lisp/eshell/em-unix-tests.el (em-unix-test/compile/interactive)
(em-unix-test/compile/pipeline, em-unix-test/compile/subcommand): Fix
indentation.
(em-unix-test/cat/file-output): New test.

* etc/NEWS: Announce these improvements.
---
 etc/NEWS                          |   7 ++
 lisp/eshell/em-dirs.el            |  13 ++--
 lisp/eshell/em-hist.el            |  13 ++--
 lisp/eshell/em-ls.el              |  14 ++--
 lisp/eshell/em-unix.el            |  25 ++++---
 lisp/eshell/esh-io.el             | 104 +++++++++++++++++++++++-------
 lisp/eshell/esh-var.el            |   7 +-
 test/lisp/eshell/em-unix-tests.el |  37 +++++++----
 8 files changed, 143 insertions(+), 77 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d6a8fa7122b..2349cc0cacb 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -963,6 +963,13 @@ files and deny read permission for users who are not members of the
 file's group.  See the Info node "(coreutils) File permissions" for
 more information on this notation.
 
+---
+*** Performance improvements for interactive output in Eshell.
+Interactive output in Eshell should now be significantly faster,
+especially for built-in commands that can print large amounts of output
+(e.g. "cat").  In addition, these commands can now update the display
+periodically to show their progress.
+
 +++
 *** New special reference type '#<marker POSITION BUFFER>'.
 This special reference type returns a marker at 'POSITION' in
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index a3d1a349540..e70f2cfe196 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -400,13 +400,12 @@ eshell/cd
 		(index 0))
 	    (if (= len 0)
 		(error "Directory ring empty"))
-	    (eshell-init-print-buffer)
-	    (while (< index len)
-	      (eshell-buffered-print
-	       (concat (number-to-string index) ": "
-		       (ring-ref eshell-last-dir-ring index) "\n"))
-	      (setq index (1+ index)))
-	    (eshell-flush)
+            (eshell-with-buffered-print
+              (while (< index len)
+                (eshell-buffered-print
+                 (concat (number-to-string index) ": "
+                         (ring-ref eshell-last-dir-ring index) "\n"))
+                (setq index (1+ index))))
 	    (setq handled t)))))
      (path
       (setq path (eshell-expand-multiple-dots path))))
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 8865cc745a3..9ffddfb611f 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -333,7 +333,6 @@ eshell-save-some-history
 
 (defun eshell/history (&rest args)
   "List in help buffer the buffer's input history."
-  (eshell-init-print-buffer)
   (eshell-eval-using-options
    "history" args
    '((?r "read" nil read-history
@@ -370,12 +369,12 @@ eshell/history
        (let* ((index (1- (or length (ring-length eshell-history-ring))))
 	      (ref (- (ring-length eshell-history-ring) index)))
 	 ;; We have to build up a list ourselves from the ring vector.
-	 (while (>= index 0)
-	   (eshell-buffered-print
-	    (format "%5d  %s\n" ref (eshell-get-history index)))
-	   (setq index (1- index)
-		 ref (1+ ref)))))))
-   (eshell-flush)
+         (eshell-with-buffered-print
+           (while (>= index 0)
+             (eshell-buffered-print
+              (format "%5d  %s\n" ref (eshell-get-history index)))
+             (setq index (1- index)
+                   ref (1+ ref))))))))
    nil))
 
 (defun eshell-put-history (input &optional ring at-beginning)
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 82d4b01393f..8bf2e20d320 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -229,7 +229,6 @@ block-size
 (defvar dereference-links)
 (defvar dir-literal)
 (defvar error-func)
-(defvar flush-func)
 (defvar human-readable)
 (defvar ignore-pattern)
 (defvar insert-func)
@@ -278,7 +277,6 @@ eshell-ls--insert-directory
           (require 'em-glob)
           (let* ((insert-func 'insert)
                  (error-func 'insert)
-                 (flush-func 'ignore)
                  (eshell-error-if-no-glob t)
                  (target ; Expand the shell wildcards if any.
                   (if (and (atom file)
@@ -324,10 +322,10 @@ eshell-ls--dired
 
 (defsubst eshell/ls (&rest args)
   "An alias version of `eshell-do-ls'."
-  (let ((insert-func 'eshell-buffered-print)
-	(error-func 'eshell-error)
-	(flush-func 'eshell-flush))
-    (apply 'eshell-do-ls args)))
+  (eshell-with-buffered-print
+    (let ((insert-func #'eshell-buffered-print)
+          (error-func #'eshell-error))
+      (apply 'eshell-do-ls args))))
 
 (put 'eshell/ls 'eshell-no-numeric-conversions t)
 (put 'eshell/ls 'eshell-filename-arguments t)
@@ -336,7 +334,6 @@ eshell/ls
 
 (defun eshell-do-ls (&rest args)
   "Implementation of \"ls\" in Lisp, passing ARGS."
-  (funcall flush-func -1)
   ;; Process the command arguments, and begin listing files.
   (eshell-eval-using-options
    "ls" (if eshell-ls-initial-args
@@ -422,8 +419,7 @@ eshell-do-ls
 		      (eshell-file-attributes
 		       arg (if numeric-uid-gid 'integer 'string))))
 	      args)
-      t (expand-file-name default-directory)))
-   (funcall flush-func)))
+      t (expand-file-name default-directory)))))
 
 (defsubst eshell-ls-printable-size (filesize &optional by-blocksize)
   "Return a printable FILESIZE."
diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index 4137c05fa41..e6bd0381a14 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -659,7 +659,6 @@ eshell/cat
 	  (if eshell-in-pipeline-p
 	      (error "Eshell's `cat' does not work in pipelines")
 	    (error "Eshell's `cat' cannot display one of the files given"))))
-    (eshell-init-print-buffer)
     (eshell-eval-using-options
      "cat" args
      '((?h "help" nil nil "show this usage screen")
@@ -672,18 +671,18 @@ eshell/cat
 	   (throw 'eshell-external
 		  (eshell-external-command "cat" args))))
      (let ((curbuf (current-buffer)))
-       (dolist (file args)
-	 (with-temp-buffer
-	   (insert-file-contents file)
-	   (goto-char (point-min))
-	   (while (not (eobp))
-	     (let ((str (buffer-substring
-			 (point) (min (1+ (line-end-position))
-				      (point-max)))))
-	       (with-current-buffer curbuf
-		 (eshell-buffered-print str)))
-	     (forward-line)))))
-     (eshell-flush))))
+       (eshell-with-buffered-print
+         (dolist (file args)
+	   (with-temp-buffer
+	     (insert-file-contents file)
+	     (goto-char (point-min))
+             (while (not (eobp))
+               (let* ((pos (min (+ (point) eshell-buffered-print-size)
+                                (point-max)))
+                      (str (buffer-substring (point) pos)))
+                 (with-current-buffer curbuf
+                   (eshell-buffered-print str))
+                 (goto-char pos))))))))))
 
 (put 'eshell/cat 'eshell-no-numeric-conversions t)
 (put 'eshell/cat 'eshell-filename-arguments t)
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 0fe177d4c60..9de9cc4509a 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -112,10 +112,30 @@ eshell-error-handle
 
 (defcustom eshell-print-queue-size 5
   "The size of the print queue, for doing buffered printing.
-This is basically a speed enhancement, to avoid blocking the Lisp code
-from executing while Emacs is redisplaying."
+This variable is obsolete.  You should use `eshell-buffered-print-size'
+instead."
   :type 'integer
   :group 'eshell-io)
+(make-obsolete-variable 'eshell-print-queue-size
+                        'eshell-buffered-print-size "30.1")
+
+(defcustom eshell-buffered-print-size 2048
+  "The size of the print queue in characters, for doing buffered printing.
+Larger values for this option will generally result in faster execution
+by reducing the overhead associated with each print operation, but will
+increase the time it takes to see any progress in the output; smaller
+values will do the reverse."
+  :type 'integer
+  :group 'eshell-io
+  :version "30.1")
+
+(defcustom eshell-buffered-print-redisplay-throttle 0.025
+  "The minimum time in seconds between redisplays when using buffered printing.
+If nil, don't redisplay while printing."
+  :type '(choice number
+                 (const :tag "Don't redisplay" nil))
+  :group 'eshell-io
+  :version "30.1")
 
 (defcustom eshell-virtual-targets
   '(;; The literal string "/dev/null" is intentional here.  It just
@@ -460,40 +480,74 @@ eshell-interactive-output-p
              (equal (caar (aref handles eshell-error-handle)) '(t)))
       (equal (caar (aref handles index)) '(t)))))
 
+(defvar eshell--buffered-print-queue nil)
+(defvar eshell--buffered-print-current-size nil)
+(defvar eshell--buffered-print-next-redisplay nil)
+
 (defvar eshell-print-queue nil)
+(make-obsolete-variable 'eshell-print-queue
+                        'eshell--buffered-print-queue "30.1")
 (defvar eshell-print-queue-count -1)
+(make-obsolete-variable 'eshell-print-queue-count
+                        'eshell--buffered-print-current-size "30.1")
 
 (defsubst eshell-print (object)
   "Output OBJECT to the standard output handle."
   (eshell-output-object object eshell-output-handle))
 
-(defun eshell-flush (&optional reset-p)
-  "Flush out any lines that have been queued for printing.
-Must be called before printing begins with -1 as its argument, and
-after all printing is over with no argument."
-  (ignore
-   (if reset-p
-       (setq eshell-print-queue nil
-	     eshell-print-queue-count reset-p)
-     (if eshell-print-queue
-	 (eshell-print eshell-print-queue))
-     (eshell-flush 0))))
-
 (defun eshell-init-print-buffer ()
   "Initialize the buffered printing queue."
-  (eshell-flush -1))
+  (declare (obsolete #'eshell-with-buffered-print "30.1"))
+  (setq eshell--buffered-print-queue nil
+        eshell--buffered-print-current-size 0))
+
+(defun eshell-flush (&optional redisplay-now)
+  "Flush out any text that has been queued for printing.
+When printing interactively, this will call `redisplay' every
+`eshell-buffered-print-redisplay-throttle' seconds so that the user can
+see the progress.  If REDISPLAY-NOW is non-nil, call `redisplay' for
+interactive output even if the throttle would otherwise prevent it."
+  (ignore
+   (when eshell--buffered-print-queue
+     (eshell-print (apply #'concat eshell--buffered-print-queue))
+     ;; When printing interactively (see `eshell-with-buffered-print'),
+     ;; periodically redisplay so the user can see some progress.
+     (when (and eshell--buffered-print-next-redisplay
+                (or redisplay-now
+                    (time-less-p eshell--buffered-print-next-redisplay
+                                 (current-time))))
+       (redisplay)
+       (setq eshell--buffered-print-next-redisplay
+             (time-add eshell--buffered-print-next-redisplay
+                       eshell-buffered-print-redisplay-throttle)))
+     (setq eshell--buffered-print-queue nil
+           eshell--buffered-print-current-size 0))))
 
 (defun eshell-buffered-print (&rest strings)
-  "A buffered print -- *for strings only*."
-  (if (< eshell-print-queue-count 0)
-      (progn
-	(eshell-print (apply 'concat strings))
-	(setq eshell-print-queue-count 0))
-    (if (= eshell-print-queue-count eshell-print-queue-size)
-	(eshell-flush))
-    (setq eshell-print-queue
-	  (concat eshell-print-queue (apply 'concat strings))
-	  eshell-print-queue-count (1+ eshell-print-queue-count))))
+  "A buffered print -- *for strings only*.
+When the buffer exceeds `eshell-buffered-print-size' in characters, this
+will flush it using `eshell-flush' (which see)."
+  (setq eshell--buffered-print-queue
+        (nconc eshell--buffered-print-queue strings))
+  (cl-incf eshell--buffered-print-current-size
+           (apply #'+ (mapcar #'length strings)))
+  (when (> eshell--buffered-print-current-size eshell-buffered-print-size)
+    (eshell-flush)))
+
+(defmacro eshell-with-buffered-print (&rest body)
+  "Initialize buffered printing for Eshell, and then evaluate BODY.
+Within BODY, call `eshell-buffered-print' to perform output."
+  (declare (indent 0))
+  `(let ((eshell--buffered-print-queue nil)
+         (eshell--buffered-print-current-size 0)
+         (eshell--buffered-print-next-redisplay
+          (when (and eshell-buffered-print-redisplay-throttle
+                     (eshell-interactive-output-p))
+            (time-add (current-time)
+                      eshell-buffered-print-redisplay-throttle))))
+     (unwind-protect
+         ,@body
+       (eshell-flush))))
 
 (defsubst eshell-error (object)
   "Output OBJECT to the standard error handle."
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 02b5c785625..f0270aca92c 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -437,10 +437,9 @@ eshell/env
    (if args
        (or (eshell-parse-local-variables args)
            (eshell-named-command (car args) (cdr args)))
-     (eshell-init-print-buffer)
-     (dolist (setting (sort (eshell-environment-variables) 'string-lessp))
-       (eshell-buffered-print setting "\n"))
-     (eshell-flush))))
+     (eshell-with-buffered-print
+       (dolist (setting (sort (eshell-environment-variables) 'string-lessp))
+         (eshell-buffered-print setting "\n"))))))
 
 (defun eshell-insert-envvar (envvar-name)
   "Insert ENVVAR-NAME into the current buffer at point."
diff --git a/test/lisp/eshell/em-unix-tests.el b/test/lisp/eshell/em-unix-tests.el
index a92c7d3f80a..2ee42c81333 100644
--- a/test/lisp/eshell/em-unix-tests.el
+++ b/test/lisp/eshell/em-unix-tests.el
@@ -26,10 +26,12 @@
 (require 'ert)
 (require 'em-unix)
 
+(eval-and-compile
+  (defvar this-directory (file-name-directory
+                          (or load-file-name default-directory))))
+
 (require 'eshell-tests-helpers
-         (expand-file-name "eshell-tests-helpers"
-                           (file-name-directory (or load-file-name
-                                                    default-directory))))
+         (expand-file-name "eshell-tests-helpers" this-directory))
 
 ;;; Tests:
 
@@ -37,11 +39,11 @@ em-unix-test/compile/interactive
   "Check that `eshell/compile' opens a compilation buffer interactively."
   (skip-unless (executable-find "echo"))
   (with-temp-eshell
-   (eshell-match-command-output "compile echo hello"
-                                "#<buffer \\*compilation\\*>")
-   (with-current-buffer "*compilation*"
-     (forward-line 3)
-     (should (looking-at "echo hello")))))
+    (eshell-match-command-output "compile echo hello"
+                                 "#<buffer \\*compilation\\*>")
+    (with-current-buffer "*compilation*"
+      (forward-line 3)
+      (should (looking-at "echo hello")))))
 
 (ert-deftest em-unix-test/compile/noninteractive ()
   "Check that `eshell/compile' writes to stdout noninteractively."
@@ -54,15 +56,26 @@ em-unix-test/compile/pipeline
   (skip-unless (and (executable-find "echo")
                     (executable-find "cat")))
   (with-temp-eshell
-   (eshell-match-command-output "compile echo hello | *cat"
-                                "\\`hello\n")))
+    (eshell-match-command-output "compile echo hello | *cat"
+                                 "\\`hello\n")))
 
 (ert-deftest em-unix-test/compile/subcommand ()
   "Check that `eshell/compile' writes to stdout from a subcommand."
   (skip-unless (and (executable-find "echo")
                     (executable-find "cat")))
   (with-temp-eshell
-   (eshell-match-command-output "echo ${compile echo hello}"
-                                "\\`hello\n")))
+    (eshell-match-command-output "echo ${compile echo hello}"
+                                 "\\`hello\n")))
+
+(ert-deftest em-unix-test/cat/file-output ()
+  "Check that `eshell/cat' can print a file's contents."
+  (with-temp-eshell
+    (let* ((this-file (expand-file-name "em-unix-tests.el" this-directory))
+           (contents (save-current-buffer
+                       (find-file this-file)
+                       (buffer-string))))
+      (eshell-match-command-output
+       (format "cat '%s'" (string-replace "'" "''" this-file))
+       (concat (regexp-quote contents))))))
 
 ;; em-unix-tests.el ends here
-- 
2.25.1


[-- Attachment #3: 0002-Improve-implementations-of-some-Eshell-output-filter.patch --]
[-- Type: text/plain, Size: 7465 bytes --]

From d797cbe58628166c0f1ca1bfce2c92a2b59a7b49 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 3 Jun 2024 22:06:49 -0700
Subject: [PATCH 2/2] Improve implementations of some Eshell output filter
 functions

* lisp/eshell/esh-mode.el (eshell-postoutput-scroll-to-bottom): Use
'get-buffer-window-list' for simplicity.
(eshell-handle-control-codes): Use 're-search-forward'; this way is much
faster.

* test/lisp/eshell/esh-mode-tests.el: New file.
---
 lisp/eshell/esh-mode.el            | 68 +++++++++++++-----------------
 test/lisp/eshell/esh-mode-tests.el | 62 +++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 38 deletions(-)
 create mode 100644 test/lisp/eshell/esh-mode-tests.el

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index e6f3cb5f6ad..ec1a07b7e2f 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -765,30 +765,25 @@ eshell-postoutput-scroll-to-bottom
 	 (current (current-buffer))
 	 (scroll eshell-scroll-to-bottom-on-output))
     (unwind-protect
-	(walk-windows
-         (lambda (window)
-           (if (eq (window-buffer window) current)
-               (progn
-                 (select-window window)
-                 (if (and (< (point) eshell-last-output-end)
-                          (or (eq scroll t) (eq scroll 'all)
-                              ;; Maybe user wants point to jump to end.
-                              (and (eq scroll 'this)
-                                   (eq selected window))
-                              (and (eq scroll 'others)
-                                   (not (eq selected window)))
-                              ;; If point was at the end, keep it at end.
-                              (>= (point) eshell-last-output-start)))
-                     (goto-char eshell-last-output-end))
-                 ;; Optionally scroll so that the text
-                 ;; ends at the bottom of the window.
-                 (if (and eshell-scroll-show-maximum-output
-                          (>= (point) eshell-last-output-end))
-                     (save-excursion
-                       (goto-char (point-max))
-                       (recenter -1)))
-                 (select-window selected))))
-	 nil t)
+        (dolist (window (get-buffer-window-list current nil t))
+          (with-selected-window window
+            (when (and (< (point) eshell-last-output-end)
+                       (or (eq scroll t) (eq scroll 'all)
+                           ;; Maybe user wants point to jump to end.
+                           (and (eq scroll 'this)
+                                (eq selected window))
+                           (and (eq scroll 'others)
+                                (not (eq selected window)))
+                           ;; If point was at the end, keep it at end.
+                           (>= (point) eshell-last-output-start)))
+              (goto-char eshell-last-output-end))
+            ;; Optionally scroll so that the text ends at the bottom of
+            ;; the window.
+            (when (and eshell-scroll-show-maximum-output
+                       (>= (point) eshell-last-output-end))
+              (save-excursion
+                (goto-char (point-max))
+                (recenter -1)))))
       (set-buffer current))))
 
 (defun eshell-beginning-of-input ()
@@ -977,27 +972,24 @@ eshell-handle-control-codes
     (goto-char eshell-last-output-block-begin)
     (unless (eolp)
       (beginning-of-line))
-    (while (< (point) eshell-last-output-end)
-      (let ((char (char-after)))
+    (while (re-search-forward (rx (any ?\r ?\a ?\C-h))
+                              eshell-last-output-end t)
+      (let ((char (char-before)))
         (cond
          ((eq char ?\r)
-          (if (< (1+ (point)) eshell-last-output-end)
-              (if (memq (char-after (1+ (point)))
-                        '(?\n ?\r))
-                  (delete-char 1)
-                (let ((end (1+ (point))))
+          (if (< (point) eshell-last-output-end)
+              (if (memq (char-after (point)) '(?\n ?\r))
+                  (delete-char -1)
+                (let ((end (point)))
                   (beginning-of-line)
                   (delete-region (point) end)))
-            (add-text-properties (point) (1+ (point))
-                                 '(invisible t))
-            (forward-char)))
+            (add-text-properties (1- (point)) (point)
+                                 '(invisible t))))
          ((eq char ?\a)
-          (delete-char 1)
+          (delete-char -1)
           (beep))
          ((eq char ?\C-h)
-          (delete-region (1- (point)) (1+ (point))))
-         (t
-          (forward-char)))))))
+          (delete-region (- (point) 2) (point))))))))
 
 (custom-add-option 'eshell-output-filter-functions
 		   'eshell-handle-control-codes)
diff --git a/test/lisp/eshell/esh-mode-tests.el b/test/lisp/eshell/esh-mode-tests.el
new file mode 100644
index 00000000000..306e11ce445
--- /dev/null
+++ b/test/lisp/eshell/esh-mode-tests.el
@@ -0,0 +1,62 @@
+;;; esh-mode-tests.el --- esh-mode test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022-2024 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's command invocation.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+;;; Tests:
+
+(ert-deftest esh-mode-test/handle-control-codes/carriage-return ()
+  "Test that Eshell handles carriage returns properly."
+  (with-temp-eshell
+    (eshell-match-command-output "(format \"hello\r\ngoodbye\")"
+                                 "\\`hello\ngoodbye\n")
+    (eshell-match-command-output "(format \"hello\rgoodbye\")"
+                                 "\\`goodbye\n")
+    (eshell-match-command-output "(format \"hello\r\")"
+                                 "\\`hello")))
+
+(ert-deftest esh-mode-test/handle-control-codes/bell ()
+  "Test that Eshell handles bells properly."
+  (cl-letf* ((beep-called nil)
+             ((symbol-function 'beep) (lambda () (setq beep-called t))))
+    (with-temp-eshell
+      (eshell-match-command-output "(format \"hello\athere\")"
+                                   "\\`hellothere\n")
+      (should beep-called))))
+
+(ert-deftest esh-mode-test/handle-control-codes/backspace ()
+  "Test that Eshell handles backspaces properly."
+  (with-temp-eshell
+    (eshell-match-command-output (format "(format \"hello%c%cp\")" ?\C-h ?\C-h)
+                                 "\\`help\n")))
+
+;; esh-mode-tests.el ends here
-- 
2.25.1


  reply	other threads:[~2024-06-08  4:25 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-04  5:36 bug#71355: 30.0.50; [PATCH] Improve performance of buffered output in Eshell Jim Porter
2024-06-04 21:52 ` Stefan Kangas
2024-06-05  1:55   ` Jim Porter
2024-06-05  3:50     ` Jim Porter
2024-06-05 12:06       ` Eli Zaretskii
2024-06-05 16:42         ` Jim Porter
2024-06-05 16:51           ` Eli Zaretskii
2024-06-05 17:35             ` Jim Porter
2024-06-05 17:57               ` Eli Zaretskii
2024-06-05 18:47                 ` Jim Porter
2024-06-05 18:58                   ` Eli Zaretskii
2024-06-05 20:07                     ` Jim Porter
2024-06-06  4:43                       ` Eli Zaretskii
2024-06-06 18:02                         ` Jim Porter
2024-06-08  4:25                           ` Jim Porter [this message]
2024-06-08  7:33                             ` Stefan Kangas
2024-06-08 19:43                               ` Jim Porter
2024-06-06  9:20     ` Stefan Kangas
2024-06-06 18:04       ` Jim Porter
2024-06-06 23:14     ` Stefan Kangas
2024-06-07  0:09       ` Jim Porter
2024-06-07  8:51         ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dd14cfe2-0f15-70ae-9e70-b75873544f9d@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=71355@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=stefankangas@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).