take matching logic out of match method and put it into a class
ClosedPublic

Authored by sitter on Dec 15 2016, 2:34 PM.

Details

Summary

refactor and qcdebug

::match was a super long spagetthi with multiple sub-matches.
This was supposedly primarily because it needed to be stateful across all sub-matches. To deal with this there's a new class which is taking care of the stateful service finding, this class is comprised of multiple matching functions that contribute to the full match set.
Good enough readability for now. Classes for this would be nicer though.

Also add qcdebug for this runner so we can get a better idea of why results are the way they are. Defaults to warning so it's silent by default.

Test Plan

Careful refactoring so hopefully no problems.

  • still matches binary names as best match
  • still matches comments
  • still matches generic names
  • still matches names

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter updated this revision to Diff 9039.Dec 15 2016, 2:34 PM
sitter retitled this revision from to take matching logic out of match method and put it into a class.
sitter updated this object.
sitter edited the test plan for this revision. (Show Details)
sitter added a reviewer: broulik.
Restricted Application added a project: Plasma. · View Herald TranscriptDec 15 2016, 2:34 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik accepted this revision.Dec 15 2016, 3:50 PM
broulik edited edge metadata.

Thanks

runners/services/servicerunner.cpp
124

isnt NotShowIn a list property? Doesn't KService have a getter for that or even take that into account in noDisplay()?

This revision is now accepted and ready to land.Dec 15 2016, 3:50 PM
sitter added inline comments.Dec 15 2016, 3:59 PM
runners/services/servicerunner.cpp
124

something to consider in a follow up, for now I'd like to get the refactor landed.

e.g. if (!service->isApplication()) { further down also seems weird considering we already queried for applications specifically.

This revision was automatically updated to reflect the committed changes.