Skip to content

Render helper render_template, ensure_dir function bug #283

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

Closed
wants to merge 3 commits into from
Closed

Render helper render_template, ensure_dir function bug #283

wants to merge 3 commits into from

Conversation

arun744-newscorp
Copy link
Contributor

In render_helper.php file in function error was identified: Ouch!! It seems that the /var/www/wp-content/themes/<>/tmp

ensure_dir function sometimes cannot override the tmp folder permission to writeable, some server forbid the chmod action. Hence we compile our files via CLI during CI build process, this generates files in tmp DIR. Now I have corrected version of ensure_dir function test in our environment to check both compiled files in tmp and also create tmp if didn't exist this applies for both scenario.

Secondly we have filter 'wordless_environment' to override the environment.

Copy link
Member

@alessandro-fazzi alessandro-fazzi left a comment

Choose a reason for hiding this comment

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

Left my thoughts in-code :)

@alessandro-fazzi
Copy link
Member

I REALLY appreciated the documentation :) Thanks.

I dropped you my very last request: since the return of ensure_tmp_dir() is now, potentially, controlled by the implementer, it would be really better to drop meaningful error if he's returning wrong values from the filter.
I don't want to rely, in core logic, on falsyness or truthyness.

Once done, I'm ready to merge this one w/o any further review thus w/o any further waiting :)

Thanks for your work.

@@ -246,21 +248,30 @@ function wl_yield() {
render_template($current_view, $current_locals);
}

private function ensure_dir($dir) {
private function ensure_tmp_dir() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private function ensure_tmp_dir() {
private function ensure_tmp_dir(): bool {

return apply_filters('wordless_tmp_dir_exists', $tmpDir, $tmp_dir_exists_and_writable );
}

private function ensure_dir( $dir ) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private function ensure_dir( $dir ) {
private function ensure_dir( string $dir ): bool {

alessandro-fazzi added a commit that referenced this pull request Apr 14, 2023
Fix #284 

Previously proposed by @arun744-newscorp, but never brought to the end.
I've just added the needed type declarations to the brought-back
original #283

:)
This pull request was closed.
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.

Error in render_helper.php file occurring in render_template func via ensure_dir func
2 participants