> On Aug. 5, 2013, 7:46 a.m., David Faure wrote: > > kdecore/services/kmimetype_p.h, line 48 > > <http://git.reviewboard.kde.org/r/111852/diff/2/?file=176232#file176232line48> > > > > Why did you make this a new virtual method? > > The concept doesn't exist in KServiceType, so I don't see the point. > > > > I would have left KMimeType::patterns() unchanged, it seemed fine. > > > > You copied the way comment() works, but it's different because there's > > a KServiceTypePrivate::comment().
Because you mentioned the code should just return patterns(), but that's at KMimeTypePrivate::property, and KMimeTypePrivate had no patterns() method, so I thought you meant moving the ensureXmlLoaded() + return m_listPatterns to KMimeTypePrivate, the same way KMimeType::comment is implemented. The other option would be to return q->patterns() but there's no q_ptr in KMimeTypePrivate. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111852/#review37101 ----------------------------------------------------------- On Aug. 5, 2013, 1:38 a.m., David Narváez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111852/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2013, 1:38 a.m.) > > > Review request for kdelibs, Albert Astals Cid and David Faure. > > > Description > ------- > > Comment information is found in the XML data so needs to be loaded before > returning property values. The same would apply for the Patterns property if > you were to query that from the property name instead of the > KMimeType::patterns() method. > > > This addresses bug 322578. > http://bugs.kde.org/show_bug.cgi?id=322578 > > > Diffs > ----- > > kdecore/services/kmimetype.cpp d748523 > kdecore/services/kmimetype_p.h 6ffee71 > kdecore/tests/kmimetypetest.h c38b036 > kdecore/tests/kmimetypetest.cpp da32e2c > > Diff: http://git.reviewboard.kde.org/r/111852/diff/ > > > Testing > ------- > > Executed the same code posted in the bug report, here's the output: > > ("Name", "Comment", "Patterns", "Icon") > QVariant(QString, "PNG image") > > > Thanks, > > David Narváez > >
