unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: Christopher Howard <christopher@librehacker.com>, 72857@debbugs.gnu.org
Cc: eliz@gnu.org
Subject: bug#72857: 30.0.90; emacs 30: eshell-execute-file DESTINATION
Date: Wed, 28 Aug 2024 21:51:10 -0700	[thread overview]
Message-ID: <f5d7a5e6-697c-8372-ef8f-513b47572edd@gmail.com> (raw)
In-Reply-To: <8690a6c6-76b1-c738-ab37-d3a4712f9499@gmail.com>

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

On 8/28/2024 7:17 PM, Jim Porter wrote:
> These should all be fixed as of b6f4ffcc106 on the master branch. Maybe 
> we should backport to the release branch, but the code has already 
> diverged here due to some restructuring on the master branch. I'll sleep 
> on it though to make sure I haven't missed some obvious detail...

Here are the patches for the release branch. Eli, do you think these 
would be ok to merge? I know we're very close to the release, but these 
are a couple of small corner cases with a new Eshell function added to 
Emacs 30, and it'd be nice to have things work as documented.

Both patches are adapted from the ones I merged to the master branch.

[-- Attachment #2: 0001-Fix-redirecting-Eshell-output-to-symbols-in-some-pla.patch --]
[-- Type: text/plain, Size: 4971 bytes --]

From a0e2d16c3ad86646c0aacd5e07ad59709eb1d5b8 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 28 Aug 2024 18:53:03 -0700
Subject: [PATCH 1/2] Fix redirecting Eshell output to symbols in some places

Do not merge to master.

* lisp/eshell/esh-io.el (eshell-output-object-to-target): Don't require
TARGET to be bound.

* lisp/eshell/em-script.el (eshell-execute-file): Quote the output/error
targets.

* test/lisp/eshell/em-script-tests.el (eshell-execute-file-output): New
variable.
(em-script-test/execute-file/output-file)
(em-script-test/execute-file/output-symbol): New tests.

* test/lisp/eshell/esh-io-tests.el (eshell-test-file-string): Move to...
* test/lisp/eshell/eshell-tests-helpers.el (eshell-test-file-string):
... here.
---
 lisp/eshell/em-script.el                 |  2 +-
 lisp/eshell/esh-io.el                    |  2 +-
 test/lisp/eshell/em-script-tests.el      | 18 ++++++++++++++++++
 test/lisp/eshell/esh-io-tests.el         |  6 ------
 test/lisp/eshell/eshell-tests-helpers.el |  6 ++++++
 5 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/lisp/eshell/em-script.el b/lisp/eshell/em-script.el
index ba020d2eb5b..ebba0440d68 100644
--- a/lisp/eshell/em-script.el
+++ b/lisp/eshell/em-script.el
@@ -119,7 +119,7 @@ eshell-execute-file
       (eshell-mode)
       (eshell-do-eval
        `(let ((eshell-current-handles
-               (eshell-create-handles ,stdout 'insert))
+               (eshell-create-handles ',stdout 'insert))
               (eshell-current-subjob-p))
           ,(eshell--source-file file args))
        t))))
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 9de9cc4509a..570ace2ebb8 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -713,7 +713,7 @@ eshell-output-object-to-target
 
 (cl-defmethod eshell-output-object-to-target (object (target symbol))
   "Output OBJECT to the value of the symbol TARGET."
-  (if (not (symbol-value target))
+  (if (not (and (boundp target) (symbol-value target)))
       (set target object)
     (setq object (eshell-stringify object))
     (if (not (stringp (symbol-value target)))
diff --git a/test/lisp/eshell/em-script-tests.el b/test/lisp/eshell/em-script-tests.el
index 86a78e43026..5e5eb80f215 100644
--- a/test/lisp/eshell/em-script-tests.el
+++ b/test/lisp/eshell/em-script-tests.el
@@ -33,6 +33,9 @@
          (expand-file-name "eshell-tests-helpers"
                            (file-name-directory (or load-file-name
                                                     default-directory))))
+
+(defvar eshell-execute-file-output)
+
 ;;; Tests:
 
 (ert-deftest em-script-test/source-script ()
@@ -113,6 +116,21 @@ em-script-test/execute-file/args
         (eshell-execute-file temp-file '(1 2 3) t))
       (should (equal (buffer-string) "6")))))
 
+(ert-deftest em-script-test/execute-file/output-file ()
+  "Test `eshell-execute-file' redirecting to a file."
+  (ert-with-temp-file temp-file :text "echo more"
+    (ert-with-temp-file output-file :text "initial"
+      (with-temp-eshell-settings
+        (eshell-execute-file temp-file nil output-file))
+      (should (equal (eshell-test-file-string output-file) "moreinitial")))))
+
+(ert-deftest em-script-test/execute-file/output-symbol ()
+  "Test `eshell-execute-file' redirecting to a symbol."
+  (ert-with-temp-file temp-file :text "echo hi\necho bye"
+    (with-temp-eshell-settings
+      (eshell-execute-file temp-file nil 'eshell-execute-file-output))
+    (should (equal eshell-execute-file-output "hibye"))))
+
 (ert-deftest em-script-test/batch-file ()
   "Test running an Eshell script file as a batch script."
   (ert-with-temp-file temp-file
diff --git a/test/lisp/eshell/esh-io-tests.el b/test/lisp/eshell/esh-io-tests.el
index b4e8c0b4a9a..6add14c05fa 100644
--- a/test/lisp/eshell/esh-io-tests.el
+++ b/test/lisp/eshell/esh-io-tests.el
@@ -34,12 +34,6 @@ eshell-test-value
 (defvar eshell-test-value-with-fun nil)
 (defun eshell-test-value-with-fun ())
 
-(defun eshell-test-file-string (file)
-  "Return the contents of FILE as a string."
-  (with-temp-buffer
-    (insert-file-contents file)
-    (buffer-string)))
-
 (defun eshell/test-output ()
   "Write some test output separately to stdout and stderr."
   (eshell-printn "stdout")
diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index bfd829c95e9..def04be0577 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -139,6 +139,12 @@ eshell-last-output
   (buffer-substring-no-properties
    (eshell-beginning-of-output) (eshell-end-of-output)))
 
+(defun eshell-test-file-string (file)
+  "Return the contents of FILE as a string."
+  (with-temp-buffer
+    (insert-file-contents file)
+    (buffer-string)))
+
 (defun eshell-match-output (regexp)
   "Test whether the output of the last command matches REGEXP."
   (string-match-p regexp (eshell-last-output)))
-- 
2.25.1


[-- Attachment #3: 0002-Support-dev-null-as-a-target-when-creating-Eshell-ha.patch --]
[-- Type: text/plain, Size: 2294 bytes --]

From f76d165a5067d65d37bbc9c0c0a9e73943ab92ec Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 28 Aug 2024 19:12:29 -0700
Subject: [PATCH 2/2] Support "/dev/null" as a target when creating Eshell
 handles

Previously, you could only use this when setting the handle afterwards.

Do not merge to master.

* lisp/eshell/esh-io.el (eshell-set-output-handle): Don't catch
'eshell-null-device' here...
(eshell-get-target): ... catch it here.
---
 lisp/eshell/esh-io.el | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 570ace2ebb8..18fd1cdb0ec 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -431,11 +431,10 @@ eshell-set-output-handle
       (when defaultp
         (cl-decf (cdar handle))
         (setcar handle (cons nil 1)))
-      (catch 'eshell-null-device
-        (let ((current (caar handle))
-              (where (eshell-get-target target mode)))
-          (unless (member where current)
-            (setcar (car handle) (append current (list where))))))
+      (let ((current (caar handle))
+            (where (eshell-get-target target mode)))
+        (when (and where (not (member where current)))
+          (setcar (car handle) (append current (list where)))))
       (setcar (cdr handle) nil))))
 
 (defun eshell-copy-output-handle (index index-to-copy &optional handles)
@@ -611,11 +610,13 @@ eshell-get-target
 marker for a file named TARGET."
   (setq mode (or mode 'insert))
   (if-let ((redir (assoc raw-target eshell-virtual-targets)))
-      (let ((target (if (nth 2 redir)
-                        (funcall (nth 1 redir) mode)
-                      (nth 1 redir))))
-        (unless (eshell-generic-target-p target)
-          (setq target (eshell-function-target-create target)))
+      (let (target)
+        (catch 'eshell-null-device
+          (setq target (if (nth 2 redir)
+                           (funcall (nth 1 redir) mode)
+                         (nth 1 redir)))
+          (unless (eshell-generic-target-p target)
+            (setq target (eshell-function-target-create target))))
         target)
     (let ((exists (get-file-buffer raw-target))
           (buf (find-file-noselect raw-target t)))
-- 
2.25.1


  reply	other threads:[~2024-08-29  4:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-28 17:45 bug#72857: 30.0.90; emacs 30: eshell-execute-file DESTINATION Christopher Howard
2024-08-28 20:51 ` Jim Porter
2024-08-29  2:17   ` Jim Porter
2024-08-29  4:51     ` Jim Porter [this message]
2024-08-29  5:50       ` Eli Zaretskii
2024-08-30  4:54         ` Jim Porter
2024-08-29  4:49   ` Eli Zaretskii
2024-08-29  4:58     ` 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=f5d7a5e6-697c-8372-ef8f-513b47572edd@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=72857@debbugs.gnu.org \
    --cc=christopher@librehacker.com \
    --cc=eliz@gnu.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).