[GitHub] [wicket] martin-g commented on a change in pull request #437: WICKET-6795 Avoid splitting and joining ajax event names

2020-06-01 Thread GitBox 


martin-g commented on a change in pull request #437:
URL: https://github.com/apache/wicket/pull/437#discussion_r433322930



##
File path: 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
##
@@ -119,11 +119,16 @@ protected void updateAjaxAttributes(AjaxRequestAttributes 
attributes)
 */
public String getEvent()
{
+   if (event.indexOf(' ') == -1)

Review comment:
   https://issues.apache.org/jira/browse/WICKET-6797





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [wicket] martin-g commented on a change in pull request #437: WICKET-6795 Avoid splitting and joining ajax event names

2020-06-01 Thread GitBox 


martin-g commented on a change in pull request #437:
URL: https://github.com/apache/wicket/pull/437#discussion_r433162126



##
File path: 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
##
@@ -119,11 +119,16 @@ protected void updateAjaxAttributes(AjaxRequestAttributes 
attributes)
 */
public String getEvent()
{
+   if (event.indexOf(' ') == -1)

Review comment:
   I don't mind to use `' '` in both places for 9.x.
   I also always thought that the separator is just space. 
   If someone complains and gives us a good reason then we can reconsider.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [wicket] martin-g commented on a change in pull request #437: WICKET-6795 Avoid splitting and joining ajax event names

2020-06-01 Thread GitBox 


martin-g commented on a change in pull request #437:
URL: https://github.com/apache/wicket/pull/437#discussion_r433122027



##
File path: 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
##
@@ -119,11 +119,16 @@ protected void updateAjaxAttributes(AjaxRequestAttributes 
attributes)
 */
public String getEvent()
{
+   if (event.indexOf(' ') == -1)

Review comment:
   We should either use `' '` as a split char everywhere or `\s+`. 
   At the moment it is inconsistent and IMO not very good.
   
   What about:
   ```diff
   diff --git 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   index a03f8d2a42..de6ed89a56 100644
   --- wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   +++ wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   @@ -18,6 +18,8 @@ package org.apache.wicket.ajax;

import java.util.ArrayList;
import java.util.List;
   +import java.util.regex.Matcher;
   +import java.util.regex.Pattern;

import org.apache.wicket.Component;
import org.apache.wicket.ajax.attributes.AjaxRequestAttributes;
   @@ -68,6 +70,11 @@ public abstract class AjaxEventBehavior extends 
AbstractDefaultAjaxBehavior

   private static final long serialVersionUID = 1L;

   +   private static final Pattern EVENT_NAME_SPLITTER = 
Pattern.compile("\\s+");
   +
   private final String event;

   /**
   @@ -119,12 +126,13 @@ public abstract class AjaxEventBehavior extends 
AbstractDefaultAjaxBehavior
*/
   public String getEvent()
   {
   -   if (event.indexOf(' ') == -1)
   +   final Matcher matcher = EVENT_NAME_SPLITTER.matcher(event);
   +   if (!matcher.matches())
   {
   return event;
   }

   -   String[] splitEvents = event.split("\\s+");
   +   String[] splitEvents = EVENT_NAME_SPLITTER.split(event);
   List cleanedEvents = new 
ArrayList<>(splitEvents.length);
   for (String evt : splitEvents)
   {
   ``` 
   ?
   
   How much slower is it than using `' '` ?
   





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [wicket] martin-g commented on a change in pull request #437: WICKET-6795 Avoid splitting and joining ajax event names

2020-06-01 Thread GitBox 


martin-g commented on a change in pull request #437:
URL: https://github.com/apache/wicket/pull/437#discussion_r433122027



##
File path: 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
##
@@ -119,11 +119,16 @@ protected void updateAjaxAttributes(AjaxRequestAttributes 
attributes)
 */
public String getEvent()
{
+   if (event.indexOf(' ') == -1)

Review comment:
   We should either use `' '` as a split char everywhere or `\s+`. 
   At the moment it is inconsistent and IMO not very good.
   
   What about:
   ```
   diff --git 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   index a03f8d2a42..de6ed89a56 100644
   --- wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   +++ wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   @@ -18,6 +18,8 @@ package org.apache.wicket.ajax;

import java.util.ArrayList;
import java.util.List;
   +import java.util.regex.Matcher;
   +import java.util.regex.Pattern;

import org.apache.wicket.Component;
import org.apache.wicket.ajax.attributes.AjaxRequestAttributes;
   @@ -68,6 +70,11 @@ public abstract class AjaxEventBehavior extends 
AbstractDefaultAjaxBehavior

   private static final long serialVersionUID = 1L;

   +   /**
   +* Splits event
   +*/
   +   private static final Pattern EVENT_NAME_SPLITTER = 
Pattern.compile("\\s+");
   +
   private final String event;

   /**
   @@ -119,12 +126,13 @@ public abstract class AjaxEventBehavior extends 
AbstractDefaultAjaxBehavior
*/
   public String getEvent()
   {
   -   if (event.indexOf(' ') == -1)
   +   final Matcher matcher = EVENT_NAME_SPLITTER.matcher(event);
   +   if (!matcher.matches())
   {
   return event;
   }

   -   String[] splitEvents = event.split("\\s+");
   +   String[] splitEvents = EVENT_NAME_SPLITTER.split(event);
   List cleanedEvents = new 
ArrayList<>(splitEvents.length);
   for (String evt : splitEvents)
   {
   ``` 
   ?
   
   How much slower is it than using `' '` ?
   





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [wicket] martin-g commented on a change in pull request #437: WICKET-6795 Avoid splitting and joining ajax event names

2020-06-01 Thread GitBox 


martin-g commented on a change in pull request #437:
URL: https://github.com/apache/wicket/pull/437#discussion_r433122027



##
File path: 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
##
@@ -119,11 +119,16 @@ protected void updateAjaxAttributes(AjaxRequestAttributes 
attributes)
 */
public String getEvent()
{
+   if (event.indexOf(' ') == -1)

Review comment:
   We should either use `' '` as a split char everywhere or `\s+`. 
   At the moment it is inconsistent and IMO not very good.
   
   What about:
   ```diff
   diff --git 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java 
wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   index a03f8d2a42..de6ed89a56 100644
   --- wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   +++ wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java
   @@ -18,6 +18,8 @@ package org.apache.wicket.ajax;

import java.util.ArrayList;
import java.util.List;
   +import java.util.regex.Matcher;
   +import java.util.regex.Pattern;

import org.apache.wicket.Component;
import org.apache.wicket.ajax.attributes.AjaxRequestAttributes;
   @@ -68,6 +70,11 @@ public abstract class AjaxEventBehavior extends 
AbstractDefaultAjaxBehavior

   private static final long serialVersionUID = 1L;

   +   /**
   +* Splits event
   +*/
   +   private static final Pattern EVENT_NAME_SPLITTER = 
Pattern.compile("\\s+");
   +
   private final String event;

   /**
   @@ -119,12 +126,13 @@ public abstract class AjaxEventBehavior extends 
AbstractDefaultAjaxBehavior
*/
   public String getEvent()
   {
   -   if (event.indexOf(' ') == -1)
   +   final Matcher matcher = EVENT_NAME_SPLITTER.matcher(event);
   +   if (!matcher.matches())
   {
   return event;
   }

   -   String[] splitEvents = event.split("\\s+");
   +   String[] splitEvents = EVENT_NAME_SPLITTER.split(event);
   List cleanedEvents = new 
ArrayList<>(splitEvents.length);
   for (String evt : splitEvents)
   {
   ``` 
   ?
   
   How much slower is it than using `' '` ?
   





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org