dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
About granularity: I think it's fine. I was about to say that the use case of
a recursive listing to calculate directory size only needs type and size, but
it actually needs name too (for dirs), in order to recurse into subdirs.
So overall, I think Name is always useful, and should always be provided,
whether or not the Basic flag is set.
Access, type and size aren't always needed, but they come at zero cost
(copying a few ints), while name implies string conversions (8bit->unicode). So
I don't think it makes sense to make access, type and size optional.
Hmm, with this reasoning, linkDest should be separated out. But that means
going through all users of StatBasic and finding out if they look at
UDS_LINK_DEST.
INLINE COMMENTS
> statjob.h:91
> */
> + KIOCORE_DEPRECATED_VERSION(5, 65, "Use setDetails(KIO::statDetails)")
> void setDetails(short int details);
Typo: s/stat/Stat/
> statjob.h:189
> + * @param details selects the level of details we want.
> + * You can fine grain the detail level you want.
> + * @param flags Can be HideProgressInfo here
Not sure what this specific sentence adds to the previous one. It's a
parameter, obviously that means we can choose what to pass to it... Maybe more
useful to mention that this is for performance reasons, less details implies
more speed.
> statjob.h:226
> */
> +KIOCORE_DEPRECATED_VERSION(5, 65, "Use KIO::stat(const QUrl &,
> KIO::StatSide, KIO::StatDetails int, JobFlags)")
> KIOCORE_EXPORT StatJob *stat(const QUrl &url, KIO::StatJob::StatSide side,
Why does this say "int" after "StatDetails"?
> statjob.h:234
> + * @since 5.65
> + * @deprecated since 5.65, use direcly KIO::StatDetails
> + */
Typo: direcly -> directly (happens again two lines down)
> statjob.h:236
> + */
> +KIOCORE_DEPRECATED_VERSION(5, 65, "Use direcly KIO::StatDetails")
> +KIOCORE_EXPORT KIO::StatDetails detailsToStatDetails(int details);
missing the same #if as the above methods, no?
In a "clean" build it should disappear.
> kdiroperator.cpp:748
> KJobWidgets::setWindow(job, this);
> - job->setDetails(0); //We only want to know if it exists, 0 ==
> that.
> + job->setDetails(KIO::StatBasic); //We only want to know if it
> exists
> job->setSide(KIO::StatJob::DestinationSide);
This would actually be a use case for an even more basic level, "NoDetails"...
> file.h:123
> QFile *mFile;
> + KIO::StatDetails getStatDetails();
> };
Please move it up with the other private methods. This last section is member
variables only.
Also, it could be marked as `const`.
> file_unix.cpp:542
>
> - const QString sDetails = metaData(QStringLiteral("details"));
> - const int details = sDetails.isEmpty() ? 2 : sDetails.toInt();
> + const KIO::StatDetails details = getStatDetails();
> //qDebug() << "========= LIST " << url << "details=" << details << "
> =========";
file_win.cpp needs to be ported the same way.
> file_unix.cpp:830
>
> +KIO::StatDetails FileProtocol::getStatDetails()
> +{
This should go to file.cpp so that it's available on Windows too, it's not Unix
specific.
OK, right now it's not really implemented on Windows, but let's make it easy
for the future developer who will implement it ;)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D25010
To: meven, #frameworks, dfaure, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
