Feature/extended search #36

iain-benson · 2019-01-25T16:24:06Z

Extend the search facility to allow Boolean logic, and a lot more fields to be searched.

selmf · 2019-01-25T21:23:18Z

thanks for the PR. As you might have noticed, development is a bit slow these days, so it could take a while before @luisangelsm can check the code.

There's also some stuff in the code that will likely require some discussion, such as switching to C++17 and adding an embedded library to the codebase.

luisangelsm · 2019-02-18T16:37:29Z

First of all, thank you very much for the PR, it is really appreciated. This is something that has been in the TODO list for so long that it is great to finally see something done about it.

I didn't have time to fully review this yet, but I have the same concerns expresed by @selmf. @iain-benson could you post here an example of a search string? I would like to think about the posiblity of write a small class to take care of the lexical parsing, it shouldn't be hard and it will be easier to mantain in the long term.

Another thing that comes to my mind is that the fields used in the query parser should be defined in a single place only, so maybe it is time to do something in the database source code so these fields can be imported directly into the query parser, so further changes/updates in the database won't requiere updating the query parser fields manualy.

luisangelsm · 2019-09-24T09:43:23Z

A couple of things need to be done before merging this:

Move the queries execution to a background thread. Now that the query is more complex, execution time can be high in large comic libraries, blocking the main thread leads to a bad user experience. Probably an execution queue is in order so we can cancel operations while the user types. This is something I wanted to do even before this PR.
Write a lexer and remove the lexertl dependency, the number of tokens is so small and simple that I think is hard to justify adding a dependency to the project.

I will take care of this at some point, if anyone is interested please ping me.

iain-benson · 2019-10-24T20:45:36Z

Apologies for having somewhat abandoned this, I just haven't had the time to revisit it, other than to fix an issue I found with string streams after moving to a newer version of Xcode.

I understand why you want to get rid of lexertl, it probably is overkill, it was just quicker, and easier for me to find a parser than try to write my own.

Fixing the input lag on queries, either by threading, or possibly a simple hold-off whilst still typing, was something I had realised needed doing, but never quite got around to, I was also planning to implement query operators (as with the syntax at https://manual.calibre-ebook.com/gui.html#the-search-interface) rather than everything being LIKE or exact matches, but as with most things, life and work have a tendency to get in the way.

luisangelsm · 2019-10-25T13:13:26Z

@iain-benson no worries :) Even if you can work any more on it, it is a good starting point. An user expresed interest in continue the work, but I didn't hear anything else from him, so I am not sure about the current status.

luisangelsm · 2021-01-08T15:11:37Z

There have been some changes to the DB code and this PR was conflicting with the current implementation. I rebased it and fixed the conflicts, hopefully I didn't break anything.

I'll try to finish otherwise it won't survive as conflicts will get harder to fix :/ and it is a feature that has been requested a lot.

This was broken while doing a rebase

QueryLexeter does not parse "atWord" because I couldn't find what it is used for.

iain-benson · 2021-01-12T18:22:09Z

Thank you for finishing this off. I knew there were issues with it stopping it being accepted, I just haven’t had the opportunity to look at it.

luisangelsm · 2021-01-12T19:33:35Z

@iain-benson thank you for starting it :), it needs some more work though, requesting data from the DB should be done in a background thread. I am working on it now and hopefully I'll be able to merge it in a few a days.

Now that I have your attention, do you remember what atWord tokens are for?

iain-benson · 2021-01-12T23:02:09Z

@iain-benson thank you for starting it :), it needs some more work though, requesting data from the DB should be done in a background thread. I am working on it now and hopefully I'll be able to merge it in a few a days.

Now that I have your attention, do you remember what atWord tokens are for?

I don’t think it’s needed. I think it’s part of a feature used in Calibre’s python code, related to custom database fields, that I mistakenly put in despite not being used.

`atWord` wasn't used at all and spaces should be eaten by the lexer And added `unspecified` token

List initialization ended using movable constructors which surprisingly caused data troubles in release mode, at least in VC2019 compiler. The tree being messed up caused crashes while SQL was generated. I have no explanation for it.

Just to keep things consistent in the whole round trip conversion

… data

It uses the same approach used by the comics search filter

It is probably worth to bump this to c++14 and ideally use c++17, but it will be done in a separate branch.

The lexer itself should be responsible for parsing those values but it will require a bigger refactoring.

luisangelsm

It could use some more work but it seems to work good enough now.

vedgy · 2021-02-02T13:29:25Z

YACReaderLibrary/db/comic_model.cpp


-        selectQuery.exec();
+    QLOG_ERROR() << "-d2>" << data->size();


Was this line left here by mistake? It spams error messages every time the search edit's text changes to a non-empty value.

Yes, my bad.

Should this line be removed or converted to QLOG_TRACE or QLOG_DEBUG?

Removed, I used that log to catch some issues while I was working on this PR but it doesn't have a purpose anymore.

Removed in #194.

luisangelsm force-pushed the feature/extended_search branch from fadd855 to 192cf89 Compare September 23, 2019 16:03

selmf mentioned this pull request Jan 12, 2020

A small list of features I would like, with request of advice for what would be easiest to tackle for a newcomer #109

Open

iain-benson and others added 10 commits January 8, 2021 15:41

Add commit 43aab01 of BenHanson/lexertl14 from github

d3de52c

Add query parser class

4a50d43

Add folder into the query parser

5fa7da1

Use the query parser in the comic and folder model

673ee1f

Some small tidy ups

30529dc

Add some documentation and attribution to the query parser

9a66035

Apply clang-format

7ccb338

Fix scopes

2d3888b

Remove c++17 dependency

4990093

Use concatenation, rather than ostringstream

8efb991

luisangelsm force-pushed the feature/extended_search branch from 4e5526f to 8efb991 Compare January 8, 2021 15:08

luisangelsm added 5 commits January 8, 2021 16:59

Fix binding values to search query

255e51d

This was broken while doing a rebase

Move query execution and model setup to the right scope

6438c92

Fix data base removal in FolderModel

5037f3a

Replace lexertl with a custom lexeter implementation

a777aa3

QueryLexeter does not parse "atWord" because I couldn't find what it is used for.

Remove misplaced code

ddb140d

luisangelsm added 3 commits January 14, 2021 08:39

Remove space and atWord tokens

f09c595

`atWord` wasn't used at all and spaces should be eaten by the lexer And added `unspecified` token

Use an explicit constructor for TreeNode

260f538

List initialization ended using movable constructors which surprisingly caused data troubles in release mode, at least in VC2019 compiler. The tree being messed up caused crashes while SQL was generated. I have no explanation for it.

Use QString::fromStdString

7b36100

Just to keep things consistent in the whole round trip conversion

luisangelsm added 13 commits January 14, 2021 09:03

Add concurrent queue based on lambdas

fa5ce25

Add a class for processing search queries and create the comics model…

dbdc7bd

… data

Use ComicQueryResultProcesor

ccc382d

Make the search line edit bigger

047fd24

Run folder search filtering in the background

5343d24

It uses the same approach used by the comics search filter

Remove unused constant

c3b0780

YACReaderLibrary compiles using c++11 just fine

dde60b7

It is probably worth to bump this to c++14 and ideally use c++17, but it will be done in a separate branch.

Add support for boolean folder fields

f803b54

Add support for true and false literals to be used with bool fields

f03ad84

The lexer itself should be responsible for parsing those values but it will require a bigger refactoring.

Add fallback for qt < 5.15

82eb5c0

Remove reference to lexertl's license

24c0a78

Import non-gui global header

6bb64c5

Remove comments

76a307d

luisangelsm self-requested a review January 14, 2021 20:35

luisangelsm approved these changes Jan 14, 2021

View reviewed changes

Update CHANGELOG

7e72c8b

luisangelsm merged commit 8c6e8cd into YACReader:develop Jan 14, 2021

vedgy reviewed Feb 2, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/extended search #36

Feature/extended search #36

iain-benson commented Jan 25, 2019

selmf commented Jan 25, 2019

luisangelsm commented Feb 18, 2019

luisangelsm commented Sep 24, 2019 •

edited

iain-benson commented Oct 24, 2019

luisangelsm commented Oct 25, 2019

luisangelsm commented Jan 8, 2021

iain-benson commented Jan 12, 2021

luisangelsm commented Jan 12, 2021

iain-benson commented Jan 12, 2021

luisangelsm left a comment

vedgy Feb 2, 2021

luisangelsm Feb 2, 2021

vedgy Feb 4, 2021

luisangelsm Feb 4, 2021

vedgy Feb 4, 2021

Feature/extended search #36

Feature/extended search #36

Conversation

iain-benson commented Jan 25, 2019

selmf commented Jan 25, 2019

luisangelsm commented Feb 18, 2019

luisangelsm commented Sep 24, 2019 • edited

iain-benson commented Oct 24, 2019

luisangelsm commented Oct 25, 2019

luisangelsm commented Jan 8, 2021

iain-benson commented Jan 12, 2021

luisangelsm commented Jan 12, 2021

iain-benson commented Jan 12, 2021

luisangelsm left a comment

Choose a reason for hiding this comment

vedgy Feb 2, 2021

Choose a reason for hiding this comment

luisangelsm Feb 2, 2021

Choose a reason for hiding this comment

vedgy Feb 4, 2021

Choose a reason for hiding this comment

luisangelsm Feb 4, 2021

Choose a reason for hiding this comment

vedgy Feb 4, 2021

Choose a reason for hiding this comment

luisangelsm commented Sep 24, 2019 •

edited