You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "theigl (via GitHub)" <gi...@apache.org> on 2024/02/09 10:27:25 UTC
[PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
theigl opened a new pull request, #781:
URL: https://github.com/apache/wicket/pull/781
This PR introduces a strategy for outputting a container's class name for debugging purposes.
Up until 8bbf158ebc4d12f3b317c0d607200441c0767162, `ClassNameStrategy.HTML_COMMENT` was the default strategy. Since then `ClassNameStrategy.TAG_ATTRIBUTE` has become the default and only option.
For applications that strip Wicket tags during development, this means that there is currently no more debug output.
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "theigl (via GitHub)" <gi...@apache.org>.
theigl commented on code in PR #781:
URL: https://github.com/apache/wicket/pull/781#discussion_r1510879670
##########
wicket-jmx/src/main/java/org/apache/wicket/jmx/DebugSettingsMBean.java:
##########
@@ -71,6 +71,8 @@ public interface DebugSettingsMBean
* Enables wrapping output of markup container in html comments that contain markup container's
* class name. (Useful for determining which part of page belongs to which markup file).
*
+ * //TODO WICKET-6890 JMX Probably does not work with enum, What do we do here?
Review Comment:
I went the "enum name as a String" way.
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #781:
URL: https://github.com/apache/wicket/pull/781#issuecomment-1975949311
@theigl Do you need help here ?
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "theigl (via GitHub)" <gi...@apache.org>.
theigl commented on code in PR #781:
URL: https://github.com/apache/wicket/pull/781#discussion_r1484134849
##########
wicket-core/src/main/java/org/apache/wicket/Page.java:
##########
@@ -827,9 +827,10 @@ public void component(final Component component, final IVisit<Void> visit)
getSession().getPageManager().touchPage(this);
}
+ //TODO WICKET-6890 What about TAG_ATTRIBUTE?
if (getApplication().getDebugSettings().isOutputMarkupContainerClassName())
{
- String className = Classes.name(getClass());
+ final String className = Classes.name(getClass());
getResponse().write("<!-- Page Class ");
getResponse().write(className);
getResponse().write(" END -->\n");
Review Comment:
What should we do here? Ignore the strategy and always write an HTML comment, or only write it when the `HTML_COMMENT` strategy is enabled?
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "theigl (via GitHub)" <gi...@apache.org>.
theigl merged PR #781:
URL: https://github.com/apache/wicket/pull/781
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "theigl (via GitHub)" <gi...@apache.org>.
theigl commented on code in PR #781:
URL: https://github.com/apache/wicket/pull/781#discussion_r1484137664
##########
wicket-jmx/src/main/java/org/apache/wicket/jmx/DebugSettingsMBean.java:
##########
@@ -71,6 +71,8 @@ public interface DebugSettingsMBean
* Enables wrapping output of markup container in html comments that contain markup container's
* class name. (Useful for determining which part of page belongs to which markup file).
*
+ * //TODO WICKET-6890 JMX Probably does not work with enum, What do we do here?
Review Comment:
@martin-g: Any idea what we should do about the JMX bean? Looking at all available beans, enums have been carefully avoided.
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #781:
URL: https://github.com/apache/wicket/pull/781#discussion_r1484220117
##########
wicket-core/src/main/java/org/apache/wicket/Page.java:
##########
@@ -827,9 +827,10 @@ public void component(final Component component, final IVisit<Void> visit)
getSession().getPageManager().touchPage(this);
}
+ //TODO WICKET-6890 What about TAG_ATTRIBUTE?
Review Comment:
This is the `END`, so there is nothing to do for `TAG_ATTRIBUTE` and `NONE`
##########
wicket-core/src/main/java/org/apache/wicket/settings/DebugSettings.java:
##########
@@ -173,10 +185,24 @@ public DebugSettings setLinePreciseReportingOnNewComponentEnabled(boolean enable
*
* @param enable
* @return {@code this} object for chaining
+ * @deprecated use {@link #setOutputMarkupContainerClassNameStrategy(ClassOutputStrategy)} instead
*/
+ @Deprecated(forRemoval = true)
public DebugSettings setOutputMarkupContainerClassName(boolean enable)
{
- outputMarkupContainerClassName = enable;
+ outputMarkupContainerClassNameStrategy = enable ? ClassOutputStrategy.HTML_COMMENT : ClassOutputStrategy.NONE;
+ return this;
+ }
+
+ /**
+ * Sets the strategy for outputting the class of a markup
+ *
+ * @param stategy
+ * @return {@code this} object for chaining
+ */
+ public DebugSettings setOutputMarkupContainerClassNameStrategy(ClassOutputStrategy stategy)
Review Comment:
```suggestion
public DebugSettings setOutputMarkupContainerClassNameStrategy(ClassOutputStrategy strategy)
```
##########
wicket-jmx/src/main/java/org/apache/wicket/jmx/DebugSettingsMBean.java:
##########
@@ -71,6 +71,8 @@ public interface DebugSettingsMBean
* Enables wrapping output of markup container in html comments that contain markup container's
* class name. (Useful for determining which part of page belongs to which markup file).
*
+ * //TODO WICKET-6890 JMX Probably does not work with enum, What do we do here?
Review Comment:
One way is to use the enum name as a String.
Another way is to use MXBean instead of MBean.
##########
wicket-core/src/main/java/org/apache/wicket/Page.java:
##########
@@ -827,9 +827,10 @@ public void component(final Component component, final IVisit<Void> visit)
getSession().getPageManager().touchPage(this);
}
+ //TODO WICKET-6890 What about TAG_ATTRIBUTE?
if (getApplication().getDebugSettings().isOutputMarkupContainerClassName())
{
- String className = Classes.name(getClass());
+ final String className = Classes.name(getClass());
getResponse().write("<!-- Page Class ");
getResponse().write(className);
getResponse().write(" END -->\n");
Review Comment:
I see two options:
1) the HTML comment is after the closing `</html>` element, so it should not cause any problems related to CSS rules.
2) if we want to support the `TAG_ATTRIBUTE` then it should be applied on the opening element (`<html>`) but I have no idea how much effort this could be.
So, I think we could live with the current solution. i.e. if the strategy is not `NONE` then render HTML comment.
##########
wicket-core/src/main/java/org/apache/wicket/settings/DebugSettings.java:
##########
@@ -173,10 +185,24 @@ public DebugSettings setLinePreciseReportingOnNewComponentEnabled(boolean enable
*
* @param enable
* @return {@code this} object for chaining
+ * @deprecated use {@link #setOutputMarkupContainerClassNameStrategy(ClassOutputStrategy)} instead
*/
+ @Deprecated(forRemoval = true)
public DebugSettings setOutputMarkupContainerClassName(boolean enable)
{
- outputMarkupContainerClassName = enable;
+ outputMarkupContainerClassNameStrategy = enable ? ClassOutputStrategy.HTML_COMMENT : ClassOutputStrategy.NONE;
+ return this;
+ }
+
+ /**
+ * Sets the strategy for outputting the class of a markup
+ *
+ * @param stategy
+ * @return {@code this} object for chaining
+ */
+ public DebugSettings setOutputMarkupContainerClassNameStrategy(ClassOutputStrategy stategy)
+ {
+ outputMarkupContainerClassNameStrategy = stategy;
Review Comment:
```suggestion
outputMarkupContainerClassNameStrategy = Args.notNull(strategy, "strategy");
```
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "theigl (via GitHub)" <gi...@apache.org>.
theigl commented on PR #781:
URL: https://github.com/apache/wicket/pull/781#issuecomment-1976159472
> @theigl Do you need help here ?
Thanks a lot for asking @martin-g! I just found the time to continue working on this.
I improved the JavaDoc some more and added a method to set the strategy to the JMX bean.
--
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
Re: [PR] WICKET-6890 Introduce strategy for class name debug output [wicket]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #781:
URL: https://github.com/apache/wicket/pull/781#discussion_r1505527158
##########
wicket-core/src/main/java/org/apache/wicket/settings/DebugSettings.java:
##########
@@ -173,10 +187,24 @@ public DebugSettings setLinePreciseReportingOnNewComponentEnabled(boolean enable
*
* @param enable
* @return {@code this} object for chaining
+ * @deprecated use {@link #setOutputMarkupContainerClassNameStrategy(ClassOutputStrategy)} instead
*/
+ @Deprecated(forRemoval = true)
public DebugSettings setOutputMarkupContainerClassName(boolean enable)
{
- outputMarkupContainerClassName = enable;
+ outputMarkupContainerClassNameStrategy = enable ? ClassOutputStrategy.HTML_COMMENT : ClassOutputStrategy.NONE;
+ return this;
+ }
+
+ /**
+ * Sets the strategy for outputting the class of a markup
Review Comment:
```suggestion
* Sets the strategy for outputting the Java class name of a markup
```
--
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