Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow helm-dictionary-database to be an alist #27

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

astoff
Copy link
Contributor

@astoff astoff commented May 30, 2016

This is concerning issue #26.

With this change, helm-source-dictionary becomes a list of helm sources. I wasn't sure whether renaming the variable is desirable.

@tmalsburg
Copy link
Member

This addresses the second paragraph in #26 but not the first, right? I wonder what use case you had in mind. Is this PR for situations were you have several dictionaries for one language?

@astoff
Copy link
Contributor Author

astoff commented May 30, 2016

Yes, this only addresses the second paragraph in #26. The use case I have in mind is the following: if you use 2 or 3 bilingual dictionaries (which may or may not involve more than 2 languages), then it is easier to just search all of them at the same time.

As to the first paragraph in #26, yes, one could add an optional argument to helm-dictionary which defaults to helm-source-dictionary, but maybe this is overkill... Copying and modifying the definition of helm-dictionary is pretty easy (and the new function helm-dictionary-build comes handy here).

@tmalsburg
Copy link
Member

I'm only using a German-English dictionary that is very comprehensive, so there is no need to use several dictionaries. But I know that dictionaries for other language pairs are often much smaller and there it might make sense to use several dictionaries simultaneously. However, there is one very simple hack to achieve this without making any changes in helm-dictionary: just concatenate the dictionary files. In both cases, duplicates are a problem, so it would be preferable to merge dictionaries in some cleaner way, but that may not be trivial.

Anyhow, I think your PR is useful and some people may even use it to combine dictionaries from different language pairs.

@astoff
Copy link
Contributor Author

astoff commented May 30, 2016

I personally use 3 dictionaries. Wiktionary's en->pt and pt->en (precisely for the reason you mentioned) and Ding's de->en. I don't find the need to have 2 separate commands to look up Portuguese and German words.

@tmalsburg
Copy link
Member

I just tested it with this configuration:

(setq helm-dictionary-database '(("Ding" . "/usr/share/trans/de-en")
                                 ("Ding 2" . "/usr/share/trans/de-en")))

I expected to see two sections showing the same dictionary (I have only one dictionary installed) but instead I just see one section. Am I missing something?

@astoff
Copy link
Contributor Author

astoff commented Jun 1, 2016

That should work, but you need to reload helm-dictionary or perhaps restart emacs — the source list helm-source-dictionary is built when the library is loaded.

@tmalsburg
Copy link
Member

I see. When I define helm-dictionary-database before loading helm-dictionary it works ok. But it's not ideal that users have to restart Emacs when they change the dictionary. I'll merge and change it a bit.

@tmalsburg tmalsburg merged commit 9e80ea2 into emacs-helm:master Jun 1, 2016
@tmalsburg
Copy link
Member

See 21de838.

@thierryvolpiatto
Copy link
Member

Titus von der Malsburg [email protected] writes:

But it's not ideal that users have to restart Emacs when they change
the dictionary.

I generally use a set function in the defcustom that rebuild source, of
course people using setq have to restart emacs.

Thierry

@tmalsburg
Copy link
Member

Thanks, @thierryvolpiatto. I'm now generating the sources on the fly which also works when setq is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants