Extend File System Class

[namithj]

Extend File System Class

Pull Request

What changed?

(Please list the changes you made...)

Why did it change?

(Please list the reasons you made the changes...)

Did you fix any specific issues?

(Please tag any specific issues here...)

CERTIFICATION

By opening this pull request, I do agree to abide by the Code of Conduct and be bound by the terms of the Contribution Guidelines in effect on the date and time of my contribution as proven by the revision information in GitHub.

[namithj]

@costdev Can you take a look here before I go too far and see if there are any issues with this approach.

[costdev]

Thanks for the ping @namithj! I don't see any immediate issues.

I understand the desire to extend WP_Filesystem_Direct as this is indeed extending the underlying functionality, and there's no need to, for example, repeat the contents of WP_Filesystem_Direct::chmod() elsewhere. I think it's fine to keep going.

What other methods do you intend to introduce to this class?

[namithj]

I will be overriding the get_contents_array function to introduce a two new parameters (no of lines to read and read direction) which will let us read from bottom up and also modify it to read via file pointer and iterate over the required no of lines instead of pulling the whole content into memory as it does now.

[namithj]

I will try and push the changes today so that we can merge this in

[costdev]

This means that after calling $wp_filesystem = self::init_filesystem(), $wp_filesystem will always be truthy (objects are always truthy). The if ( ! $wp_filesystem ) condition in Debug::verify_filesystem() will never pass in this circumstance and the method will always return true.

Therefore, we can likely remove Debug::verify_filesystem() altogether since it's a private method, and self::init_filesystem() is called in all methods that call self::verify_filesystem().

[costdev]

We can now change the @return annotation in the docblock to:

@return array|WP_Error An array of lines in the file, limited to $limit, or a WP_Error object on failure.

[costdev]

Also, we can store the filesystem object to a static property to avoid re-running WP_Filesystem() and creating a new object each time.

/**
 * The filesystem.
 *
 * @var Filesystem_Direct
 */
private static $filesystem;

// init_filesystem()
if ( ! self::$filesystem instanceof Filesystem_Direct ) {
	require_once ABSPATH . 'wp-admin/includes/file.php';
	require_once ABSPATH . 'wp-admin/includes/class-wp-filesystem-base.php';
	require_once ABSPATH . 'wp-admin/includes/class-wp-filesystem-direct.php';
	WP_Filesystem();
	self::$filesystem = new Filesystem_Direct( false );
}
return self::$filesystem;

[costdev]

Suggested change

	* @return WP_Filesystem_Direct|false The filesystem object or false on failure.
	* @return Filesystem_Direct The filesystem object.

We don't use the WP_ prefix for the class name, and the method can't return false.

[costdev]

Suggested change

	*
	* @since 2.5.0

Not applicable to AspireUpdate.