Ihor Radchenko <[email protected]> writes:
>
> I think you can simply make it an internal helper function inside
> org-attach.el. Then, you will not have problems with variables.
> Aside, the way to declare external variable is (defvar variable), but
> that should normally be the last resort (Org overuses this technique,
> leading to many internal dependency issues)
>
> If you want to generalize that specific menu layout, it is a good idea
> to find another existing command that will also use it. Otherwise, IMHO,
> it will be overengineering. (Can be done, but probably not worth the
> effort *now*).
>

Ok, I removed the org-agenda style menu system, that really is the
sensible solution.

>> I have one problem: when compiling org-mode, I get a
>>
>> org-agenda.el:2499:46: Warning: the function ‘org-attach’ is not known
>> to be defined.
>>
>> I'm not sure how I can tell the compiler that org-attach is defined?
>
> declare-function. Although I am not really sure how your changes
> triggered this warning. There are many examples in org-agenda.el at the
> beginning of the file.

That was an easy fix, thanks!

>
>> Also, if we port org-attach to org-menu, should we enable org-menu-mode
>> by default?  That will change the behaviour of the basic citation
>> handler, but this feels more like a feature enhancement than something
>> that would upset people.
>
> Good question.
> The dumbest thing we can do here is simply
>
> (org-menu-define org-attach ()
>   "The dispatcher for attachment commands.
> Shows a list of commands defined in `org-attach-commands' using `org-menu'."
>   :menu org-attach--commands-to-transient-specification
>   :default-action (org-attach--old-org-attach-menu-system nil nil)
>   :override org-attach--old-org-attach-menu-system)
>

My feeling is that that solution is less nice than just enabling
org-menu by default. The default behaviour we are protecting by leaving
it disabled is that citations automatically open to the bibliography
file. I do not think this is very useful?

>> +(defcustom org-menu-disable-overrides nil
>> +  "If this is true, `org-menu-system-overrides' will be ignored."
>> +  :group 'org-menu
>> +  :package-version '(Org . "9.8")
>> +  :type 'boolean)
>
> Not sure about this one. Some commands simply cannot support transient
> or other menu systems.

If a command can not use alternative menu systems, what is the benefit
of porting it to org-menu? If there is any case where this makes sense,
then org-menu-disable-overrides has to go, but otherwise I like the
option of guaranteeing that all menus are presented in the same way.

I have rebased against master, and make now runs without errors. When we
have decided whether org-menu-mode should be enabled by default and if
we should includ org-menu-disable-overrides I'll make a new patch set:-)

Cheers,
Tor-björn

Reply via email to