Run the same formatter in multiple buffers in parallel (#64) (#65) (#65)

* Run the same formatter in multiple buffers in parallel (#64) (#65)

Previously when apheleia formatted a buffer it created a stdout and
stderr buffer for each formatter, but it reused this buffer each time
that formatter would run. This makes sense if we only ever format one
buffer at a time (meaning we don't format a new buffer until the
previous buffer has been formatted) such as when calling
`apheleia-format-buffer` interactively (since the interval for running a
formatter is likely far below hitting a key combination for this
command). But this assumption falls apart when using `apheleia-mode` and
`apheleia--format-after-save`.
Now a lot of files could be saved, triggering the same formatters again
and again, within a short period of each other. Apheleia used to keep
track of the current formatter process and kill it when a newer
formatter is attempted, but this also kills all but the last buffer
called by `apheleia--format-after-save`.

With this commit we still have separate stdout and stderr buffers for
each formatter, but we *always* create a new one when attempting a
format. There is a new buffer type, a log buffer, which is populated
with a formatter processes stderr when it fails. We also still have a
`apheleia--current-process` variable, but instead of being global, it's
local to the current buffer being formatted. We now kill it if starting
a new format in the current buffer, but two separate buffers can call
the same formatter with no issue.

* Mark change as bugfix in changelog

* Add to docstring

* Remove no longer needed code

* Re-wrap docstring

* Remove newline

* Change spelling

* Use correct buffer when checking (buffer-size)

Co-authored-by: Radon Rosborough <radon.neon@gmail.com>

2 files changed, 97 insertions, 72 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 27dea21..db6f76b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,6 +38,10 @@ The format is based on [Keep a Changelog].
   on `apheleia-formatters`.
 * Fix mixed style line ending generated by `diff` ([#54]) by adding
   `--strip-trailing-cr` to `diff`'s argument list.
+* Allow running the same formatter in multiple buffers in parallel
+  ([#64], [#65]). Previously, when saving a number of files at the
+  same time, the contents of those buffers could be corrupted by a
+  race condition.
 
 [#24]: https://github.com/raxod502/apheleia/pull/24
 [#30]: https://github.com/raxod502/apheleia/issues/30
@@ -50,6 +54,8 @@ The format is based on [Keep a Changelog].
 [#52]: https://github.com/raxod502/apheleia/issues/52
 [#54]: https://github.com/raxod502/apheleia/pull/54
 [#55]: https://github.com/raxod502/apheleia/issues/55
+[#64]: https://github.com/raxod502/apheleia/issues/64
+[#65]: https://github.com/raxod502/apheleia/pull/65
 
 ## 1.1.2 (released 2021-02-26)
 ### Enhancements
diff --git a/apheleia.el b/apheleia.el
index a815a2e..ed2c794 100644
--- a/apheleia.el
+++ b/apheleia.el
@@ -249,36 +249,41 @@ contains the patch."
             (ignore-errors
               (scroll-down (- old-window-line new-window-line)))))))))
 
-(defvar apheleia--current-process nil
+(defvar-local apheleia--current-process nil
   "Current process that Apheleia is running, or nil.
 Keeping track of this helps avoid running more than one process
 at once.")
 
+(defvar apheleia-verbose nil
+  "When true `apheleia' produces richer log buffers.
+Specifically, formatter stderr is appended to the log buffer even
+if there is no error.")
+
 (cl-defun apheleia--make-process
     (&key command stdin callback ensure exit-status)
   "Wrapper for `make-process' that behaves a bit more nicely.
 COMMAND is as in `make-process'. STDIN, if given, is a buffer
 whose contents are fed to the process on stdin. CALLBACK is
 invoked with one argument, the buffer containing the text from
-stdout, when the process terminates (if it succeeds). ENSURE
-is a callback that's invoked whether the process exited sucessfully
-or not. EXIT-STATUS is a function which is called with the exit
+stdout, when the process terminates (if it succeeds). ENSURE is a
+callback that's invoked whether the process exited sucessfully or
+not. EXIT-STATUS is a function which is called with the exit
 status of the command; it should return non-nil to indicate that
-the command succeeded. If EXIT-STATUS is omitted, then the command
-succeeds provided that its exit status is 0."
+the command succeeded. If EXIT-STATUS is omitted, then the
+command succeeds provided that its exit status is 0."
   (when (process-live-p apheleia--current-process)
+    (message "Interrupting %s" apheleia--current-process)
     (interrupt-process apheleia--current-process)
     (accept-process-output apheleia--current-process 0.1 nil 'just-this-one)
     (when (process-live-p apheleia--current-process)
       (kill-process apheleia--current-process)))
   (let* ((name (file-name-nondirectory (car command)))
-         (stdout (get-buffer-create
+         (stdout (generate-new-buffer
                   (format " *apheleia-%s-stdout*" name)))
-         (stderr (get-buffer-create
-                  (format " *apheleia-%s-stderr*" name))))
-    (dolist (buf (list stdout stderr))
-      (with-current-buffer buf
-        (erase-buffer)))
+         (stderr (generate-new-buffer
+                  (format " *apheleia-%s-stderr*" name)))
+         (log (get-buffer-create
+               (format " *apheleia-%s-log*" name))))
     (condition-case-unless-debug e
         (progn
           (setq apheleia--current-process
@@ -291,26 +296,38 @@ succeeds provided that its exit status is 0."
                  :sentinel
                  (lambda (proc _event)
                    (unless (process-live-p proc)
-                     (with-current-buffer stderr
-                       (when (= 0 (buffer-size))
-                         (insert "[No output received on stderr]\n")))
-                     (unwind-protect
-                         (if (funcall
-                              (or exit-status
-                                  (lambda (status)
-                                    (= 0 status)))
-                              (process-exit-status proc))
-                             (when callback
-                               (funcall callback stdout))
-                           (message
-                            (concat
-                             "Failed to run %s: exit status %s "
-                             "(see hidden buffer%s)")
-                            (car command)
-                            (process-exit-status proc)
-                            stderr))
-                       (when ensure
-                         (funcall ensure)))))))
+                     (let ((exit-ok (funcall
+                                     (or exit-status
+                                         (lambda (status)
+                                           (= 0 status)))
+                                     (process-exit-status proc))))
+                       ;; Append standard-error from current formatter
+                       ;; to log buffer when `apheleia-verbose' or the
+                       ;; formatter failed. Every process output is
+                       ;; delimited by a line-feed character.
+                       (with-current-buffer log
+                         (when (or apheleia-verbose
+                                   (not exit-ok))
+                           (if (= 0 (with-current-buffer stderr
+                                      (buffer-size)))
+                               (insert "[No output received on stderr]")
+                             (insert-buffer-substring stderr))
+                           (insert "\n\C-l\n")))
+                       (unwind-protect
+                           (if exit-ok
+                               (when callback
+                                 (funcall callback stdout))
+                             (message
+                              (concat
+                               "Failed to run %s: exit status %s "
+                               "(see hidden buffer%s)")
+                              (car command)
+                              (process-exit-status proc)
+                              log))
+                         (when ensure
+                           (funcall ensure))
+                         (kill-buffer stdout)
+                         (kill-buffer stderr)))))))
           (set-process-sentinel (get-buffer-process stderr) #'ignore)
           (set-process-coding-system
            apheleia--current-process
@@ -384,18 +401,16 @@ as its sole argument."
       (with-current-buffer new-buffer
         (setq new-fname (make-temp-file "apheleia"))
         (apheleia--write-region-silently (point-min) (point-max) new-fname)))
-    (with-current-buffer (get-buffer-create " *apheleia-patch*")
-      (erase-buffer)
-      (apheleia--make-process
-       :command `("diff" "--rcs" "--strip-trailing-cr" "--"
-                  ,(or old-fname "-")
-                  ,(or new-fname "-"))
-       :stdin (if new-fname old-buffer new-buffer)
-       :callback callback
-       :exit-status (lambda (status)
-                      ;; Exit status is 0 if no changes, 1 if some
-                      ;; changes, and 2 if error.
-                      (memq status '(0 1)))))))
+    (apheleia--make-process
+     :command `("diff" "--rcs" "--strip-trailing-cr" "--"
+                ,(or old-fname "-")
+                ,(or new-fname "-"))
+     :stdin (if new-fname old-buffer new-buffer)
+     :callback callback
+     :exit-status (lambda (status)
+                    ;; Exit status is 0 if no changes, 1 if some
+                    ;; changes, and 2 if error.
+                    (memq status '(0 1))))))
 
 (defun apheleia--safe-buffer-name ()
   "Return `buffer-name' without special file-system characters."
@@ -519,33 +534,37 @@ formatter in COMMANDS. This should not be supplied by the caller
 and instead is supplied by this command when invoked recursively.
 The stdout of the previous formatter becomes the stdin of the
 next formatter."
-  (when-let ((ret (with-current-buffer buffer
-                    (apheleia--format-command (car commands) stdin))))
-    (cl-destructuring-bind (input-fname output-fname stdin &rest command) ret
-      (apheleia--make-process
-       :command command
-       :stdin (unless input-fname
-                stdin)
-       :callback
-       (lambda (stdout)
-         (when output-fname
-           ;; Load output-fname contents into the stdout buffer.
-           (erase-buffer)
-           (insert-file-contents-literally output-fname))
-
-         (if (cdr commands)
-             ;; Forward current stdout to remaining formatters, passing along
-             ;; the current callback and using the current formatters output
-             ;; as stdin.
-             (apheleia--run-formatters (cdr commands) buffer callback stdout)
-           (funcall callback stdout)))
-       :ensure
-       (lambda ()
-         (ignore-errors
-           (when input-fname
-             (delete-file input-fname))
+  ;; NOTE: We switch to the original buffer both to format the command
+  ;; correctly and also to ensure any buffer local variables correctly
+  ;; resolve for the whole formatting process (for example
+  ;; `apheleia--current-process').
+  (with-current-buffer buffer
+    (when-let ((ret (apheleia--format-command (car commands) stdin)))
+      (cl-destructuring-bind (input-fname output-fname stdin &rest command) ret
+        (apheleia--make-process
+         :command command
+         :stdin (unless input-fname
+                  stdin)
+         :callback
+         (lambda (stdout)
            (when output-fname
-             (delete-file output-fname))))))))
+             ;; Load output-fname contents into the stdout buffer.
+             (erase-buffer)
+             (insert-file-contents-literally output-fname))
+
+           (if (cdr commands)
+               ;; Forward current stdout to remaining formatters, passing along
+               ;; the current callback and using the current formatters output
+               ;; as stdin.
+               (apheleia--run-formatters (cdr commands) buffer callback stdout)
+             (funcall callback stdout)))
+         :ensure
+         (lambda ()
+           (ignore-errors
+             (when input-fname
+               (delete-file input-fname))
+             (when output-fname
+               (delete-file output-fname)))))))))
 
 (defcustom apheleia-formatters
   '((black . ("black" "-"))
@@ -772,8 +791,8 @@ changes), CALLBACK, if provided, is invoked with no arguments."
 
 ;; Prevent infinite loop.
 (defvar apheleia--format-after-save-in-progress nil
-  "Prevent apheleia--format-after-save from being called recursively.
-This will be locally bound to t while apheleia--format-after-save is
+  "Prevent `apheleia--format-after-save' from being called recursively.
+This will be locally bound to t while `apheleia--format-after-save' is
 operating, to prevent an infinite loop.")
 
 ;; Autoload because the user may enable `apheleia-mode' without