Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mkdir function when symlink is created #14

Closed

Conversation

umandalroald
Copy link
Member

For issue 12 #12

Copy link
Member

@carl-alberto carl-alberto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For complete coding standards and phpcs setup, please refer here https://github.com/WordPress/WordPress-Coding-Standards

@@ -131,7 +131,11 @@ public function save_symlinks() {
// successfully add
// fail error.
$value = $destination . ' -> ' . $source;

// Get the target folder name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring here https://github.com/WordPress/WordPress-Coding-Standards

phpcs includes/lib/class-easy-symlinks-functions.php --standard=WordPress

FILE: wp-content/plugins/easy-symlinks/includes/lib/class-easy-symlinks-functions.php

FOUND 11 ERRORS AFFECTING 4 LINES

134 | ERROR | [ ] Inline comments must end in full-stops, exclamation marks, or question marks
135 | ERROR | [x] Expected 1 spaces after opening bracket; 0 found
135 | ERROR | [x] Expected 1 spaces before closing bracket; 0 found
135 | ERROR | [x] String "/^./uploads/\W?\K.*/" does not require double quotes; use single quotes instead
137 | ERROR | [x] Expected 1 spaces after opening bracket; 0 found
137 | ERROR | [x] Expected 1 spaces before closing bracket; 0 found
137 | ERROR | [ ] Detected usage of a non-validated input variable: $_SERVER
137 | ERROR | [ ] Missing wp_unslash() before sanitization.
137 | ERROR | [ ] Detected usage of a non-sanitized input variable: $_SERVER
137 | ERROR | [x] String "/wp-content/uploads/" does not require double quotes; use single quotes instead
138 | ERROR | [x] Whitespace found at end of line

@@ -131,7 +131,11 @@ public function save_symlinks() {
// successfully add
// fail error.
$value = $destination . ' -> ' . $source;

// Get the target folder name
preg_match("/^.\/uploads\/\W?\K.*/", $target, $matches);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String "/^./uploads/\W?\K.*/" does not require double quotes; use single quotes instead
Expected 1 spaces after opening bracket; 0 foundphpcs
Expected 1 spaces before closing bracket; 0 foundphpcs

// Get the target folder name
preg_match("/^.\/uploads\/\W?\K.*/", $target, $matches);
// Create target folder under uploads folder.
mkdir($_SERVER['DOCUMENT_ROOT'] . "/wp-content/uploads/" . $matches[0], 0777, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server variables should be avoided, I have a built-in function that get the doc root instead that you can use $this->get_wp_homepath();

For consistency, it might be better if the path is declared in the "link input" from the admin settings instead of hardcoding so it will match our docs

// Create target folder under uploads folder.
mkdir($_SERVER['DOCUMENT_ROOT'] . "/wp-content/uploads/" . $matches[0], 0777, true);

mkdir( $homepath . '/wp-content/uploads/' . $matches[0], 0777, true );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@umandalroald

This creates a folder
./uploads/cache if link is from /wp-content/cache

This seems to fail:
./wp-content/uploads/rootfolder if link is from /rootfolder

@carl-alberto
Copy link
Member

carl-alberto commented Dec 26, 2019

I'll be closing this PR @umandalroald in favor of the CI bot that is added. Please open up a new PR for #12 so the recently added CI bot for code checks will kick-off. All new PR's will now have code checks in place.

@umandalroald
Copy link
Member Author

Thanks, @carl-alberto opened a new PR for this #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants