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>
This commit is contained in:
Mohsin Kaleem 2021-12-26 20:52:52 +00:00 committed by GitHub
parent 2cf903e9a2
commit e700c78a5d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 97 additions and 72 deletions

View file

@ -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

View file

@ -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