Support human-readable time suffixes in time-based search query filters

[RJ Skerry-Ryan]

E.g. "duration:<3m".

[k bashar (k.bashar)]

i am trying to fix(implement) this.
i have found three classes having "search" in name.
wsearchlineedit
searchquery
searchqueryparser

i see the emit() But i have no clue where these signals of wsearchlineedit class is called i mean where is the slot of them implemented.
 void search(const QString& text);
 void searchCleared();
 void searchStarting();

i need to know at least an aproch you would take if you are in this same situation(well you won't be if you aren't a new commer like me :) ) to find them.

[Daniel Schürmann (daschuer)]

I think you are talking about:

void WSearchLineEdit::triggerSearch()
{
    m_searchTimer.stop();
    emit(search(text()));
}

emit() is just a macro the is replaced by nothing. But this is a nice way to mark this function as a Qt signal.

Which IDE are You using? Eclipse has a nice feature "Display Call tee" which is very helpful to investigate the source.
Unfortunately it can't follow Qt's signal slot connection.

But a seach for "SIGNAL(search(" helps.
I have just found:
QWidget* LegacySkinParser::parseSearchBox(QDomElement node) {
..
    connect(pLineEditSearch, SIGNAL(search(const QString&)),
            m_pLibrary, SIGNAL(search(const QString&)));
..
Please note, this is a signal to signal connected.

This is connected here:

  connect(m_pLibrary, SIGNAL(search(const QString&)),
            pLibraryWidget, SLOT(search(const QString&)));

to void WLibrary::search(const QString& name) {

Now the magic call tree function comes in:
click click click ...

I think:

void SearchQueryParser::parseTokens(QStringList tokens,
                                    QStringList searchColumns,
                                    AndNode* pQuery) const {
is your friend

Hope that helps

[k bashar (k.bashar)]

as i see in mixxx we only support argument without any whitespace.
duration: >2m-3m 20s won't work
duration: >2m-3m20s will work (difference is 'space' between '3m' and '20s')

and this constarint is needed for any Numerical search. But when we are talking about human readable duration search it is a problem. To me "3m 20s" is more natural than "3m20s". Want to hear from others in this regard.

Anyway i have made patch for humanreadable duraton search. which now workes without space using.
h/H - is used as hour indication
m/M - is used as Minute indication
s/S - is used as second indication
use cases:
2m-3m2s
2m-200s
2m-200 (one is humanreadable other one is like before)
2m
2m20s
120-200

any use case i should consider?

i have attached the patch here and i am making a pull request in git master.

[Daniel Schürmann (daschuer)]

thank you for the patch!

It is not required to post it here, if you make a pull request as well.
For reference, it is a good practice to post here the link to the pull request.
It Is: https://github.com/mixxxdj/mixxx/pull/185
Once you are working at a bug, you should change the status to "In Progress" (I do it now )

[Daniel Schürmann (daschuer)]

Your solution is fine to me.

IMHO 2m20s is as unnatural as as 2m 20s so we should keep 2m20s

For me, this colon format is most natural.
2:30 s

Can you add just this:
2:30
since the on above has the space separator problem.

I think then we are almost complete together with your already implemented versions.

[k bashar (k.bashar)]

I will keep in mind about the good practice now on. Thanks for correcting.

yeah 2:30 s is more natural than existing options.
as i was following bug description so "2m 20s"/"2m20s" came into mind while coding but now i'm going to implement this(2:30).
should i keep existing solution or discard it?

[Daniel Schürmann (daschuer)]

please keep 2m20s if it does not hurt you, it fits perfectly to the schema.

[k bashar (k.bashar)]

i have updated the code to support 2:30 style.
here is pull request link.
https://github.com/mixxxdj/mixxx/pull/186

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7233