emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: "Rudolf Adamkovič" <salutis@me.com>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: Ihor Radchenko <yantar92@gmail.com>, emacs-orgmode@gnu.org
Subject: Re: Org 9.6-pre and Bash sessions
Date: Thu, 03 Nov 2022 17:30:29 +0100	[thread overview]
Message-ID: <m2tu3gf1ay.fsf@me.com> (raw)
In-Reply-To: <87r0yrjmue.fsf@localhost>

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

Ihor Radchenko <yantar92@posteo.net> writes:

> Feel free to change it in the patch together with tests.

Thank you for driving the discussion and for giving me the opportunity
to solve the problem (and not just temporarily, but with tests).  What
do you think about the attached patch?

Rudy


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-shell-Never-throw-away-standard-error.patch --]
[-- Type: text/x-patch, Size: 7362 bytes --]

From 7001ed80e5fd146f6acf586b3cf3ef20bbba04e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rudolf=20Adamkovi=C4=8D?= <salutis@me.com>
Date: Wed, 26 Oct 2022 13:35:26 +0200
Subject: [PATCH] ob-shell: Never throw away standard error

* lisp/ob-eval.el (org-babel-eval-error-notify): Do not insert
superfluous whitespace.
* lisp/ob-eval.el (org-babel-eval): Show standard error even if the
command exits with a zero code.
* testing/lisp/test-ob-shell.el(
ob-shell/standard-output-after-success,
ob-shell/standard-output-after-failure,
ob-shell/error-output-after-success,
ob-shell/error-output-after-failure,
ob-shell/error-output-after-failure-multiple,
ob-shell/exit-code,
ob-shell/exit-code-multiple
): Add tests to avoid regressions.
---
 lisp/ob-eval.el               | 47 +++++++++++---------
 testing/lisp/test-ob-shell.el | 82 +++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 21 deletions(-)

diff --git a/lisp/ob-eval.el b/lisp/ob-eval.el
index e1278bbad..af9283c0a 100644
--- a/lisp/ob-eval.el
+++ b/lisp/ob-eval.el
@@ -40,39 +40,44 @@
     (with-current-buffer buf
       (goto-char (point-max))
       (save-excursion
-        (insert stderr)
         (unless (bolp) (insert "\n"))
-        (insert (format "[ Babel evaluation exited with code %S ]\n" exit-code))))
+        (insert stderr)
+        (insert (format "[ Babel evaluation exited with code %S ]" exit-code))))
     (display-buffer buf))
   (message "Babel evaluation exited with code %S" exit-code))
 
 (defun org-babel-eval (command query)
   "Run COMMAND on QUERY.
+Return standard output produced by COMMAND.  If COMMAND exits
+with a non-zero code or produces error output, show it with
+`org-babel-eval-error-notify'.
+
 Writes QUERY into a temp-buffer that is processed with
-`org-babel--shell-command-on-region'.  If COMMAND succeeds then return
-its results, otherwise display STDERR with
-`org-babel-eval-error-notify'."
+`org-babel--shell-command-on-region'."
   (let ((error-buffer (get-buffer-create " *Org-Babel Error*")) exit-code)
     (with-current-buffer error-buffer (erase-buffer))
     (with-temp-buffer
       (insert query)
       (setq exit-code
-	    (org-babel--shell-command-on-region
-	     command error-buffer))
-      (if (or (not (numberp exit-code)) (> exit-code 0))
-	  (progn
-	    (with-current-buffer error-buffer
-	      (org-babel-eval-error-notify exit-code (buffer-string)))
-	    (save-excursion
-	      (when (get-buffer org-babel-error-buffer-name)
-		(with-current-buffer org-babel-error-buffer-name
-		  (unless (derived-mode-p 'compilation-mode)
-		    (compilation-mode))
-		  ;; Compilation-mode enforces read-only, but Babel expects the buffer modifiable.
-		  (setq buffer-read-only nil))))
-            ;; Return output, if any.
-	    (buffer-string))
-	(buffer-string)))))
+            (org-babel--shell-command-on-region
+             command error-buffer))
+      (let ((stderr (with-current-buffer error-buffer (buffer-string))))
+        (if (or (not (numberp exit-code))
+                (> exit-code 0)
+                (not (string-empty-p stderr)))
+            (progn
+              (org-babel-eval-error-notify exit-code stderr)
+              (save-excursion
+                (when (get-buffer org-babel-error-buffer-name)
+                  (with-current-buffer org-babel-error-buffer-name
+                    (unless (derived-mode-p 'compilation-mode)
+                      (compilation-mode))
+                    ;; Compilation-mode enforces read-only, but
+                    ;; Babel expects the buffer modifiable.
+                    (setq buffer-read-only nil))))
+              ;; Return output, if any.
+              (buffer-string))
+          (buffer-string))))))
 
 (defun org-babel-eval-read-file (file)
   "Return the contents of FILE as a string."
diff --git a/testing/lisp/test-ob-shell.el b/testing/lisp/test-ob-shell.el
index 4c00faa49..7aa45cc8a 100644
--- a/testing/lisp/test-ob-shell.el
+++ b/testing/lisp/test-ob-shell.el
@@ -170,6 +170,88 @@ ob-comint.el, which was not previously tested."
 	      "#+BEGIN_SRC sh :results table\necho 'I \"want\" it all'\n#+END_SRC"
 	    (org-babel-execute-src-block)))))
 
