Re: plasma-framework, kactivities and kactivities-stats: please consider proper de-KF-ication now

k without Baloo? It even seems to do the right thing if I trust its CMakeLists.txt. It has Baloo as a recommended but optional dependency, and disable it altogether for Win32 builds. Regards. -- Kevin Ottens, http://ervin.ipsquad.net enioka Haute Couture - proud supporting member of KDE https://hc.enio

T13927: Pop!_os style window tiling

ervin added a comment. As far as tiling is concerned, there's a KWin script available: https://github.com/kwin-scripts/kwin-tiling I've been using it productively for a while now, it's really nice. My 0.02€ hoping it might help the conversation. If not ignore me. :-) TASK DETAIL

D28194: Fix loading button icons from qrc

ervin added a comment. In D28194#632629 , @apol wrote: > Fixing QIcon would make sense but I'd say getting this in is not the worst thing either. I don't think it's QIcon's fault to be honest. The pattern used in several places of

D29120: KCM Fonts disable AA items if they are immutable

ervin added a comment. LGTM but I'll let others weight in. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D29120 To: crossi, #plasma, ervin, bport, meven Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen,

D27188: KCM Notifications : Manage app-specific notifications with KCconfigXT's magic

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcm.cpp:315 > ManagedConfigModule::defaults(); > -m_settings->defaults(); > +for (auto *behaviorSettings : m_behaviorSettingsList) { > +

D28461: In sidebar mode show if a module is in default state or not

ervin accepted this revision. ervin added a comment. LGTM REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D28461 To: bport, #plasma, ervin, meven, crossi, hchain, #vdg, mart Cc: GB_2, mart, ngraham, abetts, filipf, The-Feren-OS-Dev, ndavis, broulik,

D28462: [KCM][WIP] Add KCModuleData

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Only a small thing left I think INLINE COMMENTS > cursorthemedata.h:33 > +Q_OBJECT > +//Q_PROPERTY(CursorThemeSettings *cursorThemeSettings READ > cursorThemeSettings

D27841: Port desktoptheme, icons and workspace KCMs to SettingStateBinding

This revision was automatically updated to reflect the committed changes. Closed by commit R119:dfc144bf3f45: Port desktoptheme, icons and workspace KCMs to SettingStateBinding (authored by ervin). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D26934: KCM/Autostart Add a model to separate logic from UI

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > autostart.cpp:167 > { > -KOpenWithDialog owdlg( this ); > -if (owdlg.exec() != QDialog::Accepted) > -return; > +KOpenWithDialog* owdlg =

D28462: [KCM] Add color state probe to allow system settings to display current default state

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > colorsstateprobe.cpp:29 > +: KCModuleStateProbe(parent, args) > +, m_settings(new ColorsSettings(this)) > +{ I'm not sure I see the point of doing

D28461: In sidebar mode show if a module is in default state or not

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > MenuItem.h:160 > + > +void updateDefault(); > + I don't like the name much, I think it could be confused with updating actual default value or such...

D28331: KCM/mouse KCM/touchpad: Add a Scroll speed setting for wayland

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. LGTM REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28331 REVISION DETAIL https://phabricator.kde.org/D28331 To: meven, #kwin, #plasma, davidedmundson, ervin, bport, crossi, hchain

D26934: KCM/Autostart Add a model to separate logic from UI

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > broulik wrote in autostart.cpp:182 > Yes. > It then also needs to set a transient parent and window modality manual I > think Yes, otherwise you wouldn't get

D26934: KCM/Autostart Add a model to separate logic from UI

ervin added inline comments. INLINE COMMENTS > meven wrote in autostartmodel.h:52 > I see, I meant my code to have all Roles used explicitly declared here rather > than relying on the developer knowing about Qt::DisplayRole. Weeell... knowing about Qt::DisplayRole is kind of prerequisite to

D28331: KCM/mouse KCM/touchpad: Add a Scroll speed setting for wayland

ervin added inline comments. INLINE COMMENTS > main.qml:333 > +15, > +20]; > + nitpick: I'd put the closing square bracket on the next line and you can drop the semicolumn > main.qml:343 > +value = index; > +} > +

D26934: KCM/Autostart Add a model to separate logic from UI

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Damn turns out I still missed a few, so what @broulik says (when it still applies). @broulik : are you sure about all your comments? It looks like something odd happened, some

D26934: KCM/Autostart Add a model to separate logic from UI

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. Thanks for your patience :-) REPOSITORY R119 Plasma Desktop BRANCH D26934 REVISION DETAIL https://phabricator.kde.org/D26934 To: meven, mlaurent, ervin, #plasma, broulik, bport,

D26934: KCM/Autostart Add a model to separate logic from UI

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. One last nitpick INLINE COMMENTS > autostartmodel.cpp:121 > +case PlasmaStart: > +return static_cast (index); > +default: Please remove the spurious space before (

D26934: KCM/Autostart Add a model to separate logic from UI

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); > +

D27063: Fix disabeling of autolock timeout

ervin added a comment. Ah didn't know! I assumed you could push... But yeah, you authored a few patches now, time to apply for a developer account. You can put my name indeed. REPOSITORY R133 KScreenLocker BRANCH fix_disabled_state REVISION DETAIL https://phabricator.kde.org/D27063

D27063: Fix disabeling of autolock timeout

ervin added a comment. Well, I already accepted it, I thought you pushed long ago (and I suspect it got overlooked by the other potential reviewers). :-) REPOSITORY R133 KScreenLocker BRANCH fix_disabled_state REVISION DETAIL https://phabricator.kde.org/D27063 To: alex, mart, ervin

