Duplicate code in filesystem classes

[kurtpayne]

Each filesystem class extends a base class and most, but not all, include an implementation of chown. I propose a slight refactoring to include a default implementation of chown in the base class and let each subclass override chown when necessary.

[kurtpayne]

Sorry, all class-wp-filesystem*.php classes currently do include a chown implementation. This enhancement converges two of them ftpext and ftpsockets into the base class.

[nacin]

I noticed this recently. The base class is also missing a number of methods that some or all of the others then define. It would be much better if they listed the method as a point of reference for what the WP_Filesystem API is capable of.

[kurtpayne]

Replying to nacin:

I noticed this recently. The base class is also missing a number of methods that some or all of the others then define. It would be much better if they listed the method as a point of reference for what the WP_Filesystem API is capable of.

What if the implementations in the base class were the same as the implementations in the direct access class? This would accomplish the goal of giving a reference implementation for each method. The direct access class would be essentially empty, but open for future extension, and the API would remain intact.

[kurtpayne]

More verbose implementation of class-wp-filesystem-base.php

[dd32]

As he who wrote the classes in question:

• The Direct class was written first, and thus is the most fleshed out (without thought as to what methods would be supported over FTP)
• The FTP Sockets class was written second
• The FTP Extension class attempts to mimick the Sockets class
• The Base class had any methods common to both FTP libraries thrown into it.

The classes are not exactly standardised, I've wanted to (on many occasions) rip out all the "useless" methods and functionality which core doesn't use, or isn't implemented in all of them (For example, chown() can't be done via FTP, therefor, the abstraction shouldn't have a method for it on any of them).

I like the idea of using the Direct class as the defacto "goto place" for docs, but feel they should all be documented individually, But I don't see the need to have stubs in the baseclass - It's not a PHP5 Interface, and if all subclasses are documented correctly, there's little need for it IMHO

[dd32]

I like the idea of using the Direct class as the defacto "goto place" for docs, but feel they should all be documented individually, But I don't see the need to have stubs in the baseclass - It's not a PHP5 Interface, and if all subclasses are documented correctly, there's little need for it IMHO

I retract this statement :)

I like attachment:filesystem2.diff​​ and think it'd be the best place to document the expected behaviour of the subclasses.

[dd32]

This would go well with the Filesystem and Inline docs cleanups in 3.7.

The patch, although it'd still apply, needs some cleanup for todays docs standards.

[DrewAPicture]

docs cleanup, first-pass

[DrewAPicture]

18476.diff​ is a first-pass docs cleanup and refresh

Still need clarified parameter and return descriptions for:

::copy(),
::move(),
::delete(),
::exists(),
::is_file(),
::is_dir(),
::is_readable(),
::is_writeable(),
::atime(),
::mtime(),
::size(),
::touch(),
::mkdir(),
::rmdir(),
::dirlist()

[DrewAPicture]

Refresh at r25558

[DrewAPicture]

Remaining docs mentioned in comment:9

[DrewAPicture]

18476.3.diff​ finishes the docs for methods mentioned in comment:9.

The patch includes some docs copied from a patch on #23122, props bananastalktome

[DrewAPicture]

@since Unknown => 2.5.0

[dd32]

In 25560:

First pass at documenting the WP_Filesystem methods. This also introduces stubs of the methods into the base class which are documented, which subclasses can override, some methods were cleaned up at the same time.
See #18476 See #23122. Props kurtpayne, bananastalktome, and, DrewAPicture

[nacin]

This seems done. Please reopen if I'm wrong.