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

Ensure templates filenames without path and extension. #1941

Merged
merged 1 commit into from
Aug 17, 2017

Conversation

kaisermann
Copy link
Contributor

@kaisermann kaisermann commented Aug 8, 2017

I was reading the solution (#1939) to fix #1936 and came up with an alternative fix by changing the

    return collect($templates)
        ->map(function ($template) {
            return preg_replace('#\.(blade\.)?php$#', '', ltrim($template));
        })
...

with

    return collect($templates)
        ->map(function ($template) {
            return basename(basename(ltrim($template), '.php'), '.blade');
        })
...

@kaisermann kaisermann changed the title Ensure to get templates filenames without path and extension. Ensure templates filenames without path and extension. Aug 8, 2017
@QWp6t
Copy link
Sponsor Member

QWp6t commented Aug 8, 2017

I think that might screw up custom page templates, and it can mess with any other hooks into template hierarchy that looks for the template in a subfolder, like certain plugins.

#1936 happens because WordPress uses get_page_template_slug() to get the path to the template file. It's mostly a cosmetic issue, but it's one that we can easily solve.

The solution presented here can introduce more bugs so I'm hesitant to accept it.

@kaisermann
Copy link
Contributor Author

Hm, good point! I'm gonna test it with subfolders and some plugins that offer template files and write the feedback here.

@kaisermann
Copy link
Contributor Author

kaisermann commented Aug 9, 2017

Another approach, without the basename:

  1. Modified regex '#\.(blade\.)?php$#' with '#\.(blade\.?)?(php)?$#' to match template-custom.blade (which comes without the .php).

  2. Moved $paths out of the flatMap callback

  3. Made the flatMap check if a template name starts with each $path and remove it from the the string.

function filter_templates($templates)
{
    $paths = apply_filters('sage/filter_templates/paths', [
        'views',
        'resources/views'
    ]);

    return collect($templates)
        ->map(function ($template) {
            return preg_replace('#\.(blade\.?)?(php)?$#', '', ltrim($template));
        })
        ->flatMap(function ($template) use ($paths) {
            return collect($paths)
                ->flatMap(function ($path) use ($template) {
                    if (strpos($template, '/')) {
                        $template = preg_replace("#^$path/#", '', $template);
                    }
                    return [
                        "{$path}/{$template}.blade.php",
                        "{$path}/{$template}.php",
                        "{$template}.blade.php",
                        "{$template}.php",
                    ];
                });
        })
        ->filter()
        ->unique()
        ->all();
}

@9585999
Copy link
Contributor

9585999 commented Aug 9, 2017

@kaisermann Nice 👍

with this soberwp/controller#39 It works as it should.

img 2017-08-09 19 04 17

@kaisermann
Copy link
Contributor Author

kaisermann commented Aug 9, 2017

Being a little bit picky with the returned paths:

The previous code would still return some entries with crazy file paths.

function filter_templates($templates)
{
    $paths = apply_filters('sage/filter_templates/paths', [
        'views',
        'resources/views'
    ]);
    $paths_pattern = "#^(" . implode('|', $paths) . ")/#";

    return collect($templates)
        ->map(function ($template) use ($paths_pattern) {
            /** Remove .blade.php/.blade/.php from template names */
            $template = preg_replace('#\.(blade\.?)?(php)?$#', '', ltrim($template));

            /** Remove partial $paths from the beginning of template names */
            if (strpos($template, '/')) {
                $template = preg_replace($paths_pattern, '', $template);
            }

            return $template;
        })
        ->flatMap(function ($template) use ($paths) {
            return collect($paths)
                ->flatMap(function ($path) use ($template) {
                    return [
                        "{$path}/{$template}.blade.php",
                        "{$path}/{$template}.php",
                        "{$template}.blade.php",
                        "{$template}.php",
                    ];
                });
        })
        ->unique()
        ->filter()
        ->all();
}

Note the $paths_pattern.

@retlehs
Copy link
Sponsor Member

retlehs commented Aug 17, 2017

@kaisermann thanks! could you please push that code to this branch? then we'll get this merged in 👍

@kaisermann
Copy link
Contributor Author

@retlehs Done 👍 !!

@retlehs retlehs merged commit 499f809 into roots:master Aug 17, 2017
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.

Custom Templates Hierarchy
4 participants