Many applications like kdevelop, digikam, labplot, etc. create a menu "Color Scheme" in the main menu bar and add then menu item via KColorSchemeManager. By using "System color scheme" here we'd have "Color Scheme" -> "System color scheme" with this repeated "color scheme" string. Can we simply use "Default" or "System" or "Desktop" here?

if (index.row() == 0) would also do the job and is simpler and faster.

What should be the use-case for this new function and should it really be a slot?

"Default" is probably fine.

FWIW the parent menu item is actually mis-named, at least in Kate. It's called "Color Theme" when it should be "Color Scheme"

Also this menu should be universal, and not re-implemented on a per-app basis.

Yes, was also wrong in LabPlot. Just corrected https://invent.kde.org/kde/labplot/commit/262f37b59193ed88bf680b155dc6ecd37bd11419.

We should have maybe in this class only one public function KActionMenu *createSchemeSelectionMenu(QObject *parent) which internally sets the string to "Color Scheme" and the icon to "preferences-desktop-color". With this we'd enforce a consistent look. Or this is menu is automatically created for KXmlGuiWindow applications...

Or this is menu is automatically created for KXmlGuiWindow applications...

That's what I was thinking of, yeah.

I thought it would be nice to have as a convenience function if an application has changed the scheme to easily go back to the system color scheme. I don't know if it should be a slot. I put it there because activateScheme is here.

I think the only communication channel with KColorSchemeManager is via the menu created in createSchemeSelectionMenu(). Setting back the default scheme will also be done via this menu. I don't see why an application would need such a helper function - there is simply no other handling of the color schemes in the applications except of that menu.

Also, if somebody would still need such a helper function, he/she could use activateScheme(QModelIndex()) instead of followSystemScheme(). activateScheme() exists already and an empty QModelIndex() could be interpreted as the default color scheme.

I like that idea with an empty ModelIndex.

Would I need to add @since for a new overload?

I would say yes (your question made me search for an answer since I wanted too:)), from https://community.kde.org/Guidelines_and_HOWTOs/API_Documentation:

The @since tag tells users since when the class has existed. It is usual to put a KDE release number here.

Method documentation: We can use @author and @since just like we do for classes.

s/wanted/wanted to know/ (phabricator doesn't let us edit inline comments.... weird).

"Would I need"... put yourself in the shoes of a user of this class: you would want to know at which version this new overload is available when using it in your code which sets itself a certain version of KF as minimal expected, right? :)

So yes, you want to help the users of the API to know at which versions of KF they can expect which methods and classes to be present. And thus you want to ensure "@since" with any API changes, incl. new methods added, overloaded or not.

Propose to add a short comment explaining why we return index(0) here for the future code reader starting off at this code before having read all code and docs. E.g.
// empty string is mapped to "reset to default/system", which is first item in model
That ensures the intention of this code is clear on high level.

Could start off at 1 now, no?

also could get a short comment what the highlevel logic is here
// no color theme selected? so it's the default one

Using // for each comment line is more typical for explanation comments, why the different style here?

Here you might also want to state at which version this feature of "allows going back" was added, to make it clear that with older versions of KF this was not possible.
Same with the existing methods where behaviour was changed.

Because I didn't see another multiline comment in this file to compare to ;)

why not to use reasonable default values here like QIcon::fromTheme(QStringLiteral("preferences-desktop-color") and i18n("Color Scheme")? This function should be used then by all applications and this will help to get a more consistent behavior across the different applications.

Should we also do this for the other overloads? Or would that behavior change be a blocker? ALso an application would want to call KColorSchemeManager::createSchemeSelectionMenu(const QString &selectedSchemeName, QObject *parent) probably if the custom scheme is saved between launches.

I'm not the author of this code but I don't see why this should be a blocker. With this you wouldn't break anything and would simply add more consistency across different applications.