D25010: [StatJob] Use A QFlag to specify the details returned by StatJob

[David Faure]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191120/6fa7c749/attachment-0001.html>