Signal errors when executing ‘markdown-command’ or ‘markdown-open-com…

…mand’ fails

Fixes jrblevin#291

CHANGES.md

@@ -94,6 +94,8 @@
     -   Make code block language detection handle unspecified
         or unknown code block languages.  ([GH-284][])
     -   Fix precedence of inline code over inline links.
+    -   Improve error reporting for `markdown` and `markdown-open`.
+        ([GH-291][])
 
   [gh-171]: https://github.com/jrblevin/markdown-mode/issues/171
   [gh-216]: https://github.com/jrblevin/markdown-mode/issues/216
@@ -129,6 +131,7 @@
   [gh-276]: https://github.com/jrblevin/markdown-mode/issues/276
   [gh-277]: https://github.com/jrblevin/markdown-mode/pull/277
   [gh-284]: https://github.com/jrblevin/markdown-mode/issues/284
+  [gh-291]: https://github.com/jrblevin/markdown-mode/issues/291
   [gh-296]: https://github.com/jrblevin/markdown-mode/issues/296
 
 # Markdown Mode 2.3

markdown-mode.el

@@ -7148,25 +7148,39 @@ Return the name of the output buffer used."
 
       (unless output-buffer-name
         (setq output-buffer-name markdown-output-buffer-name))
-      (cond
-       ;; Handle case when `markdown-command' does not read from stdin
-       ((and (stringp markdown-command) markdown-command-needs-filename)
-        (if (not buffer-file-name)
-            (user-error "Must be visiting a file")
-          (shell-command (concat markdown-command " "
-                                 (shell-quote-argument buffer-file-name))
-                         output-buffer-name)))
-       ;; Pass region to `markdown-command' via stdin
-       (t
-        (let ((buf (get-buffer-create output-buffer-name)))
-          (with-current-buffer buf
-            (setq buffer-read-only nil)
-            (erase-buffer))
-          (if (stringp markdown-command)
-              (call-process-region begin-region end-region
-                                   shell-file-name nil buf nil
-                                   shell-command-switch markdown-command)
-            (funcall markdown-command begin-region end-region buf))))))
+      (let ((exit-code
+             (cond
+              ;; Handle case when `markdown-command' does not read from stdin
+              ((and (stringp markdown-command) markdown-command-needs-filename)
+               (if (not buffer-file-name)
+                   (user-error "Must be visiting a file")
+                 ;; Don’t use ‘shell-command’ because it’s not guaranteed to
+                 ;; return the exit code of the process.
+                 (shell-command-on-region
+                  ;; Pass an empty region so that stdin is empty.
+                  (point) (point)
+                  (concat markdown-command " "
+                          (shell-quote-argument buffer-file-name))
+                  output-buffer-name)))
+              ;; Pass region to `markdown-command' via stdin
+              (t
+               (let ((buf (get-buffer-create output-buffer-name)))
+                 (with-current-buffer buf
+                   (setq buffer-read-only nil)
+                   (erase-buffer))
+                 (if (stringp markdown-command)
+                     (call-process-region begin-region end-region
+                                          shell-file-name nil buf nil
+                                          shell-command-switch markdown-command)
+                   (funcall markdown-command begin-region end-region buf)
+                   ;; If the ‘markdown-command’ function didn’t signal an
+                   ;; error, assume it succeeded by binding ‘exit-code’ to 0.
+                   0))))))
+        ;; The exit code can be a signal description string, so don’t use ‘=’
+        ;; or ‘zerop’.
+        (unless (eq exit-code 0)
+          (user-error "%s failed with exit code %s"
+                      markdown-command exit-code))))
     output-buffer-name))
 
 (defun markdown-standalone (&optional output-buffer-name)
@@ -7488,7 +7502,13 @@ update this buffer's contents."
       (if (not buffer-file-name)
           (user-error "Must be visiting a file")
         (save-buffer)
-        (call-process markdown-open-command nil 0 nil buffer-file-name))
+        (let ((exit-code (call-process markdown-open-command nil 0 nil
+                                       buffer-file-name)))
+          ;; The exit code can be a signal description string, so don’t use ‘=’
+          ;; or ‘zerop’.
+          (unless (eq exit-code 0)
+            (user-error "%s failed with exit code %s"
+                        markdown-open-command exit-code))))
     (funcall markdown-open-command))
   nil)
 

tests/markdown-test.el

