Add kio recentlyused:/ to access KActivityStats data
Needs ReviewPublic

Authored by meven on Jun 28 2019, 2:50 PM.

Details

Reviewers
ivan
ngraham
dfaure
Group Reviewers
Frameworks
Summary

Prior to finish D7446, create a better recently used documents feature
See https://phabricator.kde.org/D7446#485917 for motivation.

Test Plan

compile & install
kdeinit5
kioclient5 ls recentlyused:/?limit=100
Or in dolphin

Current filter options :
agent, activity, type(mimetype), url(path fitering)
order and limit

Examples (to use with kioclient5 or see in dolphin) :
recentlyused:/?type=inode/directory
recentlyused:/?type=video/*,audio/*
recentlyused:/?url=/home/meven/kde/src/*&type=text/plain
recentlyused:/?order=HighScoredFirst
recentlyused:/?agent=gwenview

See recentlyused.h for more details.

Diff Detail

Repository
R320 KIO Extras
Branch
arcpatch-D22144
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15156
Build 15174: arc lint + arc unit
meven created this revision.Jun 28 2019, 2:50 PM
Restricted Application added projects: Dolphin, Frameworks. · View Herald TranscriptJun 28 2019, 2:50 PM
Restricted Application added subscribers: kfm-devel, kde-frameworks-devel. · View Herald Transcript
meven requested review of this revision.Jun 28 2019, 2:50 PM
meven updated this revision to Diff 60788.Jun 28 2019, 2:52 PM

Remove default debug option sevirity at debug

meven updated this revision to Diff 60789.Jun 28 2019, 2:53 PM

Improve declared protocol

meven edited the test plan for this revision. (Show Details)Jun 29 2019, 8:46 AM
meven edited the summary of this revision. (Show Details)
elvisangelaccio added inline comments.
recentlyused/recentlyused.protocol
1–14 ↗(On Diff #60789)

We should use the JSON format for new ioslaves.

You can use the slaves in KIO as example.

meven updated this revision to Diff 60886.EditedJun 30 2019, 8:52 PM
meven marked an inline comment as done.

New json metadata file

Thanks @elvisangelaccio didn't know about this metadata format ;)

meven edited the test plan for this revision. (Show Details)Jun 30 2019, 8:55 PM
meven edited the test plan for this revision. (Show Details)Jun 30 2019, 9:06 PM

Nice!

What's the right way to apply this patch? Using the instructions in the text plan section, I'm not able to get it to work:

~/kde/src/kio-extras $  (arcpatch-D22144) kb
Total of 'trivial' dependency cycles detected & eliminated: 2

Building kio-extras from kf5-base-applications (1/1)
        Source update complete for kio-extras: Skipped
        Compiling... succeeded (after 2 seconds)
        Installing.. succeeded (after 0 seconds)

<<<  PACKAGES SUCCESSFULLY BUILT  >>>
kio-extras
 
Removing 1 out of 22 old log directories...
:-)
Your logs are saved in /home/nate/kde/src/log/2019-07-01-01


~/kde/src/kio-extras $  (arcpatch-D22144) kdeinit5
kdeinit5: Shutting down running client.
kdeinit5: Got termination request (PID 28252).
klauncher: Exiting on signal 15
kdeinit5: preparing to launch '/home/nate/kde/usr/lib64/libexec/kf5/klauncher'
kdeinit5: Launched KLauncher, pid = 28914, result = 0
Connecting to deprecated signal QDBusConnectionInterface::serviceOwnerChanged(QString,QString,QString)
kdeinit5: opened connection to :0


~/kde/src/kio-extras $  (arcpatch-D22144) kioclient5 ls recentlyused:/?limit=100
kf5.kio.core: couldn't create slave: "klauncher said: Unknown protocol 'recentlyused'.\n"
"Unable to create io-slave. klauncher said: Unknown protocol 'recentlyused'.\n"
meven added a comment.Jul 8 2019, 1:57 PM

Nice!

What's the right way to apply this patch? Using the instructions in the text plan section, I'm not able to get it to work:

~/kde/src/kio-extras $  (arcpatch-D22144) kb
Total of 'trivial' dependency cycles detected & eliminated: 2

Building kio-extras from kf5-base-applications (1/1)
        Source update complete for kio-extras: Skipped
        Compiling... succeeded (after 2 seconds)
        Installing.. succeeded (after 0 seconds)

<<<  PACKAGES SUCCESSFULLY BUILT  >>>
kio-extras
 
Removing 1 out of 22 old log directories...
:-)
Your logs are saved in /home/nate/kde/src/log/2019-07-01-01


~/kde/src/kio-extras $  (arcpatch-D22144) kdeinit5
kdeinit5: Shutting down running client.
kdeinit5: Got termination request (PID 28252).
klauncher: Exiting on signal 15
kdeinit5: preparing to launch '/home/nate/kde/usr/lib64/libexec/kf5/klauncher'
kdeinit5: Launched KLauncher, pid = 28914, result = 0
Connecting to deprecated signal QDBusConnectionInterface::serviceOwnerChanged(QString,QString,QString)
kdeinit5: opened connection to :0


~/kde/src/kio-extras $  (arcpatch-D22144) kioclient5 ls recentlyused:/?limit=100
kf5.kio.core: couldn't create slave: "klauncher said: Unknown protocol 'recentlyused'.\n"
"Unable to create io-slave. klauncher said: Unknown protocol 'recentlyused'.\n"

I am not a specialist of those steps and ioslave setup/configuration, so bear with me.
A few ideas come to mind.

$ echo $QT_PLUGIN_PATH
Should return something like

/home/nate/kde/usr/lib/x86_64-linux-gnu/plugins:/plugins

If not re-export this variable
export QT_PLUGIN_PATH=/home/nate/kde/usr/lib/x86_64-linux-gnu/plugins:$QT_PLUGIN_PATH
And try again

kioclient5 ls recentlyused:/?limit=100

In case this was not it :

Please try :

KDE_FORK_SLAVE=1 kioclient5 ls recentlyused:/?limit=100

And

/home/nate/kde/usr/bin/kioclient5 ls recentlyused:/?limit=100

Did you try in dolphin to open recentlyused:/?limit=100 ?
This may work.

meven retitled this revision from - Add kio recentlyused:/ to access KactivitytStats data to Add kio recentlyused:/ to access KactivitytStats data.Jul 14 2019, 10:21 AM

I wonder if this is the problem:

ls /home/nate/kde/usr/bin/kioclient5
ls: cannot access '/home/nate/kde/usr/bin/kioclient5': No such file or directory

What actually builds that binary? My KIO doesn't seem to do it.

meven added a comment.Jul 14 2019, 1:48 PM

I wonder if this is the problem:

ls /home/nate/kde/usr/bin/kioclient5
ls: cannot access '/home/nate/kde/usr/bin/kioclient5': No such file or directory

What actually builds that binary? My KIO doesn't seem to do it.

It is in kde-cli-tools, kb kde-cli-tools ;)

Thanks, got it built. However unfortunately it didn't help.

I see that /home/nate/kde/usr/lib64/plugins/kf5/kio/recentlyused.so exists, and /home/nate/kde/usr/lib64/plugins is in my $QT_PLUGIN_PATH variable. And yet...

$ KDE_FORK_SLAVE=1 /home/nate/kde/usr/bin/kioclient5 ls recentlyused:/?limit=100
kf5.kio.core: couldn't create slave: "klauncher said: Unknown protocol 'recentlyused'.\n"
"Unable to create io-slave. klauncher said: Unknown protocol 'recentlyused'.\n"

Even adding /home/nate/kde/usr/lib64/plugins/kf5/kio/ to $QT_PLUGIN_PATH doesn't help. I feel like I must be doing something stupid here.

meven updated this revision to Diff 61860.Jul 16 2019, 2:13 PM

Fix json file

meven added a comment.Jul 16 2019, 2:15 PM

@ngraham I just fixed an issue in the recentlyused.json file, this should work much easier.

I encounter some instabilities with kactivitiymanagerd.

Aha, that was the fix that made it work for me! Thanks a bunch. I will conduct a more thorough review soon.

So interestingly enough, accessing recentlyused:/ in Dolphin's URL navigator works great, but doing recentlyused:/?limit=100 causes it to crash on an assert:

$ dolphin
org.kde.kactivities.lib.core: Setting the title:  ""
ASSERT: "!name.isEmpty()" in file /home/nate/kde/src/kio/src/core/kcoredirlister.cpp, line 1209
Aborted (core dumped)

Here's the full backtrace:

(gdb) bt
#0  0x00007ffff7972755 in raise () from /usr/lib/libc.so.6
#1  0x00007ffff795d851 in abort () from /usr/lib/libc.so.6
#2  0x00007ffff4ef08b6 in QMessageLogger::fatal(char const*, ...) const ()
   from /usr/lib/libQt5Core.so.5
#3  0x00007ffff4eefce2 in qt_assert(char const*, char const*, int) ()
   from /usr/lib/libQt5Core.so.5
#4  0x00007ffff70fff73 in KCoreDirListerCache::slotEntries (
    this=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, job=0x555555d5f300, entries=...)
    at /home/nate/kde/src/kio/src/core/kcoredirlister.cpp:1209
#5  0x00007ffff711ee53 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0, 1>, QtPrivate::List<KIO::Job*, QList<KIO::UDSEntry> const&>, void, void (KCoreDirListerCache::*)(KIO::Job*, QList<KIO::UDSEntry> const&)>::call (f=
    (void (KCoreDirListerCache::*)(KCoreDirListerCache * const, KIO::Job *, const QList<KIO::UDSEntry> &)) 0x7ffff70ff6cc <KCoreDirListerCache::slotEntries(KIO::Job*, QList<KIO::UDSEntry> const&)>, 
    o=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, arg=0x7fffffffcd90) at /usr/include/qt/QtCore/qobjectdefs_impl.h:152
#6  0x00007ffff711d6c7 in QtPrivate::FunctionPointer<void (KCoreDirListerCache::*)(KIO::Job*, QList<KIO::UDSEntry> const&)>::call<QtPrivate::List<KIO::Job*, QList<KIO::UDSEntry> const&>, void> (f=
    (void (KCoreDirListerCache::*)(KCoreDirListerCache * const, KIO::Job *, const QList<KIO::UDSEntry> &)) 0x7ffff70ff6cc <KCoreDirListerCache::slotEntries(KIO::Job*, QList<KIO::UDSEntry> const&)>, 
    o=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, arg=0x7fffffffcd90) at /usr/include/qt/QtCore/qobjectdefs_impl.h:185
#7  0x00007ffff711a752 in QtPrivate::QSlotObject<void (KCoreDirListerCache::*)(KIO::Job*, QList<KIO::UDSEntry> const&), QtPrivate::List<KIO::Job*, QList<KIO::UDSEntry> const&>, void>::impl (which=1, this_=0x555555d67c30, 
    r=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, a=0x7fffffffcd90, ret=0x0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:414
#8  0x00007ffff5124b70 in QMetaObject::activate(QObject*, int, int, void**) ()
   from /usr/lib/libQt5Core.so.5
#9  0x00007ffff70a6670 in KIO::ListJob::entries (this=0x555555d5f300, 
    _t1=0x555555d5f300, _t2=...)
    at /home/nate/kde/build/kio/src/core/KF5KIOCore_autogen/include/moc_listjob.cpp:237
#10 0x00007ffff70a520d in KIO::ListJobPrivate::slotListEntries (this=0x555555b42750, 
    list=...) at /home/nate/kde/src/kio/src/core/listjob.cpp:154
#11 0x00007ffff70a5d6a in KIO::ListJobPrivate::<lambda(const UDSEntryList&)>::operator()(const KIO::UDSEntryList &) const (__closure=0x555555d6de80, list=...)
    at /home/nate/kde/src/kio/src/core/listjob.cpp:288
#12 0x00007ffff70a72ca in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<const QList<KIO::UDSEntry>&>, void, KIO::ListJobPrivate::start(KIO::Slave*)::<lambda(const UDSEntryList&)> >::call(KIO::ListJobPrivate::<lambda(const UDSEntryList&)> &, void **)
    (f=..., arg=0x7fffffffd090) at /usr/include/qt/QtCore/qobjectdefs_impl.h:146
#13 0x00007ffff70a714f in QtPrivate::Functor<KIO::ListJobPrivate::start(KIO::Slave*)::<lambda(const UDSEntryList&)>, 1>::call<QtPrivate::List<QList<KIO::UDSEntry> const&>, void>(KIO::ListJobPrivate::<lambda(const UDSEntryList&)> &, void *, void **) (f=..., 
    arg=0x7fffffffd090) at /usr/include/qt/QtCore/qobjectdefs_impl.h:256
meven added a comment.Jul 17 2019, 9:01 AM

So interestingly enough, accessing recentlyused:/ in Dolphin's URL navigator works great, but doing recentlyused:/?limit=100 causes it to crash on an assert:

$ dolphin
org.kde.kactivities.lib.core: Setting the title:  ""
ASSERT: "!name.isEmpty()" in file /home/nate/kde/src/kio/src/core/kcoredirlister.cpp, line 1209
Aborted (core dumped)

Here's the full backtrace:

(gdb) bt
#0  0x00007ffff7972755 in raise () from /usr/lib/libc.so.6
#1  0x00007ffff795d851 in abort () from /usr/lib/libc.so.6
#2  0x00007ffff4ef08b6 in QMessageLogger::fatal(char const*, ...) const ()
   from /usr/lib/libQt5Core.so.5
#3  0x00007ffff4eefce2 in qt_assert(char const*, char const*, int) ()
   from /usr/lib/libQt5Core.so.5
#4  0x00007ffff70fff73 in KCoreDirListerCache::slotEntries (
    this=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, job=0x555555d5f300, entries=...)
    at /home/nate/kde/src/kio/src/core/kcoredirlister.cpp:1209
#5  0x00007ffff711ee53 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0, 1>, QtPrivate::List<KIO::Job*, QList<KIO::UDSEntry> const&>, void, void (KCoreDirListerCache::*)(KIO::Job*, QList<KIO::UDSEntry> const&)>::call (f=
    (void (KCoreDirListerCache::*)(KCoreDirListerCache * const, KIO::Job *, const QList<KIO::UDSEntry> &)) 0x7ffff70ff6cc <KCoreDirListerCache::slotEntries(KIO::Job*, QList<KIO::UDSEntry> const&)>, 
    o=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, arg=0x7fffffffcd90) at /usr/include/qt/QtCore/qobjectdefs_impl.h:152
#6  0x00007ffff711d6c7 in QtPrivate::FunctionPointer<void (KCoreDirListerCache::*)(KIO::Job*, QList<KIO::UDSEntry> const&)>::call<QtPrivate::List<KIO::Job*, QList<KIO::UDSEntry> const&>, void> (f=
    (void (KCoreDirListerCache::*)(KCoreDirListerCache * const, KIO::Job *, const QList<KIO::UDSEntry> &)) 0x7ffff70ff6cc <KCoreDirListerCache::slotEntries(KIO::Job*, QList<KIO::UDSEntry> const&)>, 
    o=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, arg=0x7fffffffcd90) at /usr/include/qt/QtCore/qobjectdefs_impl.h:185
#7  0x00007ffff711a752 in QtPrivate::QSlotObject<void (KCoreDirListerCache::*)(KIO::Job*, QList<KIO::UDSEntry> const&), QtPrivate::List<KIO::Job*, QList<KIO::UDSEntry> const&>, void>::impl (which=1, this_=0x555555d67c30, 
    r=0x7ffff71c2c80 <(anonymous namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, a=0x7fffffffcd90, ret=0x0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:414
#8  0x00007ffff5124b70 in QMetaObject::activate(QObject*, int, int, void**) ()
   from /usr/lib/libQt5Core.so.5
#9  0x00007ffff70a6670 in KIO::ListJob::entries (this=0x555555d5f300, 
    _t1=0x555555d5f300, _t2=...)
    at /home/nate/kde/build/kio/src/core/KF5KIOCore_autogen/include/moc_listjob.cpp:237
#10 0x00007ffff70a520d in KIO::ListJobPrivate::slotListEntries (this=0x555555b42750, 
    list=...) at /home/nate/kde/src/kio/src/core/listjob.cpp:154
#11 0x00007ffff70a5d6a in KIO::ListJobPrivate::<lambda(const UDSEntryList&)>::operator()(const KIO::UDSEntryList &) const (__closure=0x555555d6de80, list=...)
    at /home/nate/kde/src/kio/src/core/listjob.cpp:288
#12 0x00007ffff70a72ca in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<const QList<KIO::UDSEntry>&>, void, KIO::ListJobPrivate::start(KIO::Slave*)::<lambda(const UDSEntryList&)> >::call(KIO::ListJobPrivate::<lambda(const UDSEntryList&)> &, void **)
    (f=..., arg=0x7fffffffd090) at /usr/include/qt/QtCore/qobjectdefs_impl.h:146
#13 0x00007ffff70a714f in QtPrivate::Functor<KIO::ListJobPrivate::start(KIO::Slave*)::<lambda(const UDSEntryList&)>, 1>::call<QtPrivate::List<QList<KIO::UDSEntry> const&>, void>(KIO::ListJobPrivate::<lambda(const UDSEntryList&)> &, void *, void **) (f=..., 
    arg=0x7fffffffd090) at /usr/include/qt/QtCore/qobjectdefs_impl.h:256

Seeing the line mentioned and the code underneath it may due to some error in your kactitivity database where an entry has an empty name or path.

You can remove your activity database with
rm ~/.local/share/kactivitymanagerd/resources/database
And restart dolphin

Or try to find the error with like :
sqlitebrowser ~/.local/share/kactivitymanagerd/resources/database

Indeed, it was a database issue. Looks like that's on kactivitymanagerd, not you!

It's probably too late to get this into 19.08 unfortunately. In my experience big new features like this shouldn't be merged right before the deadline.

In addition to the inline comments I've left, I'd like to request less usage of auto. I find it hard to understand what types are being used in recentlyused.cpp where auto has been used. And in some cases it's just unnecessary; for example auto limitInt could just be int limit instead, which would be clearer.

recentlyused/recentlyused.cpp
29

Remove

144

Missing a space before else if

212

space after for

recentlyused/recentlyused.h
31

KAcitivitiesStats -> KActivitiesStats

50

resourcess -> resources
cantain -> contain

88

Don't commit commented-out code. If this really needs to be commented out, it needs a comment explaining why.

meven updated this revision to Diff 61929.Jul 17 2019, 5:51 PM

Fix typos, missing space...

meven marked 6 inline comments as done.Jul 17 2019, 5:52 PM
ngraham accepted this revision.Jul 17 2019, 5:56 PM

LGTM!

This revision is now accepted and ready to land.Jul 17 2019, 5:56 PM
meven updated this revision to Diff 62298.Jul 22 2019, 1:54 PM

Disallow getting mimetype or stating non root path

meven updated this revision to Diff 62417.Tue, Jul 23, 4:08 PM

Remove the current option to filter on curent application as it does not work

ngraham retitled this revision from Add kio recentlyused:/ to access KactivitytStats data to Add kio recentlyused:/ to access KActivityStats data.Tue, Jul 23, 4:17 PM
ivan requested changes to this revision.Wed, Jul 24, 9:55 AM
ivan added inline comments.
recentlyused/recentlyused.cpp
141–145

Could this just be replaced with query = query | Agent(agentValue.split(QLatin1Char(',')));?

Obviously, the downside is always creating a list (usually with a single item in it), the upside is that it does not need to traverse the string twice (once to chech for commas, and the second time to split the string.

The trade-off depends on internals of Qt - if you don't have the time to benchmark what is faster, leave it as is.

251

QStringLiteral?

This revision now requires changes to proceed.Wed, Jul 24, 9:55 AM
meven updated this revision to Diff 62464.Wed, Jul 24, 11:10 AM

Use QStringLiteral only check for commas in string once to check for multiple values parameters, typo

meven marked 2 inline comments as done.Wed, Jul 24, 11:12 AM
meven added inline comments.
recentlyused/recentlyused.cpp
141–145

I have used count() as it allows not to keep the list in memory if not needed.
Same was done for the type parameter.

meven marked 2 inline comments as done.Wed, Jul 24, 11:12 AM
meven updated this revision to Diff 63829.Thu, Aug 15, 5:50 PM

run uncrustify-kf5

ivan added a comment.Mon, Aug 19, 6:26 PM

Looks OK to me, I guess some of our resident KIO experts should review it. What do you think?

meven added a comment.Tue, Aug 20, 6:00 AM
In D22144#514633, @ivan wrote:

Looks OK to me, I guess some of our resident KIO experts should review it. What do you think?

Definitely. Do you have an idea who ?

BTW, I have one change planned for this waiting for D22775.

I'v added @dfaure as reviewer.

David feel free to not review this if you think somebody else would be better suited.