Re: Move Breeze to Framework

> requests regarding the window decoration in breeze were closely linked to > other changes in KDecoration. This probably would make sense even if we end > up keeping breeze in plasma. > - Colors: these were already moved to KColorScheme, so we could remove > the duplication one wa

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

7;t Elisa able to work 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 memb

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 h

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 `(icon

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, Zr

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) { > +beha

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, plasma-de

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 CONST

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 https://phabricator.kde.org/D27841?vs

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 = new

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 this.

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... upda

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 C

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 a

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 mak

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; > +} > + You

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, cross

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

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 T

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 C

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 Deskto

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: dav

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 &name, > const QString &command, bool disabled, const QString &fileName)

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, br

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 co

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 r

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 w

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, co

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 calli

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, n

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 > +SonnetSpellCheckin

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 bef

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 Deskt

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 DeviceModel(this))

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 DeviceModel(this))

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. REPOSI

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, The-Fe

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) && emailClientSe

Re: 2 kirigami fixes for a point release

cause. We > have to ask: what causes 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 loo

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 m_behaviorSettingsL

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 (Automou

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 b

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 = > Solid::Device::listFromType(Solid::DeviceInterface::Stora

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 > QStandardItem(KX

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 Q

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, ngr

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 of

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 https://phabricato

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 *loadedConfigW

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 ha

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, meve

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); > +registerSettings(m_no

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 anymore.

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 eno

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: plas

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 c

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

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(&kdedrc, > QStri

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 > compon

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 th

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 l

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() && > d->pluginConfig->findItem("all

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 https://phabricator.kde.org/D26

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&id=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&id=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&id=730

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&id=73049 REVISION DETAIL https

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&id=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&id=73046 REVISION DETAIL https://phabricator.kde.org/D26241 AFF

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. R

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, cr

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 t

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 enter

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 re

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, m

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, crossi,

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&id=72848 REVISION DETAIL https://phabricator.kde.org/D26389 AFFECTED FILES kcms/desktoppath

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. REPO

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 introd

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 category

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 writte

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

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

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* findDolphinRadio

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 = std::find_if(dynamicRadioButt

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 ab

  1   2   3   4   5   6   7   >