[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Bug: org-archive-subtree-save-file-p logic [9.3.6 (release_9.3.6-399

From: Kyle Meyer
Subject: Re: Bug: org-archive-subtree-save-file-p logic [9.3.6 (release_9.3.6-399-ge6df03 @ /home/n/.emacs.d/straight/build/org/)]
Date: Tue, 07 Apr 2020 00:37:23 +0000

No Wayman <address@hidden> writes:

> What do you think of something like this?

Thanks for the suggestion.  The code is somewhat oddly formatted, at
least on my end.  Could you send a proper git-format-patch output to
this thread (either via git-send-email or as an attachment)?

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

"an agenda view" sounds better to me and matches the original text from

> The value 'only-agenda saves only when the archive is initiated 
> from the agenda-view.

nitpick: Please write symbols as `foo' rather than 'foo.

Also, if you're going to expand the docstring (not a bad thing), I think
it'd make sense to slim down the :tag text a bit.
org-archive-save-context-info provides a nice example of formatting the
docstring, including the list of values.

> The value t saves in all cases where the archive target buffer is 
> not the current buffer.
> The value nil prevents saving in all cases."
>   :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 "Only save archive buffer when archiving 
>           from an agenda view"   only-agenda)

As I mentioned above, I have a slight preference for sticking with
3d0282ef8's names: from-org and from-agenda.  I suppose the main
argument against from-org is that it's not clear from the name alone
that it's referring to a non-agenda Org buffer because "org" is of
course a bit overloaded.  Considered alongside from-agenda, I don't
think it's too bad though.

>           (const :tag "Save the archive buffer unless it is the 
>           current buffer" t)

This current buffer bit also applies to unless-agenda/from-org.  Perhaps
it'd make sense to just mention the current buffer behavior in the main
docstring, given it applies to all options (even though for
only-agenda/from-agenda, it's never the case that the archive buffer is
the current buffer).

In summary

  * I'd prefer to make a more minimal change on top of 3d0282ef8,
    keeping the names chosen there.  Functionally that comes down to
    adjusting the condition that guards the save-buffer call to consider

  * I think it'd be good to expand the docstring (along the lines of
    what you suggested) as well as trim and clarify the tag text a bit.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]