You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "hosea (via GitHub)" <gi...@apache.org> on 2023/03/24 16:13:48 UTC
[GitHub] [wicket] hosea opened a new pull request, #564: WICKET-7036: MarkupFactory supports IMarkupFilters
hosea opened a new pull request, #564:
URL: https://github.com/apache/wicket/pull/564
Wicket-7036: Support IMarkupFilters within MarkupFactory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] theigl commented on pull request #564: WICKET-7036: MarkupFactory supports IMarkupFilters
Posted by "theigl (via GitHub)" <gi...@apache.org>.
theigl commented on PR #564:
URL: https://github.com/apache/wicket/pull/564#issuecomment-1484847931
Is this really necessary?
The Javadoc for `MarkupFactory` reads:
```java
* 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>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #564: WICKET-7036: MarkupFactory supports IMarkupFilters
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #564:
URL: https://github.com/apache/wicket/pull/564#issuecomment-1484927039
I agree with @theigl that one could easily extend MarkupFactory and achieve the requirement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #564: WICKET-7036: MarkupFactory supports IMarkupFilters
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on code in PR #564:
URL: https://github.com/apache/wicket/pull/564#discussion_r1148280573
##########
wicket-core/src/main/java/org/apache/wicket/markup/MarkupFactory.java:
##########
@@ -61,6 +64,8 @@ public static MarkupFactory get()
return Application.get().getMarkupSettings().getMarkupFactory();
}
+ private final LinkedHashMap<IMarkupFilter, Class<? extends IMarkupFilter>> additionalMarkupFilters = new LinkedHashMap<>();
Review Comment:
IMO it would be better to change `private final LinkedHashMap` -> `private final Map`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] hosea commented on pull request #564: WICKET-7036: MarkupFactory supports IMarkupFilters
Posted by "hosea (via GitHub)" <gi...@apache.org>.
hosea commented on PR #564:
URL: https://github.com/apache/wicket/pull/564#issuecomment-1485306655
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 extensions:
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 the libraries. 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #564: WICKET-7036: MarkupFactory supports IMarkupFilters
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #564:
URL: https://github.com/apache/wicket/pull/564#issuecomment-1486258481
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org