No Wayman <iarchivedmywholel...@gmail.com> writes: > The logic for saving the archive buffer in org-archive-subtree > does not consider the (default) value of 'from-org for > org-archive-subtree-save-file-p.
Hmm, indeed, the change from 3d0282ef8 (New option `org-archive-subtree-save-file-p', 2020-01-31) looks incomplete. > While patching this, I realized I'm not sure I understand the > intended logic of each value for org-archive-subtree-save-file-p. >From digging a bit, I think the history went like this: * All the way back in 4be4c5623 (version 4.12a, 2008-01-31), org-archive-subtree saved the archive buffer, except when it was the current buffer. [ The next two aren't relevant for the save part, but I'm noting because we should cleaned them up. ] * As of bf09955fe (Bugfix for `org-archive-subtree', 2008-03-01), the buffer was killed after being saved. * 343f3c478 (Keep archive buffer after archiving something to it, 2009-10-28) reverted the killing, though it left behind a stale comment saying the buffer will be killed. * 63f6e851b (Do not save target buffer after archiving subtree, 2017-11-25) stopped saving the buffer for performance reasons. * Some users preferred the older behavior. An associated Debian bug was opened. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887332 https://yhetil.org/orgmode/f74fce15-c2ea-048e-b2ef-7ad40e717...@arfer.net/ In response, Bastien committed 3d0282ef8 to offer more control over when saving happens. > When setting it to 't', the defcustom :tag string claims "Always > save the archive buffer". > This is not the case if archiving from within the current buffer. > Perhaps a clearer :tag string? > > #+begin_src emacs-lisp > (defcustom org-archive-subtree-save-file-p 'from-org > ;;... > (const :tag "Save the archive buffer unless it is the current > buffer" t) > ;;... > )) > #+end_src Yeah, I'd say that's an improvement, though the same "unless it is the current buffer" would be true for from-org as well, were it wired up. > The value 'from-org also still saves the archive buffer when > archiving from a buffer that is not in Org mode. I'm confused by this statement. To me it looks like save-buffer will _never_ be called when org-archive-subtree-save-file-p is from-org: (unless (eq this-buffer buffer) (when (or (eq org-archive-subtree-save-file-p t) (and (boundp 'org-archive-from-agenda) (eq org-archive-subtree-save-file-p 'from-agenda))) (save-buffer))) > I'm not entirely sure of its purpose. If the intent is to allow an > option that prevents saving only when archiving from the agenda, > I suggest a single option excluding that case and saving for > other, non-nil values: Based on the history above, I believe the main purpose is to give users a way to reverse the "no saving" behavior made in 63f6e851b (Do not save target buffer after archiving subtree, 2017-11-25). I'm _guessing_ that, on top of that, the idea adding a from-agenda value was that some users may want to save only when archiving from the agenda, because in that case they're a bit removed from the buffer and might not think to save it, or something along those lines. > #+begin_src emacs-lisp > (defcustom org-archive-subtree-save-file-p 'unless-agenda > "Conditionally save the archive file after archiving a subtree. > The value 'unless-agenda prevents saving from the agenda-view. > Other non-nil values save the archive buffer unless it is the > current buffer." > :group 'org-archive > :package-version '(Org . "9.4") > :type '(choice > (const :tag "Do not save archive buffer when archiving > from an agenda view" unless-agenda) > (const :tag "Save the archive buffer unless it is the > current buffer" t) > (const :tag "Do not save the archive buffer"))) > #+end_src > > Then the saving logic in org-archive-subtree becomes: > > #+begin_src emacs-lisp > (when org-archive-subtree-save-file-p > (unless (or (eq buffer this-buffer) > (and (eq org-archive-subtree-save-file-p > 'unless-agenda) > ;;bound when called from org-agenda.el > (boundp 'org-archive-from-agenda))) > (save-buffer))) > #+end_src Assuming what I said above is true, I think what you propose here loses the ability to save only when archiving from the agenda. And more importantly, users would not be able to give a blanket "don't save" in order to retain the behavior introduced by 63f6e851b (Do not save target buffer after archiving subtree, 2017-11-25).