syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.69k stars 4.9k forks source link

Org extensions not correctly loaded with (spacemacs|use-package-add-hook) #9979

Closed smile13241324 closed 4 years ago

smile13241324 commented 6 years ago

Description :octocat:

Various org extensions are not correctly loaded when the layer uses (spacemacs|use-package-add-hook org :post-config (require 'ox-....)) for loading. This can be observed by having a look at the org-export view which is missing all ox-twbs export choices until the export is triggered by hand.

First I suspected that the issue was somehow related to the loading order of the packages in the layer, causing org to be already loaded before the :post-config hook could be added. As a result I did replace the call to (spacemacs|use-package-add-hook org :post-config (require 'ox-....)) with (use-package ox-... :after ox) for which I have opened the merge request #9797.

Nevertheless I have now noticed that more layers are affected i.e. the restclient layer is failing to load ob-http via (spacemacs|use-package-add-hook) so I think it would be worth the time to actually have a look at (spacemacs|use-package-add-hook) to make that macro work independent of the package load order. Also I would like to mention that my elisp-fu is not deep enough to really understand of how the additional hook is added so it may be that the issue is not related to the package load order at all.

I think this really needs to be fixed as it breaks a ton of functionality in various packages right now.

Reproduction guide :beetle:

Observed behaviour: :eyes: :broken_heart: Org export does not show ox-twbs export choices.

Expected behaviour: :heart: :smile: Org export shows the ox-twbs export choices.

System Info :computer:

nixmaniack commented 6 years ago

I haven't checked this issue yet but there were some changes related to use-package hooks at upstream which might have triggered this. This used to work.

kaushalmodi commented 6 years ago

@nixmaniack This issue has been there for a while apparently https://github.com/syl20bnr/spacemacs/issues/9598.

I had found this issue when I was trying to integrate ox-hugo into spacemacs.. then found this workaround: https://github.com/syl20bnr/spacemacs/pull/9705/files#diff-2eb4bf4b13a5a4a96b6eda3d92ae8d0eR610

See if loading ox-twbs using the same way (:after ox) helps.

smile13241324 commented 6 years ago

@nixmaniack @kaushalmodi yes that's the issue why I originally made the PR I have mentioned above, replacing the macro with the :after ox style in the org layer, which would fix the issue. But a quick helm search shows that the (spacemacs|use-package-add-hook org :post-config (require 'ox-....)) is used in many other layers which are not working either.

But the PR has not been merged yet and now that I have seen that the issue is also happening for the other layers I think we should now have a discussion of how to proceed here.

I see now two possible ways of action:

@JAremko @bmag @syl20bnr @deb0ch what do you think how should we proceed?

syl20bnr commented 6 years ago

I regularly use ox-gfm and it is in the export menu.

Is it possible that you have some code that would load org-mode directly and bypass the lazy-loading mechanic ?

Anyway there is indeed a limitation with the alphabetical loading order of packages and use-package hook. I may have some possible fixes.

smile13241324 commented 6 years ago

@syl20bnr Good point I have all org stuff in a with eval after load block so this should not trigger org loading but I will try again with a clean config.

You have mentioned ox-gfm do you also see the export choices from ox-twbs?

smile13241324 commented 6 years ago

@syl20bnr I have debugged my config a little and I can trigger the issue when I deactivate my entire use config and load sml layer after I have loaded org.

Having a deeper look into package.el of sml layer I was able to identify an init function of an org extension which is triggering that issue for me.

The block is

(defun sml/init-ob-sml ()
  (use-package ob-sml
    :defer t
    :init
    (org-babel-do-load-languages 'org-babel-do-load-languages '(sml . t))))

When I deactivate that block I can load the layer and all export choices are available. Same when I reactivate my original config without sml layer.

nixmaniack commented 6 years ago
(defun sml/init-ob-sml ()
  (use-package ob-sml
    :defer t
    :init
    (org-babel-do-load-languages 'org-babel-do-load-languages '(sml . t))))

This seems to be a mistake. It should be something as follows to make use of lazy loading.

(defun sml/init-ob-sml ()
  (spacemacs|use-package-add-hook org
    :post-config
    (use-package ob-sml
      :init (add-to-list 'org-babel-load-languages '(sml . t)))))

@smile13241324 Can you check if it helps?

smile13241324 commented 6 years ago

@nixmaniack I have reenabled the layer and replaced the old code with your new one. This did the trick I can still do all the exports.

nixmaniack commented 6 years ago

Thanks for the feedback. I have made the PR to fix this issue.

smile13241324 commented 6 years ago

@nixmaniack thanks for the fix. And sorry for all that fuss I really thought that this was a more general issue @syl20bnr.

syl20bnr commented 6 years ago

@nixmaniack thank you for the fix. Should be fixed in develop.

syl20bnr commented 6 years ago

I pushed in develop the commit https://github.com/syl20bnr/spacemacs/commit/e9fb5285

Basically it makes all the pre-init functions of all packages to be executed before the init and post-init.

Before it was:

pre-init-A
init-A
post-init-A
pre-init-B
init-B
post-init-B

Now it is:

pre-init-A
pre-init-B
init-A
post-init-A
init-B
post-init-B

So we can safely set the use-package hooks in pre-init functions. In the example above it is now possible to add a use-package hook for init-A in pre-init-B (previously it was not impossible).

myrgy commented 6 years ago

@syl20bnr , why do we call post-init not after all init calls? As for me it's expected that flow is pre-init/ init / post-init, for all packages. So my proposal is to make it symmetric:

pre-init-A
pre-init-B
init-A
init-B
post-init-A
post-init-B

UPD: because otherwise this callbacks don't make any sense - we can just merge it with init.

syl20bnr commented 6 years ago

You are right, I thought about it and was lazy to do it :-D Let's wait a few days to see if everything is fine with the current change and then we can extend it to post-init.

syl20bnr commented 6 years ago

@myrgy BTW I've done what you requested.

syl20bnr commented 6 years ago

@smile13241324 Can you verify if the original issue is fixed in latest develop. I pushed a fix to initialize installed org very early when Emacs starts so I think it should fix lots of issues regarding the wrong version of org being loaded before Org's ELPA one.

smile13241324 commented 6 years ago

@syl20bnr I can confirm that the issue is gone on develop. Should I close the ticket?

github-actions[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!

smile13241324 commented 4 years ago

This is gone and therefore I have closed this issue