Introduce ProcessDataModel
ClosedPublic

Authored by davidedmundson on Feb 19 2020, 5:30 PM.

Details

Summary

Last release a plugin system was introduced that allowed plugins to
provide columns of process data in a way that included enough metadata
to allow displaying of said data appropriately without the client
needing to be aware of the semantics of what that column represents.

This patch provides all process information in that new format. This is
then exposed as new, much simler, model.

This new model is designed to be consumable from a QML API for any
potential process data viewer.

Existing models are unchanged for maximum compatibility.

Test Plan

Used in another project

Diff Detail

Repository
R111 KSysguard Library
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23981
Build 23999: arc lint + arc unit
davidedmundson created this revision.Feb 19 2020, 5:30 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 19 2020, 5:30 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson requested review of this revision.Feb 19 2020, 5:30 PM
broulik added inline comments.
processcore/extended_process_list.cpp
94

Isn't the typename auto-deduced/implied? Doesn't hurt though.

100

Capture only [this]

110

Does this need caching?

123

Add a comment that this magic number is nobody

355

Do any of the sensor names need / used to have i18n context?

398

This could have used a reserve call :)

processcore/process_data_model.cpp
2

Missing copyright header

57

Does ProcessDataModelPrivate really need to be a QObject? Can't you connect it to q with a lambda?

64

Add reserve()

146
if (d->enabledAttributes == enabledAttributes) {
     return;
}

?

184

Coding style

191

Remove equals, or better turn it around to match everone else around it:

if (row >= d->m_processes->processCount()) {
    return QModelIndex();
}
255

There is:

QMetaEnum::fromType<AdditionalRoles>();
279

Superfluous; or have it return name() instead

processcore/process_data_model.h
34

Remove second "contains"

49

would it hurt to make this a QAbstractItemModel *?

54

should this just be Qt::DisplayRole given you return for both in data?

67

explicit

105

Why is this not a nested Private class like in the extended processes list?

davidedmundson added inline comments.Feb 28 2020, 4:23 PM
processcore/process_data_model.h
54

It breaks our roleNames as you can't have two names for the same value.

Then on the QML side we can't interchange with display and formattedValue. which is what we were trying to achieve

ahiemstra added inline comments.
processcore/process_data_model.cpp
279

Oops. Both this one and the one in data() should return name() if shortName() is empty.

davidedmundson marked 15 inline comments as done.

some but not all review comments

davidedmundson added inline comments.Feb 28 2020, 5:30 PM
processcore/extended_process_list.cpp
398

not a meaningful one, each provider can add provide many attributes

davidedmundson marked 3 inline comments as done.

lambdas everywhere

broulik accepted this revision.Mar 20 2020, 10:53 AM
This revision is now accepted and ready to land.Mar 20 2020, 10:53 AM
broulik accepted this revision.Mar 20 2020, 1:04 PM
This revision was automatically updated to reflect the committed changes.