All those unrelated connect style changes would have been better in a separate commit.

Same thing the enum move could have been a separate commit.

A fallthrough would be better here.

Very welcome refactoring... in a separate commit

Just make it a const global static then... No need to recreate this hash several times.

You always use the hash with .key() which is very inefficient, so why not swap key and value types and rename it groupTypeToStateName?

Typo: hiding

Better have the curly brace on the same line than the enum name to be consistent with the other enum just on top.

Also the enum itself needs to carry over the changes which have been applied to previous commits.

That was kind of my point, isHidden could have been introduced on its own with the changes needed to remove the duplication in the same commit.

Why the intermediate struct? Could be directly the hash as a global static, should make it easier to use you wouldn't need the function below and the extra indirection.

You missed a bit, it looks like it wasn't applied on top of the latest iteration from the other patches the GroupType enum gained some values for its entries.

This can go away now

Just realized now that it'd better be named "isGroupHidden". Otherwise it'll get confusing (especially since one of the types is "PlacesType"!)

Likewise: setGroupHidden.

instead of a global static and hash, why not simply do the mapping in a function with a simple switch statement?

namespace {
QString stateNameForGroupType(KFilePlacesModel::GroupType type)
{
    switch (type) {
    case KFilePlacesModel::PlacesType:
        return QStringLiteral("GroupState-Places-IsHidden");
    ...
    }
    Q_UNREACHABLE();
}
}

why not hardcode this below? we will never hide them by default, will we?

introduce a helper lambda for this to share duplicate code

misnomer? I'd rename this function to hiddenGroups

also, take the rootBookmark by const &?

actually, remove this whole function and instead introduce a bool isGroupHidden(KFilePlacesModel::GroupType) helper function, see also below

why is this needed? i.e. doesn't this mean that once I unhide the group, all child items are still hidden? or what happens when I hide an item, then its group, and then unhide its group - will the item remain hidden or not?

is this b/c the file items don't build a tree with their groups, but rather just a list? a comment would be useful

this is overkill, what you want is only a

const bool isGroupHidden = ::isGrouphidden(item->groupType());
const bool hidingChildOnShownParent = hidden && !isGroupHidden;
const bool showingChildOnShownParent = !hidden && !isGroupHidden;

Why not naming the variable isGroupHidden and use the :: prefix on the function call like milian proposed?