Add standard pen widths and replace hardcoded numbers
ClosedPublic

Authored by ndavis on Dec 25 2019, 7:06 AM.

Details

Summary

The replaced numbers are directly related to the width of the pen.
This patch likely doesn't replace all of the numbers that could be replaced.
It just replaces the ones I'm currently certain of being related to pen width.
The goal is to make the code show the intent of the designer.

Diff Detail

Repository
R31 Breeze
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ndavis created this revision.Dec 25 2019, 7:06 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 25 2019, 7:06 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ndavis requested review of this revision.Dec 25 2019, 7:06 AM
ndavis edited the summary of this revision. (Show Details)Dec 25 2019, 7:07 AM

+1, seems sensible to me, but probably wait for @hpereiradacosta's go-ahead.

kstyle/breeze.h
179

Why three slashes for these comments?

ndavis added inline comments.Dec 26 2019, 7:22 PM
kstyle/breeze.h
179

It's a Doxygen style. If you do it, KDevelop shows the comments in the tooltips for the commented functions, classes and variables.

ngraham added inline comments.Dec 26 2019, 7:28 PM
kstyle/breeze.h
179

Probably makes sense to do this universally all at once, rather than in little bits and pieces.

ndavis updated this revision to Diff 72223.Dec 27 2019, 1:51 AM

Change comment formatting

ndavis marked 3 inline comments as done.Dec 27 2019, 1:52 AM
ngraham accepted this revision.Dec 27 2019, 2:59 AM

...And after these patches land, let's do the doxygen-friendly comment formatting all at once in another patch.

This revision is now accepted and ready to land.Dec 27 2019, 2:59 AM
hpereiradacosta accepted this revision.Dec 27 2019, 10:38 PM

looks good. See comment about the symbol pen width change and then ship it.

...And after these patches land, let's do the doxygen-friendly comment formatting all at once in another patch.

Most comments should already be doxygen friendly in breeze, except that * and /** are used instead of /

kstyle/breeze.h
180

For the record, * (and /**) should also be caught by Doxygen (and hopefully KDevelop). It is used more or less systematically in oxygen, so please use this instead of /

kstyle/breezehelper.cpp
1355

Here the penwidth is actually changed (from 1.1 to 1.01) this could affect the appearance of the actual buttons. Are you happy with the new appearance ?
Also, should doublecheck that this is consistent with button rendering in the decoration.

In fact for the sake of changing only one thing at a time, I would set penwidth::Symbol to 1.1

ndavis marked an inline comment as done.Dec 27 2019, 11:24 PM
ndavis added inline comments.
kstyle/breeze.h
180

OK. KDevelop won't always detect comments that use other doxygen styles or display them correctly, but that's a KDevelop problem, not a Breeze code problem.

kstyle/breezehelper.cpp
1355

Yes, 1.01 works fine. 1.1 is slightly visible when scaling up the UI, so I changed it. I had planned to do the KDecoration after this patch. I will put the width change in a different commit though.

ndavis updated this revision to Diff 72270.Dec 28 2019, 2:17 AM
ndavis marked 2 inline comments as done.

Change symbol pen width back to 1.1

ndavis marked 2 inline comments as done.Dec 28 2019, 2:19 AM
This revision was automatically updated to reflect the committed changes.