Turn SourceFormatterController::formatFiles into a job
ClosedPublic
Actions

Authored by kossebau on Jul 28 2017, 3:25 PM.

Details

Reviewers

Group Reviewers

Commits

R32:b6e663d10b8a: Turn SourceFormatterController::formatFiles into a job
R33:b6e663d10b8a: Turn SourceFormatterController::formatFiles into a job

Summary

Mass processing of data/files is better done in a job
(ideally one day also in own thread).

Test Plan

Works as before when e.g. invoked on toplevel project item,
Stopping the job works as well, happens right after the current
file being processed.

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.

kossebau created this revision.Jul 28 2017, 3:25 PM

Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJul 28 2017, 3:25 PM

Maybe it would make sense to use ExecuteCompositeJob and make every separate file a subjob.

It would help to simplify the async code.

shell/sourceformatterjob.cpp
139	Use `->start()`
146	should be `->start()` and then integrate by using `setError` and `setErrorString`.

This revision now requires changes to proceed.Jul 28 2017, 3:39 PM

In D6966#129780, @apol wrote:

Maybe it would make sense to use ExecuteCompositeJob and make every separate file a subjob.

It would help to simplify the async code.

Yes, there is more room for improvements here. So far SourceFormatterJob::formatFile() is simply the same logic as SourceFormatterController::formatFiles(), with the loop turned into kjob work slices.

Have to admit I am only starting to learn about KJob-style API, so would be happy if the patch could already be applied as it is right, unless there are regressions introduced.
There is surely room for more improvements, but I do not have resources reserved right now to turn the whole logic into perfect code in one go :)

Well then take the comments above as TODO items.

This revision is now accepted and ready to land.Jul 28 2017, 4:21 PM

In D6966#129796, @apol wrote:

Well then take the comments above as TODO items.

Sure, doing. Thanks for review!

Closed by commit R33:b6e663d10b8a: Turn SourceFormatterController::formatFiles into a job (authored by kossebau). · Explain WhyJul 28 2017, 9:09 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		shell/CMakeLists.txt (1 line)
M		shell/sourceformattercontroller.h (4 lines)
M		shell/sourceformattercontroller.cpp (40 lines)
A	M	shell/sourceformatterjob.h (85 lines)
A	M	shell/sourceformatterjob.cpp (153 lines)