search

emacs-helm / helm-org

53 stars 9 forks source link

Autoload menu causes error on startup #5

Closed thanhvg closed 1 year ago

thanhvg commented 5 years ago

Hi, I don't think autoload creating menu item is a good idea. Since helm-org is now a standalone pacakge, there's no need to add it to the core menu item.

The real problem is the autoload part is not a function definition, it is a computation that actually runs on package activated. This causes an error because at that time I guess easymenu has no clue what keymap to work with and I got this error on startup with use-package:

Error (use-package): helm-org/:catch: Wrong type argument: keymapp, nil

Stack trace:

Debugger entered--Lisp error: (wrong-type-argument keymapp nil)
  define-key(nil [menu-bar Tools Helm] ("Helm" keymap "Helm"))
  easy-menu-get-map(nil ("Tools" "Helm") nil)
  easy-menu-add-item(nil ("Tools" "Helm" ....

My test config is basically as follows:

(use-package helm
  :ensure t)
(use-package helm-org
  :ensure t)

;; other org stuff

Thanks

thierryvolpiatto commented 5 years ago

thanhvg notifications@github.com writes:

My test config is basically as follows:

(use-package helm :ensure t) (use-package helm-org :ensure t)

This config is wrong, what you are loading is helm.el, then helm-org.el, please read the wiki on how to install helm.

@alphapapa

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

alphapapa commented 5 years ago

This much works for me:

  1. Start clean Emacs.
  2. Install package helm-org from MELPA.
  3. Run command helm-org-in-buffer-headings.

It works without errors.

thierryvolpiatto commented 5 years ago

What confuse people is that Helm and Helm-core are kind of virtual packages:

In Helm package, helm.el doesn't exists and in Helm-core package, helm-core.el doesn't exists, so when you do (use-package helm) all what is does is loading helm.el autoloads, in this case helm-easymenu.el is not loaded. The right thing to do is (require 'helm-config) which among other things load all the helm autoloads. Doing this is very fast, all the heavy helm libraries are not loaded, only the autoloads. Once this done, you can use use-package for helm-org or whatever helm files.

thanhvg commented 5 years ago

Thank you so much for time and explanation @thierryvolpiatto and @alphapapa I promise to read the wiki next time.

alphapapa commented 5 years ago

Thanks for that explanation, @thierryvolpiatto.

I think that points to an issue with the installation instructions. There are many words for the user to read between:

Install Helm with M-x package-install RET helm RET

and:

If installed via the Emacs package manager, add the following to your init file:

(require 'helm-config)

I'm guessing that few users read that far into the document.

It would be helpful if it was more like:

  1. Install Helm (via MELPA or from source; see below).
  2. Add to your init file: (require 'helm-config).

It would also help if those simple steps were on the readme instead of on the wiki (I'm guessing few users click through to the wiki at all), and perhaps in the package commentary, for people who install directly from MELPA and don't even visit the GitHub page.

What do you think?

thierryvolpiatto commented 5 years ago

alphapapa notifications@github.com writes:

Thanks for that explanation, @thierryvolpiatto.

I think that points to an issue with the installation instructions. There are many words for the user to read between:

Install Helm with M-x package-install RET helm RET

and:

If installed via the Emacs package manager, add the following to your init file:

(require 'helm-config)

I'm guessing that few users read that far into the document.

The problem AFAIU is that many users now a day take their informations from the diverses mailing list (reddit etc...) by asking there directly before reading any documentation.

It would be helpful if it was more like:

  1. Install Helm (via MELPA or from source; see below).
  2. Add to your init file: (require 'helm-config).

It would also help if those simple steps were on the readme instead of on the wiki (I'm guessing few users click through to the wiki at all), and perhaps in the package commentary, for people who install directly from MELPA and don't even visit the GitHub page.

What do you think?

Agree, I will do this perhaps next week.

Thanks.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

casch-at commented 5 years ago

I'm also facing this error message. I do have (require 'helm-config) in my init.el, but it's after (package-initialize) otherwise helm-config wouldn't be found.

Currently I have this before (package-initialize) to solve the problem:

(add-to-list 'load-path "/home/cschwarzgruber/.emacs.d/elpa/helm-20190817.525")
(require 'helm-config)

Any help would be appreciated.

Thank you!

thierryvolpiatto commented 5 years ago

Christian Schwarzgruber notifications@github.com writes:

I'm also facing this error message. I do have (require 'helm-config) in my init.el, but it's after (package-initialize) otherwise helm-config wouldn't be found.

Currently I have this before (package-initialize) to solve the problem:

(add-to-list 'load-path "/home/cschwarzgruber/.emacs.d/elpa/helm-20190817.525") (require 'helm-config)

Oh, yes I see, package-initialize is in addition of adding all packages to load-path loading the autoloads files of all those packages, so when helm-org autoload file is loaded helm-config have not been required yet and we have an error. I have now required helm-easymenu in helm-org, so this problem should be now fixed (I remember now I create helm-easymenu for this same problem in the past with other helm extensions).

So now, you should only need:

(package-initialize)
(require 'helm-config)

Let me know if you still have problems.

Thanks.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

casch-at commented 5 years ago

Thank you @thierryvolpiatto, but the error persists. At this stage the helm keymap is not known. After removing the ;;;###autoload statement in front of (easy-menu-add-item ...) the error is gone.

Debugger entered--Lisp error: (wrong-type-argument keymapp nil)
  define-key(nil [menu-bar Tools Helm] ("Helm" keymap "Helm"))
  easy-menu-get-map(nil ("Tools" "Helm") nil)
  easy-menu-add-item(nil ("Tools" "Helm") ("Org" ["Org headlines in org agenda files" helm-org-agenda-files-headings t] ["Org headlines in buffer" helm-org-in-buffer-headings t]) "Elpa")
  eval-buffer(#<buffer  *load*-341551> nil "/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521/helm-org-autoloads.el" nil t)  ; Reading at buffer position 487
  load-with-code-conversion("/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521/helm-org-autoloads.el" "/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521/helm-org-autoloads.el" nil t)
  load("/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521/helm-org-autoloads" nil t)
  package--activate-autoloads-and-load-path(#s(package-desc :name helm-org :version (20190818 521) :summary "Helm for org headlines and keywords completion" :reqs ((helm (3 3)) (emacs (24 4))) :kind nil :archive nil :dir "/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521" :extras ((:url . "https://github.com/emacs-helm/helm-org") (:maintainer "Thierry Volpiatto" . "thierry.volpiatto@gmail.com") (:authors ("Thierry Volpiatto" . "thierry.volpiatto@gmail.com")) (:commit . "792f30df10e6ee137385f792a49e997392323309")) :signed nil))
  package--load-files-for-activation(#s(package-desc :name helm-org :version (20190818 521) :summary "Helm for org headlines and keywords completion" :reqs ((helm (3 3)) (emacs (24 4))) :kind nil :archive nil :dir "/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521" :extras ((:url . "https://github.com/emacs-helm/helm-org") (:maintainer "Thierry Volpiatto" . "thierry.volpiatto@gmail.com") (:authors ("Thierry Volpiatto" . "thierry.volpiatto@gmail.com")) (:commit . "792f30df10e6ee137385f792a49e997392323309")) :signed nil) nil)
  package-activate-1(#s(package-desc :name helm-org :version (20190818 521) :summary "Helm for org headlines and keywords completion" :reqs ((helm (3 3)) (emacs (24 4))) :kind nil :archive nil :dir "/home/cschwarzgruber/.emacs.d/elpa/helm-org-20190818.521" :extras ((:url . "https://github.com/emacs-helm/helm-org") (:maintainer "Thierry Volpiatto" . "thierry.volpiatto@gmail.com") (:authors ("Thierry Volpiatto" . "thierry.volpiatto@gmail.com")) (:commit . "792f30df10e6ee137385f792a49e997392323309")) :signed nil) nil deps)
  package-activate(helm-org)
  package-initialize()
  eval-buffer(#<buffer  *load*> nil "/home/cschwarzgruber/.emacs" nil t)  ; Reading at buffer position 2637
  load-with-code-conversion("/home/cschwarzgruber/.emacs" "/home/cschwarzgruber/.emacs" t t)
  load("~/.emacs" t t)
  #f(compiled-function () #<bytecode 0x1dda7d>)()
  command-line()
  normal-top-level()

Next steps:

Or?

Thanks!

thierryvolpiatto commented 5 years ago

Christian Schwarzgruber notifications@github.com writes:

Thank you @thierryvolpiatto, but the error persists. At this stage the helm keymap is not known.

Hmm, I tried to install helm-org from Melpa, and I can't reproduce.

1) emacs -Q 2) Eval in scratch buffer:

(package-initialize)
(require 'helm-config)

I have no errors.

However when installing Helm like this I have no Helm menu, but this is another issue.

After removing the ;;;###autoload statement in front of (easy-menu-add-item ...) the error is gone. Next steps:

• Revert 792f30d

Yes of course this would fix your issue.

• Remove ;;;###autoload at line https://github.com/emacs-helm/helm-org/blob/792f30df10e6ee137385f792a49e997392323309/helm-org.el#L38

If you do this you will not have the Helm menu for helm-org until helm-org.el is autoloaded, which defeat the purpose of adding a menu entry which will be mostly use to discover Helm commands.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

sunlin7 commented 5 years ago

Hi @thierryvolpiatto, I also was affected by the error, and I create a PR to fix it, it just use the eval-after-load function to dependent on the helm-easymenu. Please review it, thank you. https://github.com/emacs-helm/helm-org/pull/6

thierryvolpiatto commented 5 years ago

Should work now, please confirm. Thanks. @tmalsburg you may be interested by this thread as you use helm-easymenu in helm-mu.

tmalsburg commented 5 years ago

Thanks, @thierryvolpiatto. I have to confess that I do not fully understand the autoload mechanism. Does this fix look okay to you: https://github.com/emacs-helm/helm-mu/commit/481964fb26c59ea280a1ec7bce192d724ddf7d12

thierryvolpiatto commented 5 years ago

Titus von der Malsburg notifications@github.com writes:

Thanks, @thierryvolpiatto. I have to confess that I do not fully understand the autoload mechanism.

Adding an autoload cookie add the form below (easymenu... for us) in the autoload file, this way the helm-mu commands are visible in menu (and callable from there) before helm-mu.el is loaded.

Does this fix look okay to you: emacs-helm/helm-mu@481964f

Yes.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

casch-at commented 5 years ago

Thanks, @thierryvolpiatto 542dda7bc9a3b9dfb439e4f8a1e5f60cfb6cc256 fixed it.

So basically it didn't work because during autoloading helm-easymenu wasn't loaded?

Thank you!

thierryvolpiatto commented 5 years ago

Christian Schwarzgruber notifications@github.com writes:

Thanks, @thierryvolpiatto 542dda7 fixed it.

So basically it didn't work because during autoloading helm-easymenu wasn't loaded?

Yes.

Thank you!

Thanks for confirmation ;-)

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

emacs18 commented 5 years ago

The problem still remains for me when I launch spacemacs. The solution I implemented for me is to just remove the line with easy-menu-add-item within helm-org-atoloads.el. This of course would not be needed if instead the autoload cookie ";;;###autoload" is removed just above easy-menu-add-item in helm-org.el as was suggested by someone before.

Spacemacs activates packages in alphabetical order. The package activation sequence is the following where I do not show any package that does not have "helm" in the package name.

ace-jump-helm-line default-helm-config flyspell-correct-helm helm helm-ag helm-c-yasnippet helm-company helm-css-scss helm-dash helm-descbinds helm-flx helm-git-grep helm-gitignore helm-gtags helm-lsp helm-make helm-mode-manager helm-org helm-org-rifle helm-projectile helm-purpose helm-pydoc helm-rtags helm-spacemacs-faq helm-spacemacs-help helm-swoop helm-themes helm-xref

Thus helm package and few other helm packages were already activated before helm-org is activated. Still mere package activation does not seem to execute (require 'helm-config).

thierryvolpiatto commented 5 years ago

Please stop with this, all is working fine now. If something is wrong with spacemacs, report to spacemacs, anyway there is always problems with spacemacs.

emacs18 commented 5 years ago

The problem was my error, not spacemacs. I missed the fact that two changes were made on Aug 19. I was using the first change of that day rather than second one. This in turn was due to a bug in my script that built packages.

Sorry.