Allow splitting lists using ;
AbandonedPublic

Authored by apol on Aug 23 2019, 5:01 PM.

Details

Reviewers
None
Group Reviewers
Frameworks
Summary

This is what the standard says is the separator:
The multiple values should be separated by a semicolon and the value of the key may be optionally terminated by a semicolon. Trailing empty strings must always be terminated with a semicolon. Semicolons in these values need to be escaped using \;.
https://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html

Test Plan

Running: ktraderclient5 --servicetype Application --constraint "exist Exec and (exist [X-Flatpak-RenamedFrom] and 'telegramdesktop.desktop' in [X-Flatpak-RenamedFrom])"
So far it was failing because flatpak generates these lists terminated with ;, so we were getting something like { "blah.desktop;" } instead of { "blah.desktop" }

Actually the fact that we are using , instead of ; has made that most of our desktop files use the wrong separator.
Tests pass but there's the possibility that some string lists that used to have ; now will get doubly split.

Diff Detail

Repository
R237 KConfig
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15547
Build 15565: arc lint + arc unit
apol created this revision.Aug 23 2019, 5:01 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 23 2019, 5:01 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
apol requested review of this revision.Aug 23 2019, 5:01 PM
sitter added a subscriber: sitter.Aug 26 2019, 3:30 PM

Shouldn't you be using KConfigGroup::readXdgListEntry? KConfig is strangely conflicted with it's default assumption being that you want the legacy comma lists so if you want xdg-style lists you need to call different functions altogether. That said I think Kai and I also stumbled across this same sillyness a while ago, so I am rather thinking this needs dealing with to avoid more people looking at the same issue :/

Without having looked further, my gut reaction is that changing the syntax for all existing config files due to code reuse to help an issue with desktop files might not be the backward-compatible way forward here.

For what I can tell, the issue comes from the X-Flatpak-RenamedFrom entry being defined as:

[PropertyDef::X-Flatpak-RenamedFrom]
Type=QStringList

Don't we have a type to denote this entry is a xdg-style list, not kservice/kconfig-type list? If not, we perhaps might want to add one. After all there is KConfigGroup::readXdgListEntry(...), so the basic issue is known.

Looking at KServiceTypePrivate::init() (from KService) pr parseServiceTypesFile() (in desktopfileparser from KCoreAddons) seems currently the only supported property types are bool, int, double, string, stringlist (reusing QVariant::nameToType()). So to support defining entry values with a another type/encoding like the xdg-style stringlists, that might need extension there.

Guess best is to pull @dfaure into this, this should be his domain :)

apol abandoned this revision.Aug 26 2019, 5:32 PM

Doing https://phabricator.kde.org/D23470 instead, which is less radical.

This comment was removed by bcooksley.
bcooksley removed a subscriber: fsitter.