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

WICKET-7036: MarkupFactory supports IMarkupFilters #564

Open
wants to merge 2 commits into
base: wicket-9.x
Choose a base branch
from

Conversation

hosea
Copy link

@hosea hosea commented Mar 24, 2023

Wicket-7036: Support IMarkupFilters within MarkupFactory.

@@ -61,6 +64,8 @@ public static MarkupFactory get()
return Application.get().getMarkupSettings().getMarkupFactory();
}

private final LinkedHashMap<IMarkupFilter, Class<? extends IMarkupFilter>> additionalMarkupFilters = new LinkedHashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it would be better to change private final LinkedHashMap -> private final Map

@theigl
Copy link
Contributor

theigl commented Mar 27, 2023

Is this really necessary?

The Javadoc for MarkupFactory reads:

* MarkupParser's can be extended by means of {@link IMarkupFilter}. You can add your own filter
* as follows:
* 
* <pre>
*    public MyMarkupFactory {
*      ...
*      public MarkupParser newMarkupParser(final MarkupResourceStream resource) {
*         MarkupParser parser = super.newMarkupParser(resource);
*         parser.add(new MyFilter());
*         return parser;
*      }
*    }
* </pre>

@martin-g
Copy link
Member

I agree with @theigl that one could easily extend MarkupFactory and achieve the requirement.

@hosea
Copy link
Author

hosea commented Mar 27, 2023

Hi @theigl , @martin-g ,

I agree with you, that one could easily extend MarkupFactory and achieve the requirement. That's in fact, what we do now. I just want to explain, why it would be helpful to have such an extension:
I have about 20 Wicket-Applications developed by several teams sharing several libraries of wicket-components. These "central" libraries use IInitializers. So the code #Application.getMarkupSettings().setMarkupFactory(new AriaAwareMarkupFactory()) is now located in one of theselibraries. But if I want to add processing of MarkupFilters in another library, then I cannot use this initialization-code anymore: two Initializers setting a new MarkupFacatory => only one wins and the other functionality is lost. This means, that all 20 Applications have to implement a new MarkupFactory that implements both functionalities, or I have to add a dependency between libraries. Both solutions are not nice.
Adding to possibility to add IMarkupFilters without replacing the instance of the MarkupFactory makes initialization much easier in such a case.

@martin-g
Copy link
Member

Thanks for the explanation, @hosea !

I can see how the code in this PR would help for your case!
But no one ever asked for this so far and it looks like a rather unique case.

How about adding these new methods to AriaAwareMarkupFactory or to a new class, e.g. CustomMarkupFactory, as parent of AriaAwareMF, that is part of your wicket-components library ? Then each library and application can check what's the type of the current MarkupFactory and either just add filters or set a new MarkupFactory.

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.

4 participants