@@ -5103,6 +5103,7 @@ This includes preserving whitespace after the pipe."
   "Test `markdown-export-kill-buffer' equal to nil."
   (markdown-test-temp-file "inline.text"
     (let* ((markdown-export-kill-buffer nil)
+           (markdown-command #'ignore)
            (export-file (markdown-export))
            (export-buffer (get-file-buffer export-file)))
       ;; Output buffer should remain open.
@@ -5112,6 +5113,7 @@ This includes preserving whitespace after the pipe."
   "Test `markdown-export-kill-buffer' equal to t."
   (markdown-test-temp-file "inline.text"
     (let* ((markdown-export-kill-buffer t)
+           (markdown-command #'ignore)
            (export-file (markdown-export))
            (export-buffer (get-file-buffer export-file)))
       ;; Output buffer should be killed.
@@ -5124,6 +5126,7 @@ This includes preserving whitespace after the pipe."
   (markdown-test-temp-file "lists.text"
    (let* ((before-hook-run nil)
           (orig-point (point))
+          (markdown-command #'ignore)
           (func (lambda ()
                   ;; Change value of a variable
                   (setq before-hook-run t)
@@ -5158,6 +5161,7 @@ This includes preserving whitespace after the pipe."
   (markdown-test-temp-file "lists.text"
    (let* ((after-hook-run nil)
           (markdown-export-kill-buffer nil)
+          (markdown-command #'ignore)
           (func (lambda ()
                   ;; Change variable value
                   (setq after-hook-run t)
@@ -5575,58 +5579,61 @@ x|"
 (ert-deftest test-markdown-ext/live-preview-no-file ()
   "Live-preview a `markdown-mode' buffer without a file."
   (with-temp-buffer
-    (markdown-mode)
+    (let ((markdown-command #'ignore))
+      (markdown-mode)
 
-    ;; Activating `markdown-live-preview-mode' signals error
-    (should-error (markdown-live-preview-mode))
+      ;; Activating `markdown-live-preview-mode' signals error
+      (should-error (markdown-live-preview-mode))
 
-    ;; After trying to activate live preview mode, mode is not activated
-    (should-not markdown-live-preview-mode)
+      ;; After trying to activate live preview mode, mode is not activated
+      (should-not markdown-live-preview-mode)
 
-    ;; `markdown-live-preview-export' does nothing
-    (should-not (markdown-live-preview-export))
+      ;; `markdown-live-preview-export' does nothing
+      (should-not (markdown-live-preview-export))
 
-    ;; `markdown-live-preview-remove' does nothing
-    (should-not (markdown-live-preview-remove))))
+      ;; `markdown-live-preview-remove' does nothing
+      (should-not (markdown-live-preview-remove)))))
 
 (ert-deftest test-markdown-ext/live-preview-exports ()
   (markdown-test-temp-file "inline.text"
-    (unless (and (fboundp 'libxml-parse-html-region) (require 'eww nil t))
-      (should-error (markdown-live-preview-mode)))
-    (markdown-test-fake-eww
-     (markdown-live-preview-mode)
-     (should (buffer-live-p markdown-live-preview-buffer))
-     (should (eq (current-buffer)
-                 (with-current-buffer markdown-live-preview-buffer
-                   markdown-live-preview-source-buffer)))
-     (kill-buffer markdown-live-preview-buffer)
-     (should (null markdown-live-preview-buffer))
-     (set-buffer-modified-p t)
-     (save-buffer)                      ; should create new export
-     (should (buffer-live-p markdown-live-preview-buffer)))))
+    (let ((markdown-command #'ignore))
+      (unless (and (fboundp 'libxml-parse-html-region) (require 'eww nil t))
+        (should-error (markdown-live-preview-mode)))
+      (markdown-test-fake-eww
+       (markdown-live-preview-mode)
+       (should (buffer-live-p markdown-live-preview-buffer))
+       (should (eq (current-buffer)
+                   (with-current-buffer markdown-live-preview-buffer
+                     markdown-live-preview-source-buffer)))
+       (kill-buffer markdown-live-preview-buffer)
+       (should (null markdown-live-preview-buffer))
+       (set-buffer-modified-p t)
+       (save-buffer)                      ; should create new export
+       (should (buffer-live-p markdown-live-preview-buffer))))))
 
 (ert-deftest test-markdown-ext/live-preview-delete-exports ()
   (markdown-test-fake-eww
-   (let ((markdown-live-preview-delete-export 'delete-on-destroy)
-         file-output)
-     (markdown-test-temp-file "inline.text"
-       (markdown-live-preview-mode)
-       (setq file-output (markdown-export-file-name)))
-     (should-not (file-exists-p file-output)))
-   (let ((markdown-live-preview-delete-export 'delete-on-export)
-         file-output)
-     (markdown-test-temp-file "inline.text"
-       (markdown-live-preview-mode)
-       (setq file-output (markdown-export-file-name))
-       (should-not (file-exists-p file-output))))
-   (let ((markdown-live-preview-delete-export nil)
-         file-output)
-     (unwind-protect
-         (markdown-test-temp-file "inline.text"
-           (markdown-live-preview-mode)
-           (setq file-output (markdown-export-file-name))
-           (should (file-exists-p file-output)))
-       (delete-file file-output)))))
+   (let ((markdown-command #'ignore))
+     (let ((markdown-live-preview-delete-export 'delete-on-destroy)
+           file-output)
+       (markdown-test-temp-file "inline.text"
+         (markdown-live-preview-mode)
+         (setq file-output (markdown-export-file-name)))
+       (should-not (file-exists-p file-output)))
+     (let ((markdown-live-preview-delete-export 'delete-on-export)
+           file-output)
+       (markdown-test-temp-file "inline.text"
+         (markdown-live-preview-mode)
+         (setq file-output (markdown-export-file-name))
+         (should-not (file-exists-p file-output))))
+     (let ((markdown-live-preview-delete-export nil)
+           file-output)
+       (unwind-protect
+           (markdown-test-temp-file "inline.text"
+             (markdown-live-preview-mode)
+             (setq file-output (markdown-export-file-name))
+             (should (file-exists-p file-output)))
+         (delete-file file-output))))))
 
 (ert-deftest test-markdown-ext/live-preview-follow-min-max ()
   (markdown-test-eww-or-nothing "live-preview-follow-min-max"
@@ -5733,6 +5740,24 @@ https://github.com/jrblevin/markdown-mode/issues/235"
       (should (buffer-live-p buffer))
       (should (equal calls `((1 4 ,buffer)))))))
 
+(ert-deftest test-markdown-command/does-not-exist ()
+  "Test ‘markdown’ with a non-existing ‘markdown-command’."
+  (skip-unless (not (file-executable-p "/does/not/exist")))
+  (markdown-test-string "foo"
+    (let* ((markdown-command "/does/not/exist")
+           (data (should-error (markdown) :type 'user-error)))
+      (should (string-prefix-p "/does/not/exist failed with exit code "
+                               (error-message-string data))))))
+
+(ert-deftest test-markdown-command/fails ()
+  "Test ‘markdown’ with a failing ‘markdown-command’."
+  (skip-unless (executable-find "false"))
+  (markdown-test-string "foo"
+    (let* ((markdown-command "false")
+           (data (should-error (markdown) :type 'user-error)))
+      (should (string-prefix-p "false failed with exit code "
+                               (error-message-string data))))))
+
 (ert-deftest test-markdown-open-command/function ()
   "Test ‘markdown-open’ with ‘markdown-open-command’ being a function."
   (markdown-test-string ""
@@ -5741,6 +5766,26 @@ https://github.com/jrblevin/markdown-mode/issues/235"
       (markdown-open)
       (should (equal calls 1)))))
 
+(ert-deftest test-markdown-open-command/does-not-exist ()
+  "Test ‘markdown-open’ with a non-existing ‘markdown-open-command’."
+  (skip-unless (not (file-executable-p "/does/not/exist")))
+  (markdown-test-string "foo"
+    (let ((buffer-file-name
+           (expand-file-name "bar.md" temporary-file-directory))
+          (markdown-open-command "/does/not/exist"))
+      (should-error (markdown-open) :type 'file-error))))
+
+(ert-deftest test-markdown-open-command/fails ()
+  "Test ‘markdown-open’ with a failing ‘markdown-open-command’."
+  (skip-unless (executable-find "false"))
+  (markdown-test-string "foo"
+    (let* ((buffer-file-name
+            (expand-file-name "bar.md" temporary-file-directory))
+           (markdown-open-command "false")
+           (data (should-error (markdown-open) :type 'user-error)))
+      (should (string-prefix-p "false failed with exit code "
+                               (error-message-string data))))))
+
 (provide 'markdown-test)
 
 ;;; markdown-test.el ends here