Hi all, I saw a cool post the other day with a small snippet of code that was asking for reviews. I love the idea and thought I’d drop in my own little Elisp and ask for the same. This isn’t a package yet but hoping to make it one. Would love any hints or tips of what’s expected of a package as I’ve never made my own and this would be the first. The idea of the package is to have autosave work on an idle timer and trigger immediate save when navigating away in various ways. I’ve been dogfooding it and so far works great for my needs. Thanks in advance!!
(add-variable-watcher 'autosave-toggle
#'(lambda (_ value _ _)
(if value (autosave-setup) (autosave-teardown))))
(defvar autosave-timer nil
"Timer for triggering an autosave some once idle time as occurred.
Default is 1 seconds")
(defvar autosave-delay-interval 1
"The amount of time to wait before autosaving after a change has occurred.")
(defvar autosave-exclude-buffer-list
'("COMMIT_EDITMSG" "treemacs-persist")
"List of buffers to exclude from autosaving")
(defvar autosave-immediate-save-functions-list
'(switch-to-buffer other-window windmove-up windmove-down windmove-left windmove-right next-buffer previous-buffer)
"A list of functions which trigger an immediate save")
(defmacro advice-add-list (advice-list how function)
"Add list of advice to given function using the how parameter"
`(dolist (advice ,advice-list)
(advice-add advice ,how ,function)))
(defmacro advice-remove-list (advice-list function)
"Remove list of advice from given function using the how parameter"
`(dolist (advice ,advice-list)
(advice-remove advice ,function)))
(defmacro add-hooks (hook-list function)
"Add list of hooks to given function"
`(dolist (hook ,hook-list)
(add-hook hook ,function)))
(defmacro remove-hooks (hook-list function)
"Remove list of hooks from given function"
`(dolist (hook ,hook-list)
(remove-hook hook ,function)))
(defun autosave-setup ()
"Setup autosave hooks and advice"
(advice-add-list autosave-immediate-save-functions-list :before #'autosave-save)
(add-hooks autosave-immediate-save-hooks-list #'autosave-save)
(add-hook 'post-command-hook #'autosave-delay-save))
(defun autosave-teardown ()
"Teardown autosave hooks and advice"
(advice-remove-list autosave-immediate-save-functions-list #'autosave-save)
(remove-hooks autosave-immediate-save-hooks-list #'autosave-save)
(remove-hook 'post-command-hook #'autosave-delay-save))
(defvar autosave-immediate-save-hooks-list
'(mouse-leave-buffer-hook focus-out-hook)
"A list of hooks which trigger an immediate save")
(defun is-savable-buffer-p ()
"Check if the current buffer is savable. This is determined by the buffer being a
file buffer, not excluded and is modified"
(let ((buffer-not-excluded (not (member (buffer-name) autosave-exclude-buffer-list)))
(is-file-buffer buffer-file-name)
(is-modified-buffer (buffer-modified-p)))
(and buffer-not-excluded is-file-buffer is-modified-buffer)))
(defun autosave-delay-save ()
"Delay the save for a second to prevent spamming autosave"
(when (is-savable-buffer-p)
(when autosave-timer (cancel-timer autosave-timer))
(setq autosave-timer (run-with-timer autosave-delay-interval nil 'autosave-save))))
(defun autosave-save (&rest _args)
"Save the current buffer if it is savable"
(when (is-savable-buffer-p)
(when autosave-timer (cancel-timer autosave-timer))
(save-buffer)))
(defvar autosave-toggle t "Toggle for turning on autosave")
(add-variable-watcher 'autosave-toggle #'(lambda (_ value _ _) (if value (autosave-setup) (autosave-teardown))))
Don’t add an anonymous function. It will make it harder to remove. Better yet, implement this as a minor-mode.
(defvar autosave-delay-interval 1 "The amount of time to wait before autosaving after a change has occurred.") (defvar autosave-exclude-buffer-list '("COMMIT_EDITMSG" "treemacs-persist") "List of buffers to exclude from autosaving") (defvar autosave-immediate-save-functions-list '(switch-to-buffer other-window windmove-up windmove-down windmove-left windmove-right next-buffer previous-buffer) "A list of functions which trigger an immediate save") (defvar autosave-immediate-save-hooks-list '(mouse-leave-buffer-hook focus-out-hook) "A list of hooks which trigger an immediate save")
These should be user options defined via
defcustom
(defmacro advice-add-list (advice-list how function) "Add list of advice to given function using the how parameter" `(dolist (advice ,advice-list) (advice-add advice ,how ,function))) (defmacro advice-remove-list (advice-list function) "Remove list of advice from given function using the how parameter" `(dolist (advice ,advice-list) (advice-remove advice ,function))) (defmacro add-hooks (hook-list function) "Add list of hooks to given function" `(dolist (hook ,hook-list) (add-hook hook ,function))) (defmacro remove-hooks (hook-list function) "Remove list of hooks from given function" `(dolist (hook ,hook-list) (remove-hook hook ,function)))
These should be functions instead of macros. They should also be prefixed with your package’s “namespace”.
These should be functions instead of macros.
As well, being that simple, they shouldn’t generally exist at all. Rather, the
dolist
forms should just be inlined. (This is probably the usual case of a user who just learned about macros and is excited to use them, and so overuses them. We’ve all been there. :)
add-hooks
aware of use-package?
is-savable-buffer-p
-p
suffix already means a predicate, you don’t needis-
also,
let
evals all bindings, whileand
is a short-circuit thing, I don’t see any reasons to bind all these, especiallyis-file-buffer
defvar
most of these look like user-customizable variables, so
defcustom
should fit better(when (when
an antipattern, use
and
#'(lambda
don’t
also, avoid using lambdas in hooks
defmacro
never ever use macros if a function can do the job
P.S.
flycheck-package
is your friend