unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Tony Zorman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Jim Porter <jporterbugs@gmail.com>, 63748@debbugs.gnu.org
Subject: bug#63748: [PATCH] bug#63748: 30.0.50; eshell-previous-prompt doesn't work for multiline prompts
Date: Thu, 08 Jun 2023 17:11:49 +0200	[thread overview]
Message-ID: <878rcu0z0q.fsf@hyperspace> (raw)
In-Reply-To: <88ae9a36-c9f9-fe0a-80a2-d90ff1b3837a@gmail.com>

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

On Sat, Jun 03 2023 12:35, Jim Porter wrote:
> You can probably simplify the multiline wrapper in your test to 
> something like this:
>
>    (defmacro em-prompt-test--with-multiline (&rest body)
>      "Execute BODY with a multiline Eshell prompt."
>      `(let ((eshell-prompt-function (lambda () "multiline prompt\n$ ")))
>         ,@body))
>
> Then use it like so:
>
>    ;; Note: no arguments necessary.
>    (defun em-prompt-test/forward-backward-matching-input-with ()
>       ;; ...
>       )
>
>    (ert-deftest em-prompt-test/forward-backward-matching-input-multiline ()
>      (em-prompt-test--with-multiline
>       (em-prompt-test/forward-backward-matching-input-with)))
>
> That should be safer than setq'ing the prompt function. (You also don't 
> need to set the prompt regexp, since that's only useful if you want to 
> navigate via '(forward|backward)-paragraph', and we don't test that here.)

oh, this is pretty neat, thanks!

Sorry for the slow response; I have attached the (hopefully properly)
corrected patch now.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-eshell-next-prompt-More-precisely-navigate-to-the-pr.patch --]
[-- Type: text/x-patch, Size: 4405 bytes --]

From 37988ca00beb26c0ce22f1c5f0d16cc4df56662c Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Sat, 3 Jun 2023 14:23:19 +0200
Subject: [PATCH] eshell-next-prompt: More precisely navigate to the prompt
 (bug#63748)

* lisp/eshell/em-prompt.el (eshell-next-prompt): Navigate to the
current prompt more accurately by using text properties instead of
going to the beginning of the line.  This is important for multiline
prompts, as they don't necessarily start at the beginning of the
current line.

* test/lisp/eshell/em-prompt-tests.el
(em-prompt-test--with-multiline):
Execute a given body with a multiline prompt.

(em-prompt-test/next-previous-prompt-with):
(em-prompt-test/forward-backward-matching-input-with):
Helper functions for code reuse.

(em-prompt-test/forward-backward-matching-input):
(em-prompt-test/next-previous-prompt):
Rewrite in terms of the appropriate helper functions.

(em-prompt-test/next-previous-prompt-multiline):
(em-prompt-test/forward-backward-matching-input-multiline):
Add multiline variants of existing tests.
---
 lisp/eshell/em-prompt.el            |  3 ++-
 test/lisp/eshell/em-prompt-tests.el | 31 +++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index 9f9e58e83d..42f8f273b5 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -180,7 +180,8 @@ eshell-next-prompt
                   (text-property-search-forward 'field 'prompt t))
         (setq n (1- n)))
     (let (match this-match)
-      (forward-line 0)           ; Don't count prompt on current line.
+      ;; Don't count the current prompt.
+      (text-property-search-backward 'field 'prompt t)
       (while (and (< n 0)
                   (setq this-match (text-property-search-backward
                                     'field 'prompt t)))
diff --git a/test/lisp/eshell/em-prompt-tests.el b/test/lisp/eshell/em-prompt-tests.el
index 257549e40f..93bf9d84ab 100644
--- a/test/lisp/eshell/em-prompt-tests.el
+++ b/test/lisp/eshell/em-prompt-tests.el
@@ -80,8 +80,13 @@ em-prompt-test/field-properties/no-highlight
                 (apply #'propertize "hello\n"
                        eshell-command-output-properties)))))))
 
-(ert-deftest em-prompt-test/next-previous-prompt ()
-  "Check that navigating forward/backward through old prompts works correctly."
+(defmacro em-prompt-test--with-multiline (&rest body)
+  "Execute BODY with a multiline Eshell prompt."
+  `(let ((eshell-prompt-function (lambda () "multiline prompt\n$ ")))
+     ,@body))
+
+(defun em-prompt-test/next-previous-prompt-with ()
+  "Helper for checking forward/backward navigation of old prompts."
   (with-temp-eshell
    (eshell-insert-command "echo one")
    (eshell-insert-command "echo two")
@@ -98,8 +103,17 @@ em-prompt-test/next-previous-prompt
    (eshell-next-prompt 3)
    (should (equal (eshell-get-old-input) "echo fou"))))
 
-(ert-deftest em-prompt-test/forward-backward-matching-input ()
-  "Check that navigating forward/backward via regexps works correctly."
+(ert-deftest em-prompt-test/next-previous-prompt ()
+  "Check that navigating forward/backward through old prompts works correctly."
+  (em-prompt-test/next-previous-prompt-with))
+
+(ert-deftest em-prompt-test/next-previous-prompt-multiline ()
+  "Check old prompt forward/backward navigation for multiline prompts."
+  (em-prompt-test--with-multiline
+   (em-prompt-test/next-previous-prompt-with)))
+
+(defun em-prompt-test/forward-backward-matching-input-with ()
+  "Helper for checking forward/backward navigation via regexps."
   (with-temp-eshell
    (eshell-insert-command "echo one")
    (eshell-insert-command "printnl something else")
@@ -117,4 +131,13 @@ em-prompt-test/forward-backward-matching-input
    (eshell-forward-matching-input "echo" 3)
    (should (equal (eshell-get-old-input) "echo fou"))))
 
+(ert-deftest em-prompt-test/forward-backward-matching-input ()
+  "Check that navigating forward/backward via regexps works correctly."
+  (em-prompt-test/forward-backward-matching-input-with))
+
+(ert-deftest em-prompt-test/forward-backward-matching-input-multiline ()
+  "Check forward/backward regexp navigation for multiline prompts."
+  (em-prompt-test--with-multiline
+   (em-prompt-test/forward-backward-matching-input-with)))
+
 ;;; em-prompt-tests.el ends here
-- 
2.41.0


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


-- 
Tony Zorman | https://tony-zorman.com/

  reply	other threads:[~2023-06-08 15:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-27  8:38 bug#63748: 30.0.50; eshell-previous-prompt doesn't work for multiline prompts Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-30  5:02 ` Jim Porter
2023-06-03 13:27   ` bug#63748: [PATCH] " Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-03 19:35     ` Jim Porter
2023-06-08 15:11       ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2023-06-15 16:46         ` Jim Porter

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=878rcu0z0q.fsf@hyperspace \
    --to=bug-gnu-emacs@gnu.org \
    --cc=63748@debbugs.gnu.org \
    --cc=jporterbugs@gmail.com \
    --cc=soliditsallgood@mailbox.org \
    /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).