Change frameRadius to use pen widths, add frameRadiusForNewPenWidth, add PenWidth::NoPen
ClosedPublic

Authored by ndavis on Dec 26 2019, 5:07 AM.

Details

Summary

This patch reduces the amount of duplicate code and makes every part of the helper functions use the correct frame radius.

Diff Detail

Repository
R31 Breeze
Branch
replace-hardcoded-2 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20272
Build 20290: arc lint + arc unit
ndavis created this revision.Dec 26 2019, 5:07 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 26 2019, 5:07 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ndavis requested review of this revision.Dec 26 2019, 5:07 AM
ndavis updated this revision to Diff 72185.Dec 26 2019, 5:14 AM

Add default value to frameRadius penWidth

kstyle/breezehelper.h
318

Would need a new function name than "newFrameRadius", that makes it clear when this should be used on not the other.

ndavis updated this revision to Diff 72277.Dec 28 2019, 7:46 AM
  • Change newFrameRadius to newPenWidthFrameRadius

This name doesn't sound very good, but at least it's more descriptive.

ndavis retitled this revision from Add newFrameRadius, change frameRadius to use pen widths, add PenWidth::NoPen to Change frameRadius to use pen widths, add newPenWidthFrameRadius, add PenWidth::NoPen.Dec 28 2019, 7:47 AM
ndavis updated this revision to Diff 72280.Dec 28 2019, 7:57 AM

Change comment formatting

ndavis updated this revision to Diff 72281.Dec 28 2019, 8:01 AM
  • Change newPenWidthFrameRadius to frameRadiusNewPenWidth

This sounds slightly better to me and allows it to show up next to frameRadius when autocompleting.

ndavis marked an inline comment as done.Dec 29 2019, 11:48 PM
hpereiradacosta accepted this revision.Dec 30 2019, 12:12 AM
  • Change newPenWidthFrameRadius to frameRadiusNewPenWidth

    This sounds slightly better to me and allows it to show up next to frameRadius when autocompleting.

What about "frameRadiusForPenWidth" ?
I think the "New" is misleading.
If you agree, please change and then commit. The rest sounds good.

This revision is now accepted and ready to land.Dec 30 2019, 12:12 AM
ndavis updated this revision to Diff 72355.EditedDec 30 2019, 12:51 AM
  • use frameRadiusForNewPenWidth

I think the use of the word new is important here because it's for taking an existing radius and modifying it based on a new pen width. frameRadiusForPenWidth sounds like it works the same as frameRadius.

ndavis retitled this revision from Change frameRadius to use pen widths, add newPenWidthFrameRadius, add PenWidth::NoPen to Change frameRadius to use pen widths, add frameRadiusForNewPenWidth, add PenWidth::NoPen.Dec 30 2019, 12:54 AM
This revision was automatically updated to reflect the committed changes.