dfaure added a comment.
Thanks for looking into this, I'm glad that finally someone does dig into
this code.
I'm a bit surprised by the solution though. The spec simply says
- add any "Default Applications" and then "Added Associations" in the first
mimeapps.list
This doesn't need any reverse iteration or prepending, it's just about
reading Default Applications before Added Associations rather than the other
way around, isn't it?
I think my comment 5 years ago also meant that the xdg spec allows for the
default app (left-click in file manager) to be different from the preferred app
(RMB / Open With).
But looking at the spec now (Application ordering) it really treats "Default
Applications" as higher-priority Added Associations, which proves that having
separated the two is just complete nonsense, they serve the same purpose. Bleh.
At least it's easier to implement this way :-)
INLINE COMMENTS
> kmimeassociations.cpp:112
> + const QStringList services = group.readXdgListEntry(mimeName);
> + // since the first has precendence and we prepend entries, we need
> to reverse the list of service
> + std::reverse(services.begin(), services.end());
typo: precedence
> kmimeassociations.cpp:113
> + // since the first has precendence and we prepend entries, we need
> to reverse the list of service
> + std::reverse(services.begin(), services.end());
> + const QString resolvedMimeName =
> mimeName.startsWith(QLatin1String("x-scheme-handler/")) ? mimeName :
> db.mimeTypeForName(mimeName).name();
Iterating with rbegin/rend (instead of the range-for on line 119) would be
faster.
Or .... factorize the rest of the loop with the other method, since that's all
duplicated otherwise? Then that's a good reason to keep the same range-for.
> kmimeassociations.cpp:125
> + //qDebug() << "prepending mime" << resolvedMimeName <<
> "to service" << pService->entryPath() << "pref=" << pref;
> + m_offerHash.prependServiceOffer(resolvedMimeName,
> KServiceOffer(pService, pref, 0, pService->allowAsDefault()));
> + --pref;
Replace the last argument with true, clearly this service is allowed as default
:)
(I'll remove allowAsDefault in KF6 anyway)
> kmimeassociations.cpp:200
>
> +void KOfferHash::prependServiceOffer(const QString &serviceType, const
> KServiceOffer &offer)
> +{
I must be tired, but I don't see any difference between the code of this method
and the one in addServiceOffer?
(and depending on what the difference should be, I think this should be a
single method with an enum argument, to reduce duplication)
REPOSITORY
R309 KService
REVISION DETAIL
https://phabricator.kde.org/D26690
To: meven, dfaure, dvratil, ervin, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
