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

@@ -5733,6 +5733,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 +5759,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