Ihor Radchenko <yanta...@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
>From 7001ed80e5fd146f6acf586b3cf3ef20bbba04e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rudolf=20Adamkovi=C4=8D?= <salu...@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
--
"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č <salu...@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia
