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

Support hyphens in custom template names #731

Merged
merged 1 commit into from Sep 10, 2019

Conversation

@leofeyer
Copy link
Member

commented Sep 6, 2019

This PR fixes #725, however, since the underlying problem is caused by a wrong template naming scheme, I am not sure whether we should merge this.

@contao/developers WDYT?

@aschempp

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2019

Maybe we should allow any character after the base template name, not just hyphen and underscore.

@ausi

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

Maybe we should allow any character after the base template name, not just hyphen and underscore.

You mean something like this?

$strGlobPrefix = substr($strGlobPrefix, 0, -1) . '[!A-Za-z0-9]';
@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2019

No, I think @aschempp really means any character for reasons of backwards compatibility.

  • mod_article.html5
  • mod_article_custom.html5
  • mod_article-custom.html5
  • mod_articlecustom.html5
  • mod_article.custom.html5

Contao <4.8 will show all variations above in the drop-down menu I believe (did not yet test).

@fritzmg

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2019

And mod_article_list as well, which we wanted to avoid?

@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2019

Exactly. Therefore I think that [_-] is enough (until further notice).

@leofeyer leofeyer merged commit e3b1640 into 4.8 Sep 10, 2019

5 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 86.248%
Details

@leofeyer leofeyer deleted the bugfix/template-names branch Sep 10, 2019

leofeyer added a commit that referenced this pull request Sep 10, 2019
@aschempp

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

And mod_article_list as well, which we wanted to avoid?

that's not true. I would (have) include(d) anything after the template name. We can still skip anything that matches existing templates (and their derivates).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.