[Development] New API design for file system operations

[Volker Hilsheimer]

On 8 Feb 2019, at 10:43, Vitaly Fanaskov <vitaly.fanaskov at qt.io<mailto:vitaly.fanaskov at qt.io>> wrote:

On 2/7/19 5:03 PM, Volker Hilsheimer wrote:

Hi all,


TL;DR; we are considering moving file system operations from QFile into a seperate class (or set of classes) for a more consistent and flexible way of moving, copying etc files and directories.


The conversation around the design and implementation of an API to move file or directory to the trash in https://bugreports.qt.io/browse/QTBUG-47703 has during the last days moved somewhat beyond that rather straight-forward task, and has become a broader discussion around making file operations accessible outside of the existing set of classes.


[…]

Hi,

I've read QTBUG-47703 very carefully and I think that adding some file managers is overkill in this particular case. I would suggest the following changes:

  1.  Extract an abstract class (i.e., interface) for common file operations
  2.  Both QFile and QDir should implement this interface
  3.  Unify QDir API, e.g.,  QDir::remove -> QDir::removeFile, in order to match newly-extracted interface
  4.  Add a separate class which represents system trash bin and contains appropriate methods (e.g., functionality to check files inside, restore files, empty trash etc.)

Also in my point of view, it's not necessary to introduce a new method like "moveToTrash". We can extend method "remove" adding a new parameter that might be either a bool flag or some enum.

Hi Vitaly and Tor Arne,

Making QFile::remove(bool toTrash=false); is tempting, but I think Thiago has a valid point about providing a dedicated file system operation abstraction, rather than cramming a mix of simple and complex file operations into QFile and QDir.

Looking at the documentation, QFile is an interface for reading from and writing to files; QDir provides access to directory structures and their contents. I’d like it if they were simple interfaces focusing on those specific tasks.

But in addition, they provide a mix of file path operations (ie purely string operations), and a subset of operations that affect the file system. There’s some duplication, and little consistency. IIRC these classes have changed very little since Qt 1.

Unifying QFile and QDir around a common baseclass seems difficult. QFile is a QObject via QFileDevice and QIODevice; QDir is a copyable value type. Since you can point a QDir to a file, it’s perhaps closer to a value representing a file system entry. Making them both inherit an “operations” interface just to be able to remove either a file or a directory in the same way seems cluttery, esp given the ambitions for such an “operations” interface I’m lining out above (get progress updates, cancelable, transactions, batch jobs, run in a thread etc).

Adding such an interface would clutter those APIs, and require duplication in a lot of places. A dedicated operations interface that can work with both QFile and QDir (but essentially only cares about file paths) would be nice; that doens’t mean we can’t keep a QFIle API for the simple stuff, ideally one that doesn’t accidentially block your UI event loop because the user selected a 5 GB file for copying.


For the last point: Moving a file to a trash is something an application would want to do (for previous versions of saved files, for instance; for old save-games; for old downloads; for deleted workspaces etc); I absolutely see the usecases here. But I see no value in an API that provides comprehensive access to the system trash. Unless you use Qt to build a desktop environment or a file manager, you don’t need any of this. If you do the former, then you have done this already; hello KDE. If you do the latter, then that code should be in your app, and not in everyone’s Qt library.

Happy to be convinced otherwise.


Cheers,
Volker



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190208/32d07a03/attachment.html>