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

feat: added the ability to provide another Compiler #82

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

Conversation

PhilipGarnero
Copy link

Hi !

I've simply moved _Compiler to another module and renamed it to Compiler as it's now public.
I've also added a compiler parameter to XMLTemplate that default to Compiler.

The behavior is unchanged by default but now, devs can inherit from Compiler to add features and new nodes.

I'm already using this fork in production so i'll appreciate if that's merged soon.

Thanks !

@amol-
Copy link
Collaborator

amol- commented Mar 20, 2024

Personally I think it would be helpful to understand the use case. I'm not arguing against the PR, but there might be a better API to achieve the needs that we are trying to solve compared to making public a previously internal class (as internal classes aren't exactly designed with the idea of being used or subclasses usually)

@PhilipGarnero
Copy link
Author

The usecase is pretty simple : I want to be able to extend kajiki's core functionalities.

In our case, we've developed a library of components that we want to be able to use like html tags.
The components are pydantic models (to validate their inputs) that point to a kajiki fragment.
We are calling them like so directly inside our templates :

<MyComponent attr="cool" />
<MyComponent>This is some content</MyComponent>

This will call the MyComponent model with the correct argument which will do some logic and then we render it using its fragment.

With this extensibility feature, we can add pretty much what we want to kajiki without polluting the library with our personal uses.

Of course, things we'd think would be useful to the community would come as a PR 😉

@jackrosenthal
Copy link
Owner

I will try and complete this code review by the end of the week, I apologize for the delay.

@amol-
Copy link
Collaborator

amol- commented Apr 3, 2024

I think we should design a public API for extending the Compiler, allow adding custom tags by registering a function that handles them. Making an internal implementation class that wasn't designed to be public doesn't seem the best course of action. Also in general subclassing isn't a very flexible extension mechanism (imagine you have multiple custom tags coming from different Python packages, you wouldn't be able to easily add them all to the Compiler)

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.

3 participants