Introduce IFilterStrategy::progressInLine [1/3]
ClosedPublic

Authored by kfunk on Jan 8 2016, 1:34 AM.

Details

Summary

Makes it possible for filtering strategies to parse a line and report
progress if the line contains progress information.

Example make output:

[  5%] Building foo
[  6%] Building bar

A special make job filtering strategy can then parse the output and
report progress. The core idea is that this parsing is done in a
separate thread, like all the output filters do.

The progress information is also passed on to OutputExecuteJob which
emits the progress via KJob API.

Filter strategy implementations for Make & Ninja in kdevelop will follow.

Revert "Mark IFilterStrategy implementations final." [2/3]

Required for upcoming feature

This reverts commit cf8a46a637f87f78cc42518f3cc8e04155650caf.

OutputModel: Allow to set IFilterStrategy pointer [3/3]

This is crucial to allow custom filtering strategies to be set on the
output model.

Let's keep the old API (which selects the filter strategy based on enum)
intact for now.

Also install outputfilteringstrategies.h, to be able to subclass from
the standard output filtering strategies.

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kfunk updated this revision to Diff 1780.Jan 8 2016, 1:34 AM
kfunk retitled this revision from to Introduce IFilterStrategy::progressInLine [1/3].
kfunk updated this object.
kfunk edited the test plan for this revision. (Show Details)
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJan 8 2016, 1:34 AM
apol requested changes to this revision.Jan 8 2016, 2:35 AM
apol added a reviewer: apol.
apol added a subscriber: apol.

Looks really good! Kudos! :D

How about adding a test?

This revision now requires changes to proceed.Jan 8 2016, 2:35 AM
mwolff added a subscriber: mwolff.Jan 8 2016, 4:07 PM
mwolff added inline comments.
outputview/ifilterstrategy.h
46 ↗(On Diff #1780)

struct

49 ↗(On Diff #1780)

float percent?

also save a few chars via

const QString& status = {}
80 ↗(On Diff #1780)

how do you differentiate between invalid and valid progress? 0% can be valid, no?

outputview/outputexecutejob.cpp
43 ↗(On Diff #1780)

cref

409 ↗(On Diff #1780)

scoped ptr + reset?

outputview/outputfilteringstrategies.h
46

now ots public api you must use the real export macro

76

please do

outputview/outputmodel.cpp
104 ↗(On Diff #1780)

cref

outputview/outputmodel.h
81 ↗(On Diff #1780)

cref

kfunk updated this revision to Diff 1799.Jan 8 2016, 10:14 PM
kfunk edited edge metadata.

Addressed most concerns

kfunk added a comment.Jan 8 2016, 10:16 PM

Please check.

No unit test yet, it's quite pointless to add one in test_filterstrategies for progressInLine... (would just test a function return a struct)

mwolff requested changes to this revision.Jan 8 2016, 10:59 PM
mwolff added a reviewer: mwolff.

a few minor things, then it lgtm

outputview/ifilterstrategy.cpp
38 ↗(On Diff #1799)

with the above, you can still return {} here then.

outputview/ifilterstrategy.h
49 ↗(On Diff #1799)

int percent = -1;

i.e. default construct it right here.

outputview/outputfilteringstrategies.cpp
33

keep this template here and use it in this file. that way, we can still have the simple data here in RO memory.

for the outside, feel free to offer a public QVector match function or similar (too bad we don't have array_view yet), and then call this template internally.

256

revert this, see above

This revision now requires changes to proceed.Jan 8 2016, 10:59 PM
This revision was automatically updated to reflect the committed changes.