> On des. 11, 2014, 2:27 p.m., Mark Gaiser wrote: > > src/core/kfileitem.cpp, line 255 > > <https://git.reviewboard.kde.org/r/121447/diff/1/?file=332652#file332652line255> > > > > --> add here > > > > } else { > > // Fix for IO slaves that don't set UDS_MIME_TYPE for a folder. > > if (m_fileMode & QT_STAT_MASK) == QT_STAT_DIR) { > > m_entry.insert(KIO::UDSEntry::UDS_MIME_TYPE, "inode/directory"); > > m_mimeType = db.mimeTypeForName("inode/directory"); > > m_bMimeTypeKnown = true; > > } > > } > > > > Not tested! Just written in comment box :) > > > > I think that's about all you'd need to fix this. > > But if this is accaptable is probably up to David to decide. > > > > I'm also not 100% sure that you catch all cases when readUDSEntry(). > > Emmanuel Pescosta wrote: > +1 > > I also think that this is better way to fix it. > Avoids code duplication and the correct mime type for folders is set a > early as possible, so other code can rely on it. > > David Faure wrote: > This suggestion would break the case where m_guessedMimeType is set, so > it should at least that that one is empty too. > > Anyhow, the orig patch is fine, since determineMimeType is only called > once. KFileItem already takes care of caching the result. > And you can see the cache (d->m_mimeType) in currentMimeType() too, so > the new code will also only run once. > > There's no need to insert a UDS_MIME_TYPE entry. KFileItem's whole > purpose in life is to encapsulate all these details with a proper API, so as > long as it returns a correct mimetype everything's fine. > > I do agree on one thing though: a small unittest in kfileitemtest would > be nice :) > > Mark Gaiser wrote: > @David, Right, i missed that mimetype caching part which makes it a one > time call to determineMimeType. Still, it seems better to me to move it to an > initialization step since you nearly always need to know the mime type as > early as possible anyway + you only need to add code in one place (right?). > > I guess the } else { ... like i said before would become an if right > after m_guessedMimeType is set: > if (m_guessedMimeType.isEmpty() && m_fileMode & QT_STAT_MASK) == > QT_STAT_DIR) { > m_mimeType = db.mimeTypeForName("inode/directory"); > m_bMimeTypeKnown = true; > }
I will add tests and update the patch, sorry for that. - Àlex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121447/#review71804 ----------------------------------------------------------- On des. 11, 2014, 1:22 p.m., Àlex Fiestas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121447/ > ----------------------------------------------------------- > > (Updated des. 11, 2014, 1:22 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > ------- > > If we know that the item is a dir, return directly the correct mimetype for > directories. > > More info of why this is needed at: > https://git.reviewboard.kde.org/r/120909/ > > > Diffs > ----- > > src/core/kfileitem.cpp 6a2cfa5 > > Diff: https://git.reviewboard.kde.org/r/121447/diff/ > > > Testing > ------- > > > Thanks, > > Àlex Fiestas > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
