Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add global-real-auto-save-mode #55

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DivineDominion
Copy link

@DivineDominion DivineDominion commented Oct 29, 2020

I'm not an experiences Emacs package developer, so a review and feedback would be much appreciated!

With the global mode enabled, one can disable real-auto-save-mode on a per-buffer or per-major-mode basis. I found this easier for my setup than enabling auto saving for every conceivable mode.

I tested it this way:

;; init.el
(require 'real-auto-save)
(setq real-auto-save-interval 10) ;; in seconds
(defun turn-on-real-auto-save () (real-auto-save-mode 1))
(defun turn-off-real-auto-save () (real-auto-save-mode -1))
(define-globalized-minor-mode global-real-auto-save-mode
  real-auto-save-mode turn-on-real-auto-save)
(global-real-auto-save-mode 1)
(add-hook 'message-mode-hook #'turn-off-real-auto-save)

Auto-saving in .txt files works, auto-saving in message drafts is disabled. Manually disabling this in a buffer via M-: (real-auto-save-mode -1) works, too, and doesn't affect the next file you visit (i.e. auto-saving is still enabled by default).

@ChillarAnand
Copy link
Owner

ChillarAnand commented Nov 2, 2020

Any suggestions? @conao3

@conao3
Copy link
Collaborator

conao3 commented Nov 3, 2020

I think this is a good idea, but real-auto-save-initialize is better than turn-on-real-auto-save because turn-on-* is not well known prefix.
And turn-off-real-auto-save is not needed I think.

@DivineDominion
Copy link
Author

DivineDominion commented Nov 3, 2020

Thanks for the feedback! I got the inspiration for turn-on-* and turn-off-* from existing packages. Most are not interactive, but if you hit M-: (turn-on TAB (eval-expression, then type (turn-on, then TAB to get auto-completion) you'll see quite a few packages use this. (turn-off-* is far less common, that's right)

In Magit's git-commit.el, for example, you'll find calls to (turn-on-flyspell) and (turn-on-autofill). I just picked a random package from my currently frontmost buffer's mode list.

I think we can live without these functions, but I also think they are useful API. Users can then declare

(add-hook 'message-mode-hook #'turn-off-real-auto-save)

... and don't need to create lambdas or whatever.

Please note I'm not sure if this is still considered weird. I just picked this up during the past weeks and liked the convention.

@conao3
Copy link
Collaborator

conao3 commented Nov 3, 2020

OK, thanks. I'm not professional too, but I follow package-lint suggestion.
Please install package-lint and see warnings.

 real-auto-save.el   104   1 error           "turn-on-real-auto-save" doesn't start with package's prefix "real-auto-save". (emacs-lisp-package)
 real-auto-save.el   104     info            All variables and subroutines might as well have a documentation string (emacs-lisp-checkdoc)
 real-auto-save.el   105   1 error           "turn-off-real-auto-save" doesn't start with package's prefix "real-auto-save". (emacs-lisp-package)
 real-auto-save.el   105     info            All variables and subroutines might as well have a documentation string (emacs-lisp-checkdoc)

So, I suggest to use real-auto-save-* prefix.
And I think, the add-hook you suggestion is not needed. Please define real-auto-save-initialize as like below.

(defun real-auto-save-initialize ()
  (when buffer-file-name (real-auto-save-mode))

@conao3
Copy link
Collaborator

conao3 commented Nov 3, 2020

I usually use (add-hook 'find-file-hook 'real-auto-save-mode), and it is fine I think, but the global-minor-mode you suggest is the better idea.

@DivineDominion
Copy link
Author

Whoa that year went by fast. I addressed some of your comments @conao3 but didn't recall if you want the -turn-off function to be removed completely (then you cannot opt-out of global mode easily), and if you want -turn-on to be renamed to -initialize.

Grepping through the emacs lisp sources, I only found a handful of uses of the turn-on- prefix, notably turn-on-auto-revert-mode. In my own emacs config, I only find 22 completions for turn-on-. I have 32 matches for -initialize$, but most of these do something else than my proposed functions, like perform setup work. I found yas-initialize to be like the turn-on-convention, but it toggles the global mode (which makes more sense for YASnippets). Btw I found 5 matches for -turn-on$ suffix.

TL;DR: the '-initialize` suffix sounds like it'd be misleading.


I'm not a fan of the -turn-on and -turn-off suffix because it reads weird. I'm fine with removing the helpers completely and just add the global minor mode, then add the turn-off- helper in my own config for consistency and that's it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants