On 24/10/14 09:12, Martin Grigorov wrote:
On Thu, Oct 23, 2014 at 10:59 PM, <[email protected]> wrote:

Repository: wicket
Updated Branches:
   refs/heads/improvingQueuing 1f04e9e43 -> 1128d3c50


WICKET-5732 added IQueueRegion#getRegionMarkup to specify the markup of
queue region.

Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/1128d3c5
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/1128d3c5
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/1128d3c5

Branch: refs/heads/improvingQueuing
Commit: 1128d3c50804b8bb07f16882622f58558f4c8373
Parents: 1f04e9e
Author: Andrea Del Bene <[email protected]>
Authored: Thu Oct 23 21:57:59 2014 +0200
Committer: Andrea Del Bene <[email protected]>
Committed: Thu Oct 23 21:57:59 2014 +0200

----------------------------------------------------------------------
  .../java/org/apache/wicket/IQueueRegion.java    | 16 +++++++++
  .../java/org/apache/wicket/MarkupContainer.java |  8 ++++-
  .../apache/wicket/markup/html/MarkupUtil.java   | 37 ++++++++++++++++++++
  .../wicket/markup/html/border/Border.java       | 22 ++++++++++++
  .../panel/AssociatedMarkupSourcingStrategy.java | 36 ++-----------------
  .../apache/wicket/markup/html/panel/Panel.java  | 23 ++++++++++++
  6 files changed, 107 insertions(+), 35 deletions(-)
----------------------------------------------------------------------



http://git-wip-us.apache.org/repos/asf/wicket/blob/1128d3c5/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java
b/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java
index 93fa2fa..6127782 100644
--- a/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java
+++ b/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java
@@ -16,6 +16,8 @@
   */
  package org.apache.wicket;

+import org.apache.wicket.markup.IMarkupFragment;
+

  /**
   * Demarcates components that act as a root can dequeue children. These
are usually components with
@@ -31,6 +33,12 @@ package org.apache.wicket;
   */
  public interface IQueueRegion
  {
+       /**
+        * TODO: this interface might be a perfect candidate for Java 8
interface default methods.
+        *  Now methods implementation is in MarkupContainer while it
should simply be in those
+        *  class which implement this interface.

It is not clear to me:
1) when this TODO should be resolved/removed. I guess Wicket 8 (assuming it
requires Java 8). Then use "TODO Wicket 8:" as a prefix so it is easier
later to grep for it. We have used this approach for previous versions
Ok, that means that Wicket 8 will be based on Java 8? Great!
2) Where the impls should be ? How #getRegionMarkup() can be implemented
here as default method ?
Well yes, it would be nice to have. I'm aware that at the moment it's difficult to implement a default version for this method but maybe with a little bit of refactoring we could do this semplicemente. For example we could abstract some MarkupContainer methods (like getAssociatedMarkup) in a new interface and make IQueueRegion extend it.Just thinking out loud....

+        * */
+
         /**
          * Creates a new {@link DequeueContext} that will be used to
dequeue children of this region.
          *
@@ -48,4 +56,12 @@ public interface IQueueRegion
          * actual dequeueing. The context's markup is retrieved using the
{@link MarkupContainer#getAssociatedMarkup()}.
          */
         public void dequeue();
+
+       /**
+        * Returns the markup to use for queuing. Normally, this is the
markup of the component
+        * implementing this interface.
+        *
+        * @return the markup to use for queuing
+        */
+       public IMarkupFragment getRegionMarkup();
  }


http://git-wip-us.apache.org/repos/asf/wicket/blob/1128d3c5/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
index a2b94a8..6b0ddb1 100644
--- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
@@ -2112,7 +2112,7 @@ public abstract class MarkupContainer extends
Component implements Iterable<Comp
      /** @see IQueueRegion#newDequeueContext() */
         public DequeueContext newDequeueContext()
         {
-               IMarkupFragment markup = getAssociatedMarkup();
+               IMarkupFragment markup = getRegionMarkup();

                 if (markup == null)
                 {
@@ -2122,6 +2122,12 @@ public abstract class MarkupContainer extends
Component implements Iterable<Comp
                 return new DequeueContext(markup, this, false);
         }

+       /** @see IQueueRegion#getRegionMarkup() */

No need to use @see as javadoc. It is inherited by javadoc tool anyway.

Better use @Override annotation to make if safer for refactoring.
Actually MarkupContainer does not implements IQueueRegion so it doesn't override its method. That's why it would be nice to have (with Java 8) a "real" interface implemented only by those components that are queue regions.


+       public IMarkupFragment getRegionMarkup()
+       {
+               return getAssociatedMarkup();
+       }
+
         /**
          * Checks if this container can dequeue a child represented by the
specified tag. This method
          * should be overridden when containers can dequeue components
represented by non-standard tags.


http://git-wip-us.apache.org/repos/asf/wicket/blob/1128d3c5/wicket-core/src/main/java/org/apache/wicket/markup/html/MarkupUtil.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/MarkupUtil.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/MarkupUtil.java
index 3831ede..614bbab 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/MarkupUtil.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/MarkupUtil.java
@@ -19,8 +19,11 @@ package org.apache.wicket.markup.html;
  import org.apache.wicket.MarkupContainer;
  import org.apache.wicket.Page;
  import org.apache.wicket.WicketRuntimeException;
+import org.apache.wicket.markup.ComponentTag;
  import org.apache.wicket.markup.IMarkupFragment;
  import org.apache.wicket.markup.MarkupResourceStream;
+import org.apache.wicket.markup.MarkupStream;
+import org.apache.wicket.markup.WicketTag;
  import org.apache.wicket.util.lang.Args;
  import org.apache.wicket.util.visit.IVisit;
  import org.apache.wicket.util.visit.IVisitor;
@@ -71,4 +74,38 @@ public class MarkupUtil

                 return rtn[0];
         }
+
+       /**
+        * Searches for {@code tagName} in the given {@code markup}.
+        *
+        * @param markup
+        * @param tagName
+        * @return The {@link IMarkupFragment} corresponding to {@code
tagName}. Null, if such {@code tagName} is not found
+        */
+       public static IMarkupFragment findStartTag(final IMarkupFragment
markup, final String tagName)
+       {
+               MarkupStream stream = new MarkupStream(markup);
+
+               while (stream.skipUntil(ComponentTag.class))

Why not skipUntil(WicketTag.class) ?
The javadoc doesn't say anything about the requirement to be a WicketTag

ok, sounds correct.


+               {
+                       ComponentTag tag = stream.getTag();
+                       if (tag.isOpen() || tag.isOpenClose())
+                       {
+                               if (tag instanceof WicketTag)
+                               {
+                                       WicketTag wtag = (WicketTag)tag;
+                                       if
(tagName.equalsIgnoreCase(wtag.getName()))
+                                       {
+                                               return
stream.getMarkupFragment();
+                                       }
+                               }
+
+                               stream.skipToMatchingCloseTag(tag);
+                       }
+
+                       stream.next();
+               }
+
+               return null;
+       }
  }


http://git-wip-us.apache.org/repos/asf/wicket/blob/1128d3c5/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
index 934ab39..aba5b7f 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
@@ -29,6 +29,7 @@ import org.apache.wicket.markup.MarkupFragment;
  import org.apache.wicket.markup.MarkupStream;
  import org.apache.wicket.markup.TagUtils;
  import org.apache.wicket.markup.WicketTag;
+import org.apache.wicket.markup.html.MarkupUtil;
  import org.apache.wicket.markup.html.WebMarkupContainer;
  import org.apache.wicket.markup.html.panel.BorderMarkupSourcingStrategy;
  import org.apache.wicket.markup.html.panel.IMarkupSourcingStrategy;
@@ -608,4 +609,25 @@ public abstract class Border extends
WebMarkupContainer implements IComponentRes
                 // components queued in border get dequeued into the
border not into the body container
                 addToBorder(component);
         }
+
+       /**
+        * Returns the markup inside &lt;wicket:border&gt; tag.
+        * If such tag is not found, all the markup is returned.
+        *
+        * @see IQueueRegion#getRegionMarkup()
+        */
+       @Override
+       public IMarkupFragment getRegionMarkup()
+       {
+               IMarkupFragment markup = super.getRegionMarkup();
+
+               if (markup == null)
+               {
+                       return markup;
+               }
+
+               IMarkupFragment panelMarkup =
MarkupUtil.findStartTag(markup, BORDER);

copy/paste error ?
borderMarkup as a variable name sounds more correct
yep!

+
+               return panelMarkup != null ? panelMarkup : markup;

could panelMarkup be null ?! In what cases ?
It might be if markup doesn't contain wicket:panel/border tag. This is not a real use-case, but in some test cases we explicitly set panel/border markup with Component#setMarkup and we omit the relative wicket tag.

Reply via email to