Bug: org-archive-subtree-save-file-p logic [9.3.6 (release_9.3.6-399-ge6df03 @ /home/n/.emacs.d/straight/build/org/)]

[No Wayman <iarchivedmywholelife@gmail.com>]

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.

#+begin_src emacs-lisp
;; Save and kill the buffer, if it is not the same
;; buffer and depending on `org-archive-subtree-save-file-p'
(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)))
#+end_src

While patching this, I realized I'm not sure I understand the 
intended logic of each value for org-archive-subtree-save-file-p.
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

The value 'from-org also still saves the archive buffer when 
archiving from a buffer that is not in Org mode.
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:

#+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

If this is the intended logic, I think it could be cleaned up even 
more so that it reads better and I'm happy to submit a patch.
If not, I'm interested to hear the actual case.

Thanks


Emacs  : GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X 
toolkit, cairo version 1.17.3, Xaw3d scroll bars)
 of 2020-02-23
Package: Org mode version 9.3.6 (release_9.3.6-399-ge6df03 @ 
/home/n/.emacs.d/straight/build/org/)