always support all command line options
ClosedPublic

Authored by sitter on Mar 21 2019, 12:24 PM.

Details

Summary

with the current way options are handled we'd always have a tight lock
between kcrash and drkonqi making it unnecessarily difficult to add
new options because newer kcrashes cannot simply pass them regardless of
drkonqi's version.

e.g.

  • drkonqi 5.20 adds --displayName
  • kcrash 5.70 adds support for passing --displayName
  • kcrash now needs to somehow determine the precise version of drkonqi because passing --displayName to 5.19 or below results in exit1

by having drkonqi ignore unknown options we can eliminate this restriction
as kcrash can then simply always pass all arguments it knows (which it
needs to do for the entire so-series; 5.70 needs to still work with
plasma 5.0 in theory)

NB: 5.x still needs some variant of version checking as kcrash needs to
find out if drkonqi has this new behavior. come 6.x we would be able to
ignore versions entirely in kcrash

Test Plan

passing arbitrary cli options no longer causes drkonqi to exit in error

Diff Detail

Repository
R871 DrKonqi
Branch
arbitrary-options
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9936
Build 9954: arc lint + arc unit
sitter created this revision.Mar 21 2019, 12:24 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 21 2019, 12:24 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter requested review of this revision.Mar 21 2019, 12:24 PM

I should add for the record that I'd actually find stricter versioning better, but arg handling in kcrash is already a bit tricky because of signal-safety restrictions and adding versioning complexity on top would probably do nothing for how easy the code base is to approach. On top of that, even with minimal options (correctly) set, drkonqi will and should be able to help the user file a bug report. In theory we need as little as the pid to do something of use.

apol accepted this revision.Mar 22 2019, 12:43 AM
apol added a subscriber: apol.

Sounds good to me.

This revision is now accepted and ready to land.Mar 22 2019, 12:43 AM
This revision was automatically updated to reflect the committed changes.