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

Request for prependTemplateDir() or more options with addTemplateDir() - weight, duplicate handling #1022

Closed
eileenmcnaughton opened this issue May 29, 2024 · 9 comments · Fixed by #1025

Comments

@eileenmcnaughton
Copy link

We have a code base (CiviCRM) that uses Smarty & can be extended by extensions.

Our main code base registers the template directory but the plugins can also register their own. However, in many cases the plugins want their directories to take precedence.

This is a feature request to be able to call addTemplateDir() with a weight or similar or prependTemplateDir().

A secondary request is that this would handle duplicates. We have found that calling getTemplateDirs() is expensive if done between calls to addTemplateDir() and often the extensions are unaware of each other.

@wisskid
Copy link
Contributor

wisskid commented May 29, 2024

Could you elaborate on that second request? I'm afraid I don't understand what you mean.

@wisskid
Copy link
Contributor

wisskid commented May 29, 2024

And as for your first request, would you need something like this?

	public function prependTemplateDir($new_template_dir, $is_config = false): void {
		$current_template_dirs = $this->getTemplateDir(null, $is_config);
		array_unshift($current_template_dirs, $new_template_dir);
		$this->setTemplateDir($current_template_dirs, $is_config);
	}

@totten
Copy link

totten commented May 29, 2024

@wisskid Yeah, that's the basic idea. Part of the reason for filing this is... that we recently found that's non-performant to repeatedly combine getTemplateDir() and setTemplateDir(). The going theory: each call to getTemplateDir() causes it to re-run normalization over all the paths.

For a small number of paths, that's fine. But it turns out - whenever one extension adds a path, it (re)normalizes n-many paths for all prior extensions. So it's basically O(n^2). Or here's the longer rationalization of that theory:

Screenshot from 2024-05-29 00-45-38

With a handful of extensions, no one notices. But for folks with a large number of extensions, it balloons.

So prependTemplateDirs() would probably want to bypass the (re)normalization that's built into getTemplateDir().

@eileenmcnaughton
Copy link
Author

thanks for the quick reply - I was writing a response but seems Tim did first so I'll go with 'what he said' on the second.

On the first, I might be wrong on the second request but it's my understanding that if I have 2 bits of code that don't know about each other & each do

 $smarty->addTemplateDir('\srv\var\template');

Then the template will potentially be added and / or processed through _realPath() twice. I kinda get that it needs to be for symlinks but maybe there is a cheap check that can be done before going through all the expensive stuff

@wisskid
Copy link
Contributor

wisskid commented May 29, 2024

I see. In that case, we can change it as follows:

	public function prependTemplateDir($new_template_dir, $is_config = false): void {
		$current_template_dirs = $is_config ? $this->config_dir : $this->template_dir;
		array_unshift($current_template_dirs, $new_template_dir);
		$this->setTemplateDir($current_template_dirs, $is_config);
	}

That would postpone the expensive call to _normalizeTemplateConfig up until the first load of a template. Correct?

@eileenmcnaughton
Copy link
Author

Yes that seems correct to me

@wisskid wisskid changed the title Request for more options with addTemplateDir() - weight, duplicate handling Request for prependTemplateDir() or more options with addTemplateDir() - weight, duplicate handling May 29, 2024
@wisskid
Copy link
Contributor

wisskid commented May 29, 2024

I'll try to squeeze it in asap

@eileenmcnaughton
Copy link
Author

thank you - you have been incredibly responsive!

@wisskid
Copy link
Contributor

wisskid commented May 29, 2024

@eileenmcnaughton you're welcome!

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

Successfully merging a pull request may close this issue.

3 participants