ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed.
INLINE COMMENTS > autostartitem.cpp:60 > + m_comboBoxStartup->addItem( AutostartModel::listPathName()[1], > AutostartEntrySource::PlasmaShutdown); > + m_comboBoxStartup->addItem( AutostartModel::listPathName()[2], > AutostartEntrySource::PlasmaStart); > + Remove the space after the opening parenthesis on those three lines. It's much better like this BTW, the loop wasn't indeed buying us much for just three values. > autostartmodel.cpp:116 > +{ > + switch (index) { > + case XdgAutoStart: return XdgAutoStart; Heh, so we're playing who's the most stubborn here? I still find that "switch not saying it's a static_cast" really ugly and non idiomatic. > autostartmodel.cpp:122 > + } > + Q_UNREACHABLE(); > + return XdgAutoStart; I'd still advise getting rid of the switch but if it doesn't disappear at least move that to the "default:" case to silence potential warning on type's non exhaustion. But really... static_cast needs to appear please. Alternatively to the if I was proposing you could also do: switch (index) { case XdgAutoStart: case XdgScripts: case PlasmaShutdown: case PlasmaStart: return static_cast<AutostartEntrySource>(index); default: Q_UNREACHABLE(); } return XdgAutoStart; At least it'd be DRY and it'd be more obvious we got a cast... REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D26934 To: meven, mlaurent, ervin, #plasma, broulik, bport, crossi Cc: alex, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart