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

Adding slashes at the end of plugin name on 'plugin-tests' subcomand generates corrupted bootstrap.php #133

Open
enrico-sorcinelli opened this issue Mar 27, 2018 · 4 comments

Comments

@enrico-sorcinelli
Copy link
Contributor

Adding one or more slashes at the end of plugin name like for example:

%> wp-cli scaffold plugin-tests my-plugin/

runs right, but causes the creation of a corrupted bootstrap.php file that produces a PHP Fatal error on require() that file when launching tests.
(The usual message is like following: "PHP Fatal error: require(): Failed opening required '/path/to/my-plugin/.php' (include_path='.') in /path/to/my-plugin/tests/bootstrap.php on line 26.")

I'd like to send a PR in order to fix this for example by right trimming slashes to $slug before to send to mustache template.

@enrico-sorcinelli enrico-sorcinelli changed the title Adding slashes at the end of plugin name on 'plugin-tests' subcomand generates corrupted boostrap.php Adding slashes at the end of plugin name on 'plugin-tests' subcomand generates corrupted bootstrap.php Mar 27, 2018
@enrico-sorcinelli
Copy link
Contributor Author

I noticed that in general the not strict syntax check of slug afflict almost all subcommands. For example:

  • wp scaffold plugin foo-plugin/
    fails on creating plugin file itself (Error: Error creating file: /path/to/wp-content/plugins/foo-plugin/foo-plugin/.php).

  • wp scaffold child-tests foo-theme/ or wp scaffold child-theme foo-theme/
    dont fails but uses Foo_Theme/ on annotations (@package Foo_Theme/) and on theme stylesheet (Theme Name: Foo-theme/).

  • wp scaffold taxonomy 'foo-ty pe'
    The scaffold subcommands post-type and taxonomy don't fail but generate bad code since they use slug for PHP function names.

  • wp scaffold block foo-block --plugin=foo-plugin/
    Both editor.css and style.css have .wp-block-foo-plugin/-foo-block selector.

Maybe another (and better) approach could be to check allowed slug characters like only _s or block do, so I'm planning to prepare a different PR instead of only rtrim slashes.

@danielbachhuber
Copy link
Member

I'd like to send a PR in order to fix this for example by right trimming slashes to $slug before to send to mustache template.

What do you think about hard-erroring instead? I'm not sure it makes sense to magically transform user input in this case; probably better to inform the user they've provided invalid input (as we've done in #26)

@enrico-sorcinelli
Copy link
Contributor Author

@danielbachhuber I agree. As I previously wrote, in fact, I worked also on another branch that issues a WP_CLI::errors when the user uses unallowed characters in slugs (or in values used for theme/plugin directories) in all cases I mentioned above.
So I'd like to send a PR very soon.

The only thing to do is to add (where missing) or fix correct regexes for each slug checking.
For example the regex for _s subcommand is '/^[a-z_]\w+$/i' that currently allows to type:

%> wp scaffold _s -_

returning a generic error: "Error: Couldn't create theme (received 500 response)."

In reality, the web interface, even if it returns 500, says also "Theme slug could not be used to generate valid function names. Please go back and try again.".
Also following command:

%> wp scaffold _s -foo

unzip _s theme into foo theme directory and also create -foo directory containing olny .editorconfig file. In this case we should to apply the same slug sanitization that undescore.me does or we do not allow the use of the starting dashes at all (why dashes have been replaced with undescores?)

I'm doing same kind of analysis for all others scaffold commands.

@danielbachhuber
Copy link
Member

So I'd like to send a PR very soon.

Sounds good! I'll defer to your judgement on what changes make sense.

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

No branches or pull requests

2 participants