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

(feat) add jisho.org #1030

Merged
merged 4 commits into from
Apr 9, 2022
Merged

(feat) add jisho.org #1030

merged 4 commits into from
Apr 9, 2022

Conversation

austinhuang0131
Copy link
Contributor

@austinhuang0131 austinhuang0131 commented Mar 31, 2022

What does this PR do?

Add https://jisho.org as an engine. Note that this only supports searching words (either English or Japanese input).

example

A longer infobox:

infobox

Why is this change important?

¯\_(ツ)_/¯

How to test this PR locally?

make run test

Author's checklist

Related issues

Closes #1016

Copy link
Member

@dalf dalf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments here and there

searx/engines/jisho.py Show resolved Hide resolved
searx/engines/jisho.py Outdated Show resolved Hide resolved
infobox_content += f"<li><i>{pos}</i>: {engdef}"
if extra != '':
infobox_content += f" ({extra})"
infobox_content += '</li>'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it always close an <i> ?
What about infobox_content += f" ({extra})" and infobox_content += '</ul><small>Wikipedia, CC BY-SA 3.0.</small><ul>' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean <li>

What about infobox_content += f" ({extra})"

There always exist engdef. Plus <li> is always appended since I have both an if and an else

and infobox_content += '</ul><small>Wikipedia, CC BY-SA 3.0.</small><ul>'

At the very top I exclude any entries whose first entry is Wikipedia. My understanding is that the dictionary entries are always before Wikipedia entries.

<small><a href="https://www.edrdg.org/wiki/index.php/JMdict-EDICT_Dictionary_Project">JMdict</a>
and <a href="https://www.edrdg.org/enamdict/enamdict_doc.html">JMnedict</a>
by <a href="https://www.edrdg.org/edrdg/licence.html">EDRDG</a>, CC BY-SA 3.0.</small><ul>
'''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the topic of another issue and PR : the infobox template is missing a field about the license:
https://github.com/searxng/searxng/blob/master/searx/templates/simple/infobox.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be documented imo

searx/engines/jisho.py Outdated Show resolved Hide resolved
@return42
Copy link
Member

return42 commented Apr 1, 2022

Sorry I haven't had time to test in deep yet, but a first try results in a IndexError:

Traceback (most recent call last):
  File "SearXNG/searx/search/processors/online.py", line 143, in search
    search_results = self._search_basic(query, params)
  File "SearXNG/searx/search/processors/online.py", line 131, in _search_basic
    return self.engine.response(response)
  File "SearXNG/searx/engines/jisho.py", line 43, in response
    if page['senses'][0]['parts_of_speech'][0] != 'Wikipedia definition':
IndexError: list index out of range

Should it be

engine_type = 'online_dictionary'

@dalf
Copy link
Member

dalf commented Apr 2, 2022

Should it be engine_type = 'online_dictionary'

yes and no.

  • yes, because it would be nice to en-ja house to pick the right engine.
  • no, because both !jisho house or !jisho 家 and there is no syntax to say "auto detect" to "English" for example (no user query syntax and no implementation).
  • no because wordnik and other dictionnaries don't use the online_dictionnary.
    --> there is a global task about the dictionaries.

@austinhuang0131 > I have refactored the code in dalf@0e2754e . Can you have a look?

@austinhuang0131
Copy link
Contributor Author

That's good, you can use it as-is.

@dalf dalf merged commit 592cea0 into searxng:master Apr 9, 2022
@dalf
Copy link
Member

dalf commented Apr 9, 2022

Merged. Thank you!

@Nocifer
Copy link

Nocifer commented Apr 10, 2022

Wow, that's what I'd call a very prompt response. Thank you all for the effort!

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.

Jisho.org (Japanese <-> English dictionary)
4 participants