search

jacktasia / dumb-jump

an Emacs "jump to definition" package for 50+ languages
GNU General Public License v3.0
1.57k stars 150 forks source link

Call helm-make-source directly #400

Closed psibi closed 3 years ago

psibi commented 3 years ago

Avoids macro expansion issues if Helm is not yet loaded.

Fixes issue in environment like NixOS. Fixes https://github.com/jacktasia/dumb-jump/issues/224

psibi commented 3 years ago

Tested that the CI pases fine in my fork. And this change seems to have fixed it in my NixOS setup.

jacktasia commented 3 years ago

Thanks for opening this! This lgtm, but curious what @phikal thinks

phikal commented 3 years ago

I'm not familiar with Helm, so I cannot say much. Could @psibi explain what the issue specifically with NixOS is in more detail?

psibi commented 3 years ago

@phikal This was what was happening in NixOS:

This is my configuration:

(use-package dumb-jump
  :ensure t
  :bind (("M-g o" . dumb-jump-go-other-window)
         ("M-g b" . dumb-jump-back))
  :config (progn
            (setq dumb-jump-selector 'helm)
            (setq dumb-jump-force-searcher 'rg)))

Steps to reproduce:

dumb-jump-prompt-user-for-choice: Invalid function: helm-build-sync-source

A quick google suggests that this is a issue in Nix systems: https://github.com/alphapapa/org-ql/issues/60#issuecomment-559910327

I applied the same fix of org-ql in dumb-jump and it started working fine for me. Also it seems other nix users of dumb-jump are facing the same issue: https://github.com/jacktasia/dumb-jump/issues/224#issuecomment-562897799 Unfortunately, I didn't dig more deeply to find more about the issue.

phikal commented 3 years ago

I'm going to assume there are no downsides to this, so I don't see any reason to object.