D27971: Solid-device-automounter/kcm: correctly update automountOn

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > DeviceAutomounterKCM.cpp:66 > > -auto emitChanged = [this] { > - > m_devices->setAutomaticMountOnLogin(kcfg_AutomountOnLogin->isChecked()); > -

D27956: [Various KCMs] Notify about changes in GTK related settings

ervin added a comment. In D27956#625419 , @ngraham wrote: > I'm finding myself wondering why we don't just make everything notify by default. IMHO it's mostly a question of limiting the chatter on the bus. REPOSITORY R119 Plasma

D27944: KCM Colors fix apply button always disabled

ervin added a comment. I marked it accepted, but of course this is assuming David's patch wouldn't make it quickly and we'd need the fix here ASAP. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27944 To: crossi, #plasma, ervin, bport, meven, broulik Cc:

D26934: KCM/Autostart Add a model to separate logic from UI

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > autostart.h:52 > +void updateDesktopStartItem(DesktopStartItem *item, const QString , > const QString , bool disabled, const QString ); > +void

D27155: libnotificationmanager : add app-specific kconfig settings

ervin accepted this revision. ervin added a comment. Although might become "parentGroupName" depending on what you do about my comments on the review introducing sub-group handling. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27155 To: crossi, ervin,

D27841: Port desktoptheme, icons and workspace KCMs to SettingStateBinding

ervin created this revision. ervin added reviewers: crossi, hchain, meven, bport, davidedmundson, mart, ngraham. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ervin requested review of this revision. TEST PLAN Has been tested with the three modules mentioned in the

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin added inline comments. INLINE COMMENTS > bport wrote in spellchecking.cpp:88 > Yes, or I can probably also use item like before, and that will be handled > for me That would create other issues I think (not fully sure TBH). Well in any case that's an extra level of indirection we don't

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin added inline comments. INLINE COMMENTS > bport wrote in spellchecking.cpp:88 > no load only do findItem for managed widget on the skeleton Sure but still, the skeleton is expected to be already be in a "loaded" state at that point, that's why it works for the items (otherwise a findItem

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ervin wrote in spellchecking.cpp:55 > What about calling toSet() on those? Not overly efficient but would compress > a bit the resulting code. Alternatively,

D26934: KCM/Autostart Add a model to separate logic from UI

ervin added inline comments. INLINE COMMENTS > autostart.cpp:87 > > +setMinimumHeight(300); > +setMinimumWidth(400); Unrelated right? Where do this come from? > autostart.cpp:112 > > -void Autostart::addItem( DesktopStartItem* item, const QString& name, const > QString& run, const

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > spellchecking.cpp:55 > + > +auto referenceValue = m_settings->currentIgnoreList(); > +auto currentValue = m_configWidget->ignoreList(); What about

D27380: [GTK Config] Construct font style by hand instead of relying on Qt function

ervin added a comment. LGTM, now requires swift testing to not miss the next bugfix release REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D27380 To: gikari, #plasma, ervin, bport, meven, davidedmundson Cc: chauvin, davidre, davidedmundson, cfeck,

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > spellchecking.cpp:30 > +#include > +#include > +#include Good opportunity to switch to the camel case includes? > spellchecking.cpp:40 >

D27482: Update kdeglobals config file for Breeze widgetStyle

ervin added inline comments. INLINE COMMENTS > bport wrote in style_widgetstyle_default_breeze.pl:8 > This will fix only for Breeze theme > This will fix existing case but we also need to fix the root cause (i.e. how > we end up with a lowercase name). Sure but that would be a different patch

D27380: [GTK Config] Construct font style by hand instead of relying on Qt function

ervin added a comment. A minor style nitpick, otherwise LGTM. As mentioned by the author, this likely require extensive testing before going in. INLINE COMMENTS > configvalueprovider.cpp:53 > > +QString ConfigValueProvider::fontStyleHelper(const QFont& font) const > +{ Space should be

D27024: Solid-device-automounter/kcm: show disconnected known device when disconnecting it

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. I guess extra care will need to be taken with your other patch moving away from the singleton (dunno which one is based on which, I might review out of order). REPOSITORY R119 Plasma

D27480: Solid-device-automounter/kcm: Get rid of singleton for AutomounterSettings

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > DeviceAutomounterKCM.cpp:44 > : KCModule(parent, args)//DeviceAutomounterKCMFactory::componentData(), > parent) > - , m_devices(new

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > DeviceAutomounterKCM.cpp:44 > : KCModule(parent, args)//DeviceAutomounterKCMFactory::componentData(), > parent) > + , m_devices(new

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

ervin added inline comments. INLINE COMMENTS > meven wrote in DeviceAutomounterKCM.cpp:57 > Please beware this will make my patch quite a lot more intrusive, DeviceModel > for instance, will need a field to keep some reference to the > AutomounterSettings Sure, moving away from a singleton is

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

ervin added inline comments. INLINE COMMENTS > meven wrote in DeviceAutomounterKCM.cpp:57 > I removed the m_settings instead, relying solely on the singleton. I tend to consider this as a step back to be honest. Singletons tend to be more trouble down the line when something goes wrong.

D27052: Solid-device-automounter/kcm: Convert some foreach

ervin added inline comments. INLINE COMMENTS > meven wrote in DeviceAutomounter.cpp:73 > Unfortunately this is not possible line 78 requires a non-const ref to volume. const auto for volumes was possible though. I wonder if it wouldn't have made sense to have a non const ref in the loop then.

D27156: KCM Fonts port anti aliasing part to KPropertySkeletonItem

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. Looks good to me now, maybe try to get more reviewers before pushing though. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27156 To: bport, #plasma, ervin,

D27407: Breeze widgetStyle value is Breeze

ervin added a comment. Shall we expect the migration script in that patch or another one? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27407 To: crossi, ervin, bport, meven, mart, davidedmundson, ngraham Cc: ngraham, plasma-devel, Orage, LeGast00n,

D27380: [GTK Config] Write Font without style name

ervin added a comment. Can we give a shot to @davidedmundson proposal? Maybe slightly extended to cover some extra enum cases if necessary? REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D27380 To: gikari, #plasma, ervin, bport, meven Cc: davidre,

D27395: KCM/ComponentChooser Treat cases when there is no app for a usage

ervin added inline comments. INLINE COMMENTS > componentchooseremail.cpp:155 > KSharedConfig::Ptr profile = > KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), > KConfig::NoGlobals, QStandardPaths::GenericConfigLocation); > -if (profile->isConfigWritable(true) &&

Re: 2 kirigami fixes for a point release

uses buggy releases? Well, I'd first ask, which parts of frameworks are buggy in 5.67? Which are the affected frameworks? If we're talking about a couple in a set of 80, I'd first ask what those have in common that the others don't? I'd look at all that before blaming and overhauling the release mod

D27188: KCM Notifications : Manage app-specific notifications with KCconfigXT's magic

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcm.cpp:277 > +if (!m_currentBehavior) { > +setBehaviorSettingsToLoad(m_behaviorSettingsList.begin().key()); > +} What about

D27127: Solid-device-automounter/kcm: Use KConfigXT in ui

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > DeviceAutomounterKCM.cpp:57 > > +m_settings = new AutomounterSettings(); > +addConfig(m_settings, this); Missing this as parent, this is leaked. Also

D27024: Solid-device-automounter/kcm: show disconnected known device when disconnecting it

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > DeviceModel.cpp:101 > // is no longer available when the device is gone > +// Unless we have some settings for the device > +if

D27116: KCM/Component email: simplify code

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Tiny code style breakage INLINE COMMENTS > componentchooseremail.cpp:85 > +setCurrentIndex(count() -1); > +m_currentIndex = count() -1; > } The spaces should

D27052: Solid-device-automounter/kcm: Convert some foreach

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. Just a nitpick, your call if you want to tackle it or not. INLINE COMMENTS > DeviceAutomounter.cpp:73 > +const QList volumes = >

D27156: KCM Fonts port anti aliasing part to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > fonts.cpp:136 > +for (KXftConfig::Hint::Style s : {KXftConfig::Hint::None, > KXftConfig::Hint::Slight, KXftConfig::Hint::Medium, KXftConfig::Hint::Full}) {

D27156: KCM Fonts port anti aliasing part to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > fonts.cpp:131 > +for (int t = KXftConfig::SubPixel::None; t <= > KXftConfig::SubPixel::Vbgr; ++t) { > +QStandardItem *item = new >

D27057: Solid-device-automounter/kcm: Enable/Disable columns automount onLogin/onAttached depending on corresponding checkbox

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > DeviceModel.cpp:185 > if (index.isValid()) { > -if (index.parent().isValid()) { > -if (index.column() > 0) { > -return

D27053: Solid-device-automounter/kcm: Improve width of columns

ervin added a comment. LGTM, waiting for nate's opinion. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27053 To: meven, ngraham, ervin, #plasma Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot,

D27063: Fix disabeling of autolock timeout

ervin added a comment. Looks like it's caused by the initial state of the ui file being "wrong" (checkbox unchecked and spinbox enabled), what about simply having the spinbox disabled in the ui file? Maybe we miss a connect there too. It's the kind of ui specific details I try to push out

D26834: libnotificationmanager : deprecate Settings ctor that takes a config

ervin added inline comments. INLINE COMMENTS > settings.cpp:40 > +{ > +static const char s_configFile[] = "plasmanotifyrc"; > +} I don't think that's what Kai had in mind regarding his comment. @broulik could you confirm? REPOSITORY R120 Plasma Workspace REVISION DETAIL

D26797: KCM/Component Refactor UI to a single list of combobox

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > componentchooser.cpp:62 > +// fill the form layout > +const auto name = cg.readEntry("Name",i18n("Unknown")); > +CfgPlugin

D26842: Fix fonts KCM button state

ervin added inline comments. INLINE COMMENTS > broulik wrote in fonts.cpp:479 > Kinda defies the purpose of using kconfigxt if we end up hardcoding the state > in code again? That part was never transitioned to KConfigXT though since it doesn't have a KConfig backend. Clearly inheriting by

D26842: Fix fonts KCM button state

ervin added inline comments. INLINE COMMENTS > fonts.cpp:472 > > +bool FontAASettings::isDefaults() const { > +State defaultState{}; The curly brace is misplaced REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D26842 To: bport, #plasma, ervin, crossi,

D26842: Fix fonts KCM button state

ervin added inline comments. INLINE COMMENTS > ngraham wrote in fontssettings.kcfg:77 > Won't this worsen https://bugs.kde.org/show_bug.cgi?id=378523? I guess this depends on how the font get serialized... and looking into KConfig we use toString() so stylename will appear AFAICT. REPOSITORY

D26047: Expose KConfig settings to allow registration in KCM Notification

ervin accepted this revision. ervin added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > broulik wrote in settings.cpp:173 > I still want to be able to specify what `config` (constructor argument) to > use for autotests After discussing with kai, let's mark

D26048: KCM Notification port to ManagedConfigModule

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Wondering, do we still need "settings"? I guess it's for the per-app settings? INLINE COMMENTS > kcm.cpp:115 > + > +registerSettings(m_dndSettings); > +

D26047: Expose KConfig settings to allow registration in KCM Notification

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. I like it, just an unwanted change to clean up still. @broulik what say you? INLINE COMMENTS > settings.h:33 > > +class KCoreConfigSkeleton; > + This change isn't needed

D26797: KCM/Component Refactor UI to a single list of combobox

ervin added inline comments. INLINE COMMENTS > meven wrote in componentchooser.cpp:98 > I kept it as the compiler might be able to do some optimization in the loop > when val is true line 105. > It may skip the `plugin->hasChanged();` calls in that case, but I am not sure > it gcc is clever

D26797: KCM/Component Refactor UI to a single list of combobox

ervin added inline comments. INLINE COMMENTS > componentchooser.cpp:98 > > +void ComponentChooser::emitChanged(bool val) > +{ I still think val is useless. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D26797 To: meven, #plasma, #vdg, ngraham, ervin Cc:

D26797: KCM/Component Refactor UI to a single list of combobox

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > componentchooser.cpp:80 > + > +if (cfgType==QLatin1String("internal_email")) { > +loadedConfigWidget = new CfgEmailClient(this); I know it's older

D26565: KCM/Component Revamp email config

ervin accepted this revision. ervin added inline comments. INLINE COMMENTS > componentchooseremail.cpp:109 > +} > +if (service) { > +// avoid duplicates entry when email clients are present in > mimeapps.list's Added Associations too I really meant if (!service) +

D26784: KCM KDED: Add immutability and fix default, reset, apply buttons

ervin added inline comments. INLINE COMMENTS > kcmkded.cpp:234 > const bool autoloadEnabled = > idx.data(ModulesModel::AutoloadEnabledRole).toBool(); > +m_model->setData(idx, > autoloadEnabled,ModulesModel::AutoloadEnabledSavedRole); > KConfigGroup cg(, >

D26731: KCM/Component Revamp Browser config

ervin added inline comments. INLINE COMMENTS > meven wrote in componentchooserbrowser.cpp:79 > Do we want to set falkon or konqueror as default ? If any it'd be falkon I think REPOSITORY R119 Plasma Desktop BRANCH browser-settings REVISION DETAIL https://phabricator.kde.org/D26731 To:

D26705: KCM/Component Revamp Terminal Emulator UI

ervin added inline comments. INLINE COMMENTS > componentchooserterminal.cpp:46 > > -void CfgTerminalEmulator::configChanged() > -{ > - emit changed(true); > +void CfgTerminalEmulator::selectTerminalEmulator(int index) { > +Q_UNUSED(index) Curly brace should be on its own line >

D26565: KCM/Component Revamp email config

ervin added inline comments. INLINE COMMENTS > componentchooseremail.cpp:67 > } > +static const char s_AddedAssociations[] = "Added Associations"; > +static const auto s_mimetype = QStringLiteral("x-scheme-handler/mailto"); Not a huge fan of static consts appearing like this in the middle of

D26523: KCM kded, fix immutability and reset/apply/default button state

ervin added inline comments. INLINE COMMENTS > kcmkded.cpp:319 > + if (_lvStartup->topLevelItem( i )->flags() & Qt::ItemIsEnabled) > { > + _lvStartup->topLevelItem( i )->setCheckState( > StartupUse, Qt::Checked ); > + } I know you didn't write that

D26398: [KCM/Activities] Use KConfigXT in ui

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > BlacklistedApplicationsModel.cpp:191 > +emit > changed(d->pluginConfig->findItem("blockedApplications")->isSaveNeeded() && >

D26389: DesktopPaths KCM: Move the settings logic to a KCoreConfigSkeleton class

This revision was automatically updated to reflect the committed changes. Closed by commit R119:ea8009b35011: DesktopPaths KCM: Move the settings logic to a KCoreConfigSkeleton class (authored by ervin). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D26390: DesktopPaths KCM: Move the view logic in a ui file

This revision was automatically updated to reflect the committed changes. Closed by commit R119:f9fb111537f7: DesktopPaths KCM: Move the view logic in a ui file (authored by ervin). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26390?vs=72678=73051

D26391: DesktopPaths KCM: Remove the moving directory logic

This revision was automatically updated to reflect the committed changes. Closed by commit R119:db1ebdc9090a: DesktopPaths KCM: Remove the moving directory logic (authored by ervin). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26391?vs=72679=73052

D26242: Port the translations module to ManagedConfigModule

This revision was automatically updated to reflect the committed changes. Closed by commit R120:34216709c060: Port the translations module to ManagedConfigModule (authored by ervin). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26242?vs=72243=73048

D26388: Get rid of KGlobalSettings

This revision was automatically updated to reflect the committed changes. Closed by commit R119:29c01adcc4a5: Get rid of KGlobalSettings (authored by ervin). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26388?vs=72676=73049 REVISION DETAIL

D26241: Remove Kirigami DelegateRecycler

This revision was automatically updated to reflect the committed changes. Closed by commit R120:56529e7be715: Remove Kirigami DelegateRecycler (authored by ervin). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26241?vs=73046=73047 REVISION DETAIL

D26241: Remove Kirigami DelegateRecycler

ervin updated this revision to Diff 73046. ervin added a comment. Fix drag and drop issues after discussing with Marco REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26241?vs=72241=73046 REVISION DETAIL https://phabricator.kde.org/D26241

D26518: ModuleView: Hide button when KCModule don't need them

ervin accepted this revision. ervin added a comment. Doesn't quite make sense to both hide and disable, but no big deal. INLINE COMMENTS > ervin wrote in ModuleView.cpp:398 > Nope, see my comment on the other patch :-) Alright, I'll contradict myself, the doc claimed the contrary forever.

D26518: ModuleView: Hide button when KCModule don't need them

ervin added inline comments. INLINE COMMENTS > meven wrote in ModuleView.cpp:398 > `d->mHelp->setVisible(buttons & KCModule::Help )` ? Nope, see my comment on the other patch :-) REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D26518 To: bport, ervin, meven,

D26398: [KCM/Activities] Use KConfigXT in ui

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. A few changes needed to make the code easier to understand in a few months time. Also there's a larger concern of a piece of code being prone to later bugs (although I'd expect it

D26467: KCM runners: fix default button

ervin added a comment. In D26467#588812 , @davidedmundson wrote: > We're entering an awkward part of the Plasma release cycle. > > Our next beta is in 3 weeks but will depend on the framework that has just been tagged. > Any changes

D26401: KCM Baloo: Migrate to KConfigXT and add immutability

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > filteredfoldermodel.cpp:5 > * Copyright (C) 2019 Tomaz Canabrava > - * > + Copyright (c) 2020 Benjamin Port > * This library is free software; you can

D26467: KCM runners: fix default button

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. Waiting on D26466 to get in of course. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D26467 To: bport, #plasma, ervin,

D26456: KCM runners : fix reset and default behavior

ervin accepted this revision. ervin added a comment. Might require further work to nail down the "back to defaults" case which is not handled by KPluginSelector currently REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D26456 To: bport, #plasma, ervin,

D26389: DesktopPaths KCM: Move the settings logic to a KCoreConfigSkeleton class

ervin updated this revision to Diff 72848. ervin added a comment. Addresses bport comments REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26389?vs=72677=72848 REVISION DETAIL https://phabricator.kde.org/D26389 AFFECTED FILES

D26390: DesktopPaths KCM: Move the view logic in a ui file

ervin created this revision. ervin added reviewers: Plasma, crossi, bport, meven, mart, davidedmundson. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ervin requested review of this revision. REVISION SUMMARY Now the revert/apply/defaults buttons work as expected.

D26391: DesktopPaths KCM: Remove the moving directory logic

ervin created this revision. ervin added reviewers: Plasma, crossi, bport, meven, mart, davidedmundson. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ervin requested review of this revision. REVISION SUMMARY This simplifies greatly this otherwise mundane KCM. It

D26388: Get rid of KGlobalSettings

ervin created this revision. ervin added reviewers: Plasma, mart, davidedmundson. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ervin requested review of this revision. REVISION SUMMARY After grepping in all our repositories, it looks like the SETTINGS_PATHS

D26389: DesktopPaths KCM: Move the settings logic to a KCoreConfigSkeleton class

ervin created this revision. ervin added reviewers: Plasma, crossi, bport, meven, mart, davidedmundson. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ervin requested review of this revision. REVISION SUMMARY Our KCoreConfigSkeleton subclass is interestingly hand

D26324: [KCM/Component] filemanager: make dolphin the default filemananger

ervin added inline comments. INLINE COMMENTS > componentchooserfilemanager.cpp:32 > + > +QRadioButton *findDolphinRadio(const QList ) { > +auto it = std::find_if(radioButtons.begin(), radioButtons.end(), > [=](QRadioButton *radio) { And of course as I was about to hit "accept" I notice

D26324: [KCM/Component] filemanager: make dolphin the default filemananger

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Note the :: prefix is likely unnecessary in this context (doesn't hurt either). INLINE COMMENTS > componentchooserfilemanager.h:46 > private: > +QRadioButton*

D26324: [KCM/Component] filemanager: make dolphin the default filemananger

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Almost there. INLINE COMMENTS > componentchooserfilemanager.cpp:41 > > +QRadioButton * findDolphinRadio(QList dynamicRadioButtons) { > +auto it =

D26324: [KCM/Component] filemanager: make dolphin the default filemananger

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > componentchooserfilemanager.cpp:57 > + > +QRadioButton* CfgFileManager::findDolphinRadio() const { > +auto it = std::find_if(mDynamicRadioButtons.begin(),

D26324: [KCM/Component] filemanager: make dolphin the default filemananger

ervin added a comment. Damn, sorry mate... I meant find_if... dunno why I spluttered any_of... :-/ Indeed if you make a tiny wrapper function for find_if on the "storageId == org.kde.dolphin.desktop" condition, you can use it both in defaults and isDefaults. In one case it's just

D26324: [KCM/Component] filemanager: make dolphin the default filemananger

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. Note that I think std::any_of wrapped in a helper to get an iterator on the dolphin radio button would make that code simpler. To be checked maybe? Not mandatory at all, feel free to ignore

  1   2   3   4   >