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

make tags and filters private for templateSet #335

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

honmaple
Copy link

TemplateSet add tags and filters variable, and remove global variable, RegisterTag and RegisterFilter will change DefaultSet.tags and DefaultSet.filters.
After modification, different function but same name can be added for different templateSet

@@ -68,6 +83,8 @@ func NewSet(name string, loaders ...TemplateLoader) *TemplateSet {
name: name,
loaders: loaders,
Globals: make(Context),
tags: make(map[string]*tag),
filters: make(map[string]FilterFunction),
Copy link

Choose a reason for hiding this comment

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

Gave this a spin - having empty filters leads to panic when redering variables with escaping, since escape filter is not present. Think NewDefaultSet will have to include at least that one!

Copy link
Author

@honmaple honmaple Oct 20, 2023

Choose a reason for hiding this comment

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

@draganm It might be a good idea to use newSet instead of NewDefaultSet. Users can only use NewSet which includes all default filters and tags to create a special template set. What do you think of this idea?

Copy link

Choose a reason for hiding this comment

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

Yeah, that works, but as I said, NewDefaultSet has this huge dependency list of things that need to be provided, that it makes is quite niche. Upon reflection, probably just a word of warning in the GoDoc would be probably enough to point people to NewSet

@iamcalledrob
Copy link

This behaviour would be super useful for a project I'm working on that uses pongo2 internally. I think it's a better, safer design for the package. Ideally there would be no global variables, except for convenience (e.g. defaults)

Currently due to the global nature of registering tags/filters, a user of my package may end up introducing regressions if they themselves use pongo2 as well.

@draganm
Copy link

draganm commented Oct 25, 2023

I agree about usefulness of this feature - @flosch is there a chance that we can get this merged any time soon? Think at least 3 people are itching to be using it!

user can only use NewSet with default filters and tags to create special template_set
@sonarcloud
Copy link

sonarcloud bot commented Oct 30, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@iamcalledrob
Copy link

@flosch I'd love to see this functionality get merged, is there anything we can do to help?

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.

None yet

3 participants