+;;; Standard output
+
+(ert-deftest ob-shell/standard-output-after-success ()
+  "Test standard output after exiting with a zero code."
+  (should (= 1
+             (org-babel-execute:sh
+              "echo 1" nil))))
+
+(ert-deftest ob-shell/standard-output-after-failure ()
+  "Test standard output after exiting with a non-zero code."
+  (should (= 1
+             (org-babel-execute:sh
+              "echo 1; exit 2" nil))))
+
+;;; Standard error
+
+(ert-deftest ob-shell/error-output-after-success ()
+  "Test that standard error shows in the error buffer, alongside the
+exit code, after exiting with a zero code."
+  (should
+   (string= "1
+[ Babel evaluation exited with code 0 ]"
+            (progn (org-babel-eval-wipe-error-buffer)
+                   (org-babel-execute:sh
+                    "echo 1 >&2" nil)
+                   (with-current-buffer org-babel-error-buffer-name
+                     (buffer-string))))))
+
+(ert-deftest ob-shell/error-output-after-failure ()
+  "Test that standard error shows in the error buffer, alongside the
+exit code, after exiting with a non-zero code."
+  (should
+   (string= "1
+[ Babel evaluation exited with code 2 ]"
+            (progn (org-babel-eval-wipe-error-buffer)
+                   (org-babel-execute:sh
+                    "echo 1 >&2; exit 2" nil)
+                   (with-current-buffer org-babel-error-buffer-name
+                     (buffer-string))))))
+
+(ert-deftest ob-shell/error-output-after-failure-multiple ()
+  "Test that multiple standard error strings show in the error
+buffer, alongside multiple exit codes."
+  (should
+   (string= "1
+[ Babel evaluation exited with code 2 ]
+3
+[ Babel evaluation exited with code 4 ]"
+            (progn (org-babel-eval-wipe-error-buffer)
+                   (org-babel-execute:sh
+                    "echo 1 >&2; exit 2" nil)
+                   (org-babel-execute:sh
+                    "echo 3 >&2; exit 4" nil)
+                   (with-current-buffer org-babel-error-buffer-name
+                     (buffer-string))))))
+
+;;; Exit codes
+
+(ert-deftest ob-shell/exit-code ()
+  "Test that the exit code shows in the error buffer after exiting
+with a non-zero return code."
+  (should
+   (string= "[ Babel evaluation exited with code 1 ]"
+            (progn (org-babel-eval-wipe-error-buffer)
+                   (org-babel-execute:sh
+                    "exit 1" nil)
+                   (with-current-buffer org-babel-error-buffer-name
+                     (buffer-string))))))
+
+(ert-deftest ob-shell/exit-code-multiple ()
+  "Test that multiple exit codes show in the error buffer after
+exiting with a non-zero return code multiple times."
+  (should
+   (string= "[ Babel evaluation exited with code 1 ]
+[ Babel evaluation exited with code 2 ]"
+            (progn (org-babel-eval-wipe-error-buffer)
+                   (org-babel-execute:sh
+                    "exit 1" nil)
+                   (org-babel-execute:sh
+                    "exit 2" nil)
+                   (with-current-buffer org-babel-error-buffer-name
+                     (buffer-string))))))
 
 (provide 'test-ob-shell)
 
-- 
2.38.1


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

-- 
"One can begin to reason only when a clear picture has been formed in
the imagination."
-- Walter Warwick Sawyer, Mathematician's Delight, 1943

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

  reply	other threads:[~2022-11-03 16:31 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-13 15:14 Org 9.6-pre and Bash sessions Rudolf Adamkovič
2022-10-13 17:07 ` Michael Welle
2022-10-14  3:44 ` Ihor Radchenko
2022-10-15 20:56   ` Rudolf Adamkovič
2022-10-17  8:34     ` Ihor Radchenko
2022-10-21  5:29       ` Ihor Radchenko
2022-10-21 13:38         ` Rudolf Adamkovič
2022-10-22  4:22           ` Ihor Radchenko
2022-10-22  9:44             ` Rudolf Adamkovič
2022-10-22 10:59               ` Ihor Radchenko
2022-10-23  4:27                 ` Ihor Radchenko
2022-10-26 11:56                   ` Rudolf Adamkovič
2022-10-26 13:21                     ` Rudolf Adamkovič
2022-10-27  3:53                     ` Ihor Radchenko
2022-10-28 13:12                       ` Rudolf Adamkovič
2022-10-28 13:29                         ` Ihor Radchenko
2022-10-28 21:52                           ` Rudolf Adamkovič
2022-10-29  4:05                             ` [FR] Display stderr contents after executing shell blocks (even when stdout :results output is requested) (was: Org 9.6-pre and Bash sessions) Ihor Radchenko
2022-10-29  6:14                               ` tomas
2022-10-29  6:43                                 ` Samuel Wales
2022-10-29  9:00                                   ` tomas
2022-10-29  9:09                                     ` Ihor Radchenko
2022-10-29  9:18                                       ` tomas
2022-10-30  3:31                                         ` Ihor Radchenko
2022-10-30  6:11                                           ` tomas
2022-10-30  7:09                                             ` Ihor Radchenko
2022-10-30  8:18                                               ` tomas
2022-10-29 11:58                               ` Max Nikulin
2022-10-30  3:37                                 ` Ihor Radchenko
2022-10-30 20:28                               ` Tim Cross
2022-10-31  1:13                                 ` Org babel API (was: [FR] Display stderr contents after executing shell blocks (even when stdout :results output is requested) (was: Org 9.6-pre and Bash sessions)) Ihor Radchenko
2022-10-31  2:03                                   ` Tim Cross
2022-10-31  3:12                                     ` Ihor Radchenko
2022-10-29  4:05                             ` Org 9.6-pre and Bash sessions Ihor Radchenko
2022-11-03 16:30                               ` Rudolf Adamkovič [this message]
2022-11-04  2:52                                 ` Ihor Radchenko
2022-11-05  1:12                                   ` Rudolf Adamkovič

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.orgmode.org/

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

  git send-email \
    --in-reply-to=m2tu3gf1ay.fsf@me.com \
    --to=salutis@me.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=yantar92@gmail.com \
    --cc=yantar92@posteo.net \
    /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/org-mode.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).