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

search gives "No results" for a certain word #41

Closed
omatsuda opened this issue Jun 2, 2021 · 8 comments
Closed

search gives "No results" for a certain word #41

omatsuda opened this issue Jun 2, 2021 · 8 comments
Labels
Milestone

Comments

@omatsuda
Copy link

omatsuda commented Jun 2, 2021

Thank you very much for this nice software.

Everything works just fine, except one remaining strange behavior. When I search the word "avoid", qolibri just returns 'No rusults for "avoid".' It happens for either "Exact word search" or "Forward search". If I search "avoi", it gives the results for avoid, avoidable, .. etc.

Backword search for "avoid" gives the correct results.

Could anybody help me to solve this?

I'm using several dictionaries including Longman 4th Edition, and some English-Japanese dictionalies.

The program itself has been compiled on FreeBSD 12.2-Release.
(For the build, I had to add a line
link_directories(/usr/lib /usr/local/lib)
just below the line
include(GNUInstallDirs)
in CMakeLists.txt. Otherwise the build fails at the linker stage to find libeb.so which is installed at /usr/local/lib in FreeBSD.)

@darlopvil
Copy link

Same here. I'm using Windows 10 and literally, you can't search any word.

@mvf
Copy link
Owner

mvf commented Jun 13, 2021

Confirmed, thanks for reporting this! That's indeed a pretty nasty bug. It's strange that it works on 1.x, things shouldn't have changed much in and around the EB library.

Also, thanks for reporting the linker path issue. This is probably because we (sloppily) just link_libraries the eb lib without doing find_library first. This works as long as eb is installed in /usr (as is typical on Linux), but it's technically incorrect (everywhere) because it bypasses CMAKE_SYSTEM_PREFIX_PATH.

@mvf mvf added the bug label Jun 13, 2021
@mvf
Copy link
Owner

mvf commented Jun 13, 2021

Quick workaround in src/searchpage.cpp:

RET_SEARCH SearchPage::search(const Query& query)
     ...
     switch (query.method.direction) {
     case ExactWordSearch:
     case ForwardSearch:
-        queries << stemWords(query.query);
         break;
     default:
         break;

Turns out that since 2010 with change f01a37c, qolibri tries to deinflect English words in Exact and Forward searches, e.g. "avoids" will do an implicit second query for "avoid". This bug presents only when the last one of these pseudo-deinflections is not found. For "avoid" that's "avoy", which doesn't exist → "No results". For "vices" it's "vex", so it works, even though the words are unrelated. "house" doesn't pseudo-deinflect, so it's unaffected.

The bug is that when looping over the queries for the individual words, the outcome of the last search determines the overall outcome:

RET_SEARCH SearchPage::doSearch(const Query& query, PageItems &items, int &itemIndex)
{
    ...
    foreach (const QString &q, queries) {
        retStatus = doSearch(Query(q, query.method), items, itemIndex);
    }

This worked at the time because in 2009 15dd5b3 had changed what is now SearchPage::doSearch to succeed, even when there were no hits. This changed in 2012 with 6eb4787, but the regression went unnoticed until now, so thanks again for reporting!

@mvf mvf added this to the 2.1.4 milestone Jun 13, 2021
@omatsuda
Copy link
Author

Thank you very much!
The suggested workaround solved the issue perfectly.

@darlopvil
Copy link

Hi, I would like to know what a Windows 10 user could do to with the workaround. It seems I cannot use the program until a new version gets released.

@omatsuda
Copy link
Author

omatsuda commented Jun 27, 2021

In FreeBSD, we have "ports collection" which offers a simple way to compile and install third party applications.
I have prepared a port for qolibri and now it is open to the public.
I included the workaround patch suggested by Matthias in this port. But now I'm worrying that I should have got a permission to include this before the port is open. I'm sorry about this, and if you don't like this, please tell me.

For CMakeList.txt, I was suggested by the FreeBSD team the following patch to make it more generic.

--- CMakeLists.txt.orig 2020-02-28 16:02:25 UTC
+++ CMakeLists.txt
@@ -16,6 +16,8 @@ set(CMAKE_AUTOUIC ON)
 set(CMAKE_AUTORCC ON)

 find_package(Qt5 COMPONENTS LinguistTools Multimedia Network WebEngine WebEngineWidgets Widgets REQUIRED)
+find_library(EB_LIBRARY eb)
+find_library(Z_LIBRARY z)

 add_executable(qolibri MACOSX_BUNDLE WIN32
     images/qolibri.icns
@@ -154,6 +156,6 @@ set_source_files_properties(${TS_FILES} PROPERTIES OUT
 qt5_add_translation(QM_FILES ${TS_FILES})
 target_sources(qolibri PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/translations/translations.qrc" ${QM_FILES} ${TS_FILES})

-target_link_libraries(qolibri Qt5::Multimedia Qt5::Network Qt5::WebEngine Qt5::WebEngineWidgets Qt5::Widgets eb z)
+target_link_libraries(qolibri Qt5::Multimedia Qt5::Network Qt5::WebEngine Qt5::WebEngineWidgets Qt5::Widgets ${EB_LIBRARY} ${Z_LIBRARY})

 install(TARGETS qolibri DESTINATION "${CMAKE_INSTALL_BINDIR}")

@mvf
Copy link
Owner

mvf commented Jun 28, 2021

@omatsuda Thanks so much for bringing qolibri to FreeBSD! Of course, shipping the workaround to your users is more than fine. It's me who should apologize for the lazy CMakeLists. 😉 If you don't mind, I'll push the patch you suggested.

@darlopvil Yes, you'd have to compile yourself. I think we're about ready to release 2.1.4 though.

@omatsuda
Copy link
Author

Thank you! And I would be happy if the patch for CMakeLists.txt is adopted in the next release.

mvf added a commit that referenced this issue Jul 4, 2021
Details: #41 (comment)

Since 6eb4787, this would report "No
results" for many alphabetic terms in Exact and Forward searches. While
this aspect could have been fixed, the deinflection could also dig up
unrelated words and was slowing down searches with additional queries.

A Forward Search for the stem accomplishes everything this feature was
trying to do, minus the surprise. So let's just drop it.

This reverts commit f01a37c
@mvf mvf closed this as completed in 9b7fb60 Jul 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants