emacs-helm / helm-dictionary

Helm source for looking up dictionaries
31 stars 12 forks source link

helm-source-dictionary-online: should never be narrowed. #15

Closed thierryvolpiatto closed 10 years ago

thierryvolpiatto commented 10 years ago

As you told me this source should not match against helm-pattern. Using dummy attr doesn't help here, here what you can do:

(defvar helm-source-dictionary-online
  `((name . "Lookup online")
    (match (lambda (_candidate) t))
    (candidates . helm-dictionary-online-dicts)
    (nohighlight)
    (action
     . (lambda (cand)
         (let ((browse-url-browser-function
                (or helm-dictionary-browser-function
                    browse-url-browser-function)))
           (helm-browse-url (format cand (url-hexify-string helm-pattern)))))))
  "Source for online lookup.")
thierryvolpiatto commented 10 years ago

BTW you can use now either the two form below to define a match function with a lambda:

(match (lambda (_candidate) t)) or (match . (lambda (_candidate) t))

tmalsburg commented 10 years ago

Your solution is more elegant but there is a problem: every time I enter a character the order of the entries changes. That's somewhat confusing. The order of entries should be the same as in the customization variable helm-dictionary-online-dicts.

michael-heerdegen commented 10 years ago

I don't see this here, with neither branch.

tmalsburg commented 10 years ago

I didn't commit it because it doesn't work properly. Or do you mean that you tried it in your local version and it worked?

michael-heerdegen commented 10 years ago

Yes, I tried it by evaluating the defvar that Thierry posted (with C-M-x, of course, I'm sure it was evaluated). Now I tried a second time with emacs -Q, and there I didn't see your order problem neither.

michael-heerdegen commented 10 years ago

Also @thierryvolpiatto: OOps, after upgrading helm now, I see this, too! So this must be related to a recent change to helm, maybe the

(match (lambda (_candidate) t))

vs. (match . (lambda (_candidate) t))

change? Thierry, can you please have a look?

michael-heerdegen commented 10 years ago

No, after downgrading helm again, I still see it. I did just not enter the "right" (or wrong) characters the first time. Conclusion: Yes, I can reproduce the problem, sorry for the noise.

thierryvolpiatto commented 10 years ago

michael-heerdegen notifications@github.com writes:

No, after downgrading helm again, I still see it. I did just not enter the "right" (or wrong) characters the first time. Conclusion: Yes, I can reproduce the problem, sorry for the noise.

No I can't reproduce using my branch. And no it can't be related to the last changes I did in helm.el. Try with my branch:(master) https://github.com/thierryvolpiatto/helm-dictionary

But wait, you are still using the dummy plugin ? That may be the reason.

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

tmalsburg commented 10 years ago

No dummy plugin. I used exactly what you proposed above. My Emacs is a fairly recent development version (perhaps three weeks old) and I installed Helm from Melpa today.

tmalsburg commented 10 years ago

Another strange thing: sometimes matches in the online source are highlighted although (nohighlight) is defined. While I type, highlighting randomly switched on and off.

michael-heerdegen commented 10 years ago

Thierry, it doesn't always happen, it depends on what string you are entering. For me, it always happens when I enter "abc" (master of this package, recent helm). It happens with emacs -Q. Please retry, I'm sure you'll see the phenomenon.

tmalsburg commented 10 years ago

Confirm: master branch + "abc" reproduces the problem in my setup as well.

thierryvolpiatto commented 10 years ago

Which master branch ? your master branch here doesn't work at all (wrong-type-argument: stringp nil) On my master branch I can't reproduce, the online entries stay in the same order whatever I type (abc, def ...) However 'nohighlight' have been implemented for working in a single source so it is not source independent. I have to fix this among other (many!) things.

michael-heerdegen commented 10 years ago

Thierry, this was exactly what I first experienced, too. One of the things I missed was that the name of the online source changed, so be sure that you really use your proposed source definition. I think there was another trivial reason for the (wrong-type-argument: stringp nil) error, but I don't remember. Maye you need to restart Emacs.

tmalsburg commented 10 years ago

My master branch. I use it right now, no errors.

tmalsburg commented 10 years ago

Just tried it in the candidates-file branch. There, I can't reproduce the problem. You, Michael?

michael-heerdegen commented 10 years ago

Yes, I could reproduce it there, too But note that the source has a different name in this branch, so be sure your defvar is used at all! And I had to bind :candidate-number-limit to 20 in `helm-dictionary' to see the online source early enough to let the effect happen.

michael-heerdegen commented 10 years ago

@thierryvolpiatto Here is a recipe for helm.sh, derived from your proposal, showing the problem:

(setq test-candidates
    '(("translate.reference.com de->eng" .
       "http://translate.reference.com/translate?query=%s&src=de&dst=en")
      ("translate.reference.com eng->de" .
       "http://translate.reference.com/translate?query=%s&src=en&dst=de")
      ("leo eng<->de" .
       "http://dict.leo.org/ende?lp=ende&lang=de&search=%s")
      ("en.wiktionary.org" . "http://en.wiktionary.org/wiki/%s")
      ("de.wiktionary.org" . "http://de.wiktionary.org/wiki/%s")
      ("linguee-eng<->de"    . "http://www.linguee.de/deutsch-englisch/search=%s")))

(setq helm-source-test
  `((name . "Test")
    (match (lambda (_candidate) t))
    (candidates . test-candidates)
    (nohighlight)
    (action . #'ignore)))

(defun helm-test ()
  (interactive)
  (helm :sources 'helm-source-test
        :full-frame t
        :buffer "Test"))

;; M-x helm-test a b c
thierryvolpiatto commented 10 years ago

michael-heerdegen notifications@github.com writes:

@thierryvolpiatto Here is a recipe for helm.sh, derived from your proposal, showing the problem:

(setq test-candidates
    '(("translate.reference.com de->eng" .
       "http://translate.reference.com/translate?query=%s&src=de&dst=en")
      ("translate.reference.com eng->de" .
       "http://translate.reference.com/translate?query=%s&src=en&dst=de")
      ("leo eng<->de" .
       "http://dict.leo.org/ende?lp=ende&lang=de&search=%s")
      ("en.wiktionary.org" . "http://en.wiktionary.org/wiki/%s")
      ("de.wiktionary.org" . "http://de.wiktionary.org/wiki/%s")
      ("linguee-eng<->de"    . "http://www.linguee.de/deutsch-englisch/search=%s")))

(setq helm-source-test
  `((name . "Test")
    (match (lambda (_candidate) t))
    (candidates . test-candidates)
    (nohighlight)
    (action . #'ignore)))

(defun helm-test ()
  (interactive)
  (helm :sources 'helm-source-test
        :full-frame t
        :buffer "Test"))

;; M-x helm-test a b c

Ok I got it, thanks for the recipe, I could reproduce, the reason is that the match function is always returning t but the other match-plugin function are still running. Disabling match-plugin in this source with no-matchplugin fix the problem.

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

michael-heerdegen commented 10 years ago

Thanks, that fixes it indeed! Titus, can you please make that change?

tmalsburg commented 10 years ago

Fixed in 7e9fff83ce7c3505e0a4bc8c75aaec9153746493. Thanks Thierry and Michael!