emacs-helm / helm-dictionary

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

Some more thoughts #5

Closed michael-heerdegen closed 10 years ago

michael-heerdegen commented 10 years ago

Hi,

after working a bit with the code, I have questions aboutt the following points:

  1. :candidate-number-limit 500. Is it useful/realistic to use such a huge number? Nobody can read 500 items in a reasonable amount of time. OTOH, it probably doesn't harm.
  2. (action . (("Insert German term" . helm-dictionary-insert-l1term) ("Insert English term" . helm-dictionary-insert-l2term)))

I think it's not good to hardcode language names, because this package can be used for dictionaries of other languages as well. Maybe we should call the actions "Insert from left|right side" or so?

  1. helm-dictionary-word-at-point' - is this really useful? You can always use C-w (helm-yank-text-at-point') to insert the current word. Defining an extra command seems a bit like overkill.
tmalsburg commented 10 years ago

candidate-number-limit

I used 100 at first but then changed it to 500. It depends on your typical use cases I guess. I use the en-de dictionary which often returns many entries.

Insert English term

Right, forgot to change that after I found out that there are other dictionaries using that format.

helm-dictionary-word-at-point

I didn't know about helm-yank-text-at-point. So, yes, I agree, it's perhaps overkill to have an extra command. Alhthough, C-w uses only the part of the word at point to the left of the cursor. Not sure if I like that behavior.

Thanks for the comments!

michael-heerdegen commented 10 years ago

Ok, thanks. Let's keep 1. and 3. as is. Can you please DTRT for the hardcoded language names?

tmalsburg commented 10 years ago

Yes, I will take care of it. Thanks for the suggestions.

tmalsburg commented 10 years ago

Just fixed the titles of the actions to use source and target language instead of German and English: 1d6042d085a2924305294d61107ac8cd083392d6