You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by et...@apache.org on 2008/02/01 12:05:05 UTC

svn commit: r617472 - in /incubator/shindig/trunk: features/core-features.txt features/core/util.js features/views/feature.xml features/views/views.js java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java

Author: etnu
Date: Fri Feb  1 03:05:01 2008
New Revision: 617472

URL: http://svn.apache.org/viewvc?rev=617472&view=rev
Log:
Fixed features/views to actually use the current view data. requestNavigateTo is currently a NOOP. It should just use gadgets.rpc, but technically this feature should be core-views -- in other words, it should not be an explicit feature according to the gadgets API spec. I have petitioned for a spec change (the "views" javascript should not be a part of core), but if that doesn't happen then this will need to be moved to core, which means that IFPC / gadgets.rpc will always be delivered to every gadget. This is extraordinarily wasteful. One possible ugly solution would be to only serve views when we see an actual reference to "gadgets.views" in the content section, but this is hacky, and really there's no reason for views to be in the core spec anyway.

Takes data in the following form:

views.getParams() -- Assumes that any parameter named "view-<name>" is a key that belongs in this. When Cajoled this won't make sense, so we should probably add a check for whether or not we're rendering in an iframe or not first.

views.getCurrentView() -- Assumes that the current view is named in the "view" query string parameter, which is what GadgetRenderingServlet uses to discriminate between views. 

May be initialized by calling gadgets.views.init and passing in all of the supported views for this container and whether or not they're the "only visible" gadget at render time.

Also fixed a possible NPE with GadgetSpecParser and removed a dead file.


Removed:
    incubator/shindig/trunk/features/core-features.txt
Modified:
    incubator/shindig/trunk/features/core/util.js
    incubator/shindig/trunk/features/views/feature.xml
    incubator/shindig/trunk/features/views/views.js
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java

Modified: incubator/shindig/trunk/features/core/util.js
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/core/util.js?rev=617472&r1=617471&r2=617472&view=diff
==============================================================================
--- incubator/shindig/trunk/features/core/util.js (original)
+++ incubator/shindig/trunk/features/core/util.js Fri Feb  1 03:05:01 2008
@@ -114,6 +114,23 @@
     },
 
     /**
+     * Utility function for generating an "enum" from an array.
+     *
+     * @param {Array.<String>} values The values to generate.
+     * @return {Map&lt;String,String&gt;} An object with member fields to handle
+     *   the enum.
+     *
+     * @private Implementation detail.
+     */
+    makeEnum : function (values) {
+      var obj = {};
+      for (var i = 0, v; v = values[i]; ++i) {
+        obj[v] = v;
+      }
+      return obj;
+    },
+
+    /**
      * Gets the feature parameters.
      *
      * @param {String} feature The feature to get parameters for

Modified: incubator/shindig/trunk/features/views/feature.xml
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/views/feature.xml?rev=617472&r1=617471&r2=617472&view=diff
==============================================================================
--- incubator/shindig/trunk/features/views/feature.xml (original)
+++ incubator/shindig/trunk/features/views/feature.xml Fri Feb  1 03:05:01 2008
@@ -18,7 +18,12 @@
 -->
 <feature>
   <name>views</name>
+  <!-- <dependency>ifpc</dependency> -->
   <gadget>
     <script src="views.js"/>
+    <!-- TODO: Clean this up and unify the configuration model -->
+    <script>
+      gadgets.views.init({"default" : false});
+    </script>
   </gadget>
 </feature>

Modified: incubator/shindig/trunk/features/views/views.js
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/views/views.js?rev=617472&r1=617471&r2=617472&view=diff
==============================================================================
--- incubator/shindig/trunk/features/views/views.js (original)
+++ incubator/shindig/trunk/features/views/views.js Fri Feb  1 03:05:01 2008
@@ -24,126 +24,86 @@
 var gadgets = gadgets || {};
 
 /**
- * @static
- * @class Provides operations for dealing with Views.
- * @name gadgets.views
+ * Implements the gadgets.views API spec. See
+ * http://code.google.com/apis/gadgets/docs/reference/gadgets.views.html
  */
-gadgets.views = gadgets.views || {};
+gadgets.views = function() {
 
-/**
- * Attempts to navigate to this gadget in a different view. If the container
- * supports parameters will pass the optional parameters along to the gadget in
- * the new view.
- *
- * @param {gadgets.views.View} view The view to navigate to
- * @param {Map.&lt;String, String&gt;} opt_params Parameters to pass to the
- *     gadget after it has been navigated to on the surface
- *
- * @member gadgets.views
- */
-gadgets.views.requestNavigateTo = function(surface, opt_params) {
-  return opensocial.Container.get().requestNavigateTo(surface, opt_params);
-};
-
-/**
- * Returns the current view.
- *
- * @return {gadgets.views.View} The current view
- * @member gadgets.views
- */
-gadgets.views.getCurrentView = function() {
-  return this.surface;
+  /**
+   * Reference to the current view object.
+   */
+  var currentView = null;
+
+  /**
+   * Map of all supported views for this container.
+   */
+  var supportedViews = {};
+
+  /**
+   * Map of parameters passed to the current request.
+   */
+  var params = {};
+
+  return {
+    requestNavigateTo : function(view, opt_params) {
+      // TODO: Actually implementing this is going to require gadgets.rpc or
+      // ifpc or something.
+    },
+
+    getCurrentView : function() {
+      return currentView;
+    },
+
+    getSupportedViews : function() {
+      return supportedViews;
+    },
+
+    getParams : function() {
+      return params;
+    },
+
+    /**
+     * Initializes views. Assumes that the current view is the "view"
+     * url parameter (or default if "view" isn't supported), and that
+     * all view parameters are in the form view-<name>
+     * TODO: Use unified configuration when it becomes available.
+     * @param {Map&lt;String, Boolean&gt;} supported The views you support,
+     *   where keys = name of the view and values = isOnlyVisible
+     */
+    init : function(supported) {
+      if (typeof supported["default"] === "undefined") {
+        throw new Error("default view required!");
+      }
+
+      for (var s in supported) if (supported.hasOwnProperty(s)) {
+        supportedViews[s] = new gadgets.views.View(s, supported[s]);
+      }
+
+      var urlParams = gadgets.util.getUrlParameters();
+      // extract all parameters prefixed with "view-".
+      for (var p in urlParams) if (urlParams.hasOwnProperty(p)) {
+        if (p.substring(0, 5) === "view-") {
+          params[p.substr(5)] = urlParams[p];
+        }
+      }
+      currentView = supportedViews[urlParams.view] || supportedViews["default"];
+    }
+  };
+}();
+
+gadgets.views.View = function(name, opt_isOnlyVisible) {
+  this.name_ = name;
+  this.isOnlyVisible_ = !!opt_isOnlyVisible;
 };
 
-/**
- * Returns a map of all the supported views. Keys each gadgets.view.View by
- * its name.
- *
- * @return {Map&lt;gadgets.views.ViewType | String, gadgets.views.View&gt;} All
- *   supported views, keyed by their name attribute.
- * @member gadgets.views
- */
-gadgets.views.getSupportedViews = function() {
-  return this.supportedSurfaces;
-};
-
-/**
- * Returns the parameters passed into this gadget for this view. Does not
- * include all url parameters, only the ones passed into
- * gadgets.views.requestNavigateTo
- *
- * @return {Map.&lt;String, String&gt;} The parameter map
- * @member gadgets.views
- */
-gadgets.views.getParams = function() {
-  return this.params;
-};
-
-
-/**
- * @class Base interface for all view objects.
- * @name gadgets.views.View
- */
-
-/**
- * @private
- * @constructor
- */
-gadgets.views.View = function(name, opt_isOnlyVisibleGadgetValue) {
-  this.name = name;
-  this.isOnlyVisibleGadgetValue = !!opt_isOnlyVisibleGadgetValue;
-};
-
-/**
- * Returns the name of this view.
- *
- * @return {gadgets.views.ViewType | String} The view name, usually specified as
- * a gadgets.views.ViewType
- */
 gadgets.views.View.prototype.getName = function() {
-  return this.name;
+  return this.name_;
 };
 
-/**
- * Returns true if the gadget is the only visible gadget in this view.
- * On a canvas page or in maximize mode this is most likely true; on a profile
- * page or in dashboard mode, it is most likely false.
- *
- * @return {boolean} True if the gadget is the only visible gadget; otherwise, false
- */
 gadgets.views.View.prototype.isOnlyVisibleGadget = function() {
-  return this.isOnlyVisibleGadgetValue;
+  return this.isOnlyVisible_;
 };
 
-
-/**
- * @static
- * @class
- * Used by <a href="gadgets.views.View.html"> View</a>s.
- * @name gadgets.views.ViewType
- */
-gadgets.views.ViewType = {
- /**
-  * A view where the gadget is displayed in a very large mode. It should be the
-  * only thing on the page. In a social context, this is usually called the
-  * canvas page.
-  *
-  * @member gadgets.views.ViewType
-  */
-  FULL_PAGE : 'FULL_PAGE',
-
- /**
-  * A view where the gadget is displayed in a small area usually on a page with
-  * other gadgets. In a social context, this is usually called the profile page.
-  *
-  * @member gadgets.views.ViewType
-  */
-  DASHBOARD : 'DASHBOARD',
-
- /**
-  * A view where the gadget is displayed in a small separate window by itself.
-  *
-  * @member gadgets.views.ViewType
-  */
-  POPUP : 'POPUP'
-};
+gadgets.views.ViewType = gadgets.util.makeEnum([
+  "FULL_PAGE", "DASHBOARD", "POPUP"
+]);
\ No newline at end of file

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java?rev=617472&r1=617471&r2=617472&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java Fri Feb  1 03:05:01 2008
@@ -160,7 +160,7 @@
     Node messagesAttr = attrs.getNamedItem("messages");
     Node languageAttr = attrs.getNamedItem("lang");
     Node countryAttr = attrs.getNamedItem("country");
-    Node rtlAttr = attrs.getNamedItem("rtl");
+    Node rtlAttr = attrs.getNamedItem("language_direction");
 
     String messages = null;
     if (null != messagesAttr) {
@@ -181,10 +181,8 @@
       language = languageAttr.getNodeValue();
     }
 
-    boolean rightToLeft;
-    if (null == rtlAttr) {
-      rightToLeft = false;
-    } else {
+    boolean rightToLeft = false;
+    if (rtlAttr != null && rtlAttr.getTextContent().equals("rtl")) {
       rightToLeft = true;
     }
 
@@ -548,19 +546,25 @@
     public String getContentData() {
       return getContentData(DEFAULT_VIEW);
     }
-    
+
     public String getContentData(String view) {
       Check.is(contentType == ContentType.HTML,
                "getContentData() requires contentType HTML");
       if (view == null || view == "") {
         view = DEFAULT_VIEW;
       }
-      if (!contentData.containsKey(view)) {
-        return null;
+
+      StringBuilder content = contentData.get(view);
+      if (content == null) {
+        return "";
+      } else {
+        return content.toString();
       }
-      return contentData.get(view).toString();
     }
-    
+
+    // TODO: Synchronizing this seems unnecessary...a parse job should never
+    // happen across threads, and addContent *can't* be called anywhere but
+    // within a call to GadgetSpecParser.parse()
     public synchronized void addContent(String view, String content) {
       if (view == null || view.equals("")) {
         view = DEFAULT_VIEW;
@@ -569,7 +573,7 @@
       if (!contentData.containsKey(view)) {
         contentData.put(view, new StringBuilder());
       }
-      
+
       contentData.get(view).append(content);
     }
   }



Re: svn commit: r617472 - in /incubator/shindig/trunk: features/core-features.txt features/core/util.js features/views/feature.xml features/views/views.js java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java

Posted by Kevin Brown <et...@google.com>.
It's a common convention for hasOwnProperty tests (which really should be
done any time you iterate over object keys, especially now that I noticed
that caja modifies Object.prototype).

On Feb 1, 2008 10:36 AM, Cassie <do...@google.com> wrote:

> On Fri, Feb 1, 2008 at 3:05 AM, <et...@apache.org> wrote:
>
> > Author: etnu
> > Date: Fri Feb  1 03:05:01 2008
> > New Revision: 617472
> >
> > URL: http://svn.apache.org/viewvc?rev=617472&view=rev
> > Log:
> > Fixed features/views to actually use the current view data.
> > requestNavigateTo is currently a NOOP. It should just use gadgets.rpc,
> but
> > technically this feature should be core-views -- in other words, it
> should
> > not be an explicit feature according to the gadgets API spec. I have
> > petitioned for a spec change (the "views" javascript should not be a
> part of
> > core), but if that doesn't happen then this will need to be moved to
> core,
> > which means that IFPC / gadgets.rpc will always be delivered to every
> > gadget. This is extraordinarily wasteful. One possible ugly solution
> would
> > be to only serve views when we see an actual reference to "gadgets.views
> "
> > in the content section, but this is hacky, and really there's no reason
> for
> > views to be in the core spec anyway.
> >
> > Takes data in the following form:
> >
> > views.getParams() -- Assumes that any parameter named "view-<name>" is a
> > key that belongs in this. When Cajoled this won't make sense, so we
> should
> > probably add a check for whether or not we're rendering in an iframe or
> not
> > first.
> >
> > views.getCurrentView() -- Assumes that the current view is named in the
> > "view" query string parameter, which is what GadgetRenderingServlet uses
> to
> > discriminate between views.
> >
> > May be initialized by calling gadgets.views.init and passing in all of
> the
> > supported views for this container and whether or not they're the "only
> > visible" gadget at render time.
> >
> > Also fixed a possible NPE with GadgetSpecParser and removed a dead file.
> >
> >
> > Removed:
> >    incubator/shindig/trunk/features/core-features.txt
> > Modified:
> >    incubator/shindig/trunk/features/core/util.js
> >    incubator/shindig/trunk/features/views/feature.xml
> >    incubator/shindig/trunk/features/views/views.js
> >
> >
>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> >
> > Modified: incubator/shindig/trunk/features/core/util.js
> > URL:
> >
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/core/util.js?rev=617472&r1=617471&r2=617472&view=diff
> >
> >
> ==============================================================================
> > --- incubator/shindig/trunk/features/core/util.js (original)
> > +++ incubator/shindig/trunk/features/core/util.js Fri Feb  1 03:05:01
> 2008
> > @@ -114,6 +114,23 @@
> >     },
> >
> >     /**
> > +     * Utility function for generating an "enum" from an array.
> > +     *
> > +     * @param {Array.<String>} values The values to generate.
> > +     * @return {Map&lt;String,String&gt;} An object with member fields
> to
> > handle
> > +     *   the enum.
> > +     *
> > +     * @private Implementation detail.
> > +     */
> > +    makeEnum : function (values) {
> > +      var obj = {};
> > +      for (var i = 0, v; v = values[i]; ++i) {
> > +        obj[v] = v;
> > +      }
> > +      return obj;
> > +    },
> > +
> > +    /**
> >      * Gets the feature parameters.
> >      *
> >      * @param {String} feature The feature to get parameters for
> >
> > Modified: incubator/shindig/trunk/features/views/feature.xml
> > URL:
> >
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/views/feature.xml?rev=617472&r1=617471&r2=617472&view=diff
> >
> >
> ==============================================================================
> > --- incubator/shindig/trunk/features/views/feature.xml (original)
> > +++ incubator/shindig/trunk/features/views/feature.xml Fri Feb  1
> 03:05:01
> > 2008
> > @@ -18,7 +18,12 @@
> >  -->
> >  <feature>
> >   <name>views</name>
> > +  <!-- <dependency>ifpc</dependency> -->
> >   <gadget>
> >     <script src="views.js"/>
> > +    <!-- TODO: Clean this up and unify the configuration model -->
> > +    <script>
> > +      gadgets.views.init({"default" : false});
> > +    </script>
> >   </gadget>
> >  </feature>
> >
> > Modified: incubator/shindig/trunk/features/views/views.js
> > URL:
> >
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/views/views.js?rev=617472&r1=617471&r2=617472&view=diff
> >
> >
> ==============================================================================
> > --- incubator/shindig/trunk/features/views/views.js (original)
> > +++ incubator/shindig/trunk/features/views/views.js Fri Feb  1 03:05:01
> > 2008
> > @@ -24,126 +24,86 @@
> >  var gadgets = gadgets || {};
> >
> >  /**
> > - * @static
> > - * @class Provides operations for dealing with Views.
> > - * @name gadgets.views
> > + * Implements the gadgets.views API spec. See
> > + *
> http://code.google.com/apis/gadgets/docs/reference/gadgets.views.html
> >  */
> > -gadgets.views = gadgets.views || {};
> > +gadgets.views = function() {
> >
> > -/**
> > - * Attempts to navigate to this gadget in a different view. If the
> > container
> > - * supports parameters will pass the optional parameters along to the
> > gadget in
> > - * the new view.
> > - *
> > - * @param {gadgets.views.View} view The view to navigate to
> > - * @param {Map.&lt;String, String&gt;} opt_params Parameters to pass to
> > the
> > - *     gadget after it has been navigated to on the surface
> > - *
> > - * @member gadgets.views
> > - */
> > -gadgets.views.requestNavigateTo = function(surface, opt_params) {
> > -  return opensocial.Container.get().requestNavigateTo(surface,
> > opt_params);
> > -};
> > -
> > -/**
> > - * Returns the current view.
> > - *
> > - * @return {gadgets.views.View} The current view
> > - * @member gadgets.views
> > - */
> > -gadgets.views.getCurrentView = function() {
> > -  return this.surface;
> > +  /**
> > +   * Reference to the current view object.
> > +   */
> > +  var currentView = null;
> > +
> > +  /**
> > +   * Map of all supported views for this container.
> > +   */
> > +  var supportedViews = {};
> > +
> > +  /**
> > +   * Map of parameters passed to the current request.
> > +   */
> > +  var params = {};
> > +
> > +  return {
> > +    requestNavigateTo : function(view, opt_params) {
> > +      // TODO: Actually implementing this is going to require
> gadgets.rpcor
> > +      // ifpc or something.
> > +    },
> > +
> > +    getCurrentView : function() {
> > +      return currentView;
> > +    },
> > +
> > +    getSupportedViews : function() {
> > +      return supportedViews;
> > +    },
> > +
> > +    getParams : function() {
> > +      return params;
> > +    },
> > +
> > +    /**
> > +     * Initializes views. Assumes that the current view is the "view"
> > +     * url parameter (or default if "view" isn't supported), and that
> > +     * all view parameters are in the form view-<name>
> > +     * TODO: Use unified configuration when it becomes available.
> > +     * @param {Map&lt;String, Boolean&gt;} supported The views you
> > support,
> > +     *   where keys = name of the view and values = isOnlyVisible
> > +     */
> > +    init : function(supported) {
> > +      if (typeof supported["default"] === "undefined") {
> > +        throw new Error("default view required!");
> > +      }
> > +
> > +      for (var s in supported) if (supported.hasOwnProperty(s)) {
>
>
> we really need to agree on some code style docs for shindig.
> this for and if on the same line realllllyyy makes me cringe.
>
>
> >
> > +        supportedViews[s] = new gadgets.views.View(s, supported[s]);
> > +      }
> > +
> > +      var urlParams = gadgets.util.getUrlParameters();
> > +      // extract all parameters prefixed with "view-".
> > +      for (var p in urlParams) if (urlParams.hasOwnProperty(p)) {
> > +        if (p.substring(0, 5) === "view-") {
> > +          params[p.substr(5)] = urlParams[p];
> > +        }
> > +      }
> > +      currentView = supportedViews[urlParams.view] ||
> > supportedViews["default"];
> > +    }
> > +  };
> > +}();
> > +
> > +gadgets.views.View = function(name, opt_isOnlyVisible) {
> > +  this.name_ = name;
> > +  this.isOnlyVisible_ = !!opt_isOnlyVisible;
> >  };
> >
> > -/**
> > - * Returns a map of all the supported views. Keys each
> gadgets.view.Viewby
> > - * its name.
> > - *
> > - * @return {Map&lt;gadgets.views.ViewType | String,
> gadgets.views.View&gt;}
> > All
> > - *   supported views, keyed by their name attribute.
> > - * @member gadgets.views
> > - */
> > -gadgets.views.getSupportedViews = function() {
> > -  return this.supportedSurfaces;
> > -};
> > -
> > -/**
> > - * Returns the parameters passed into this gadget for this view. Does
> not
> > - * include all url parameters, only the ones passed into
> > - * gadgets.views.requestNavigateTo
> > - *
> > - * @return {Map.&lt;String, String&gt;} The parameter map
> > - * @member gadgets.views
> > - */
> > -gadgets.views.getParams = function() {
> > -  return this.params;
> > -};
> > -
> > -
> > -/**
> > - * @class Base interface for all view objects.
> > - * @name gadgets.views.View
> > - */
> > -
> > -/**
> > - * @private
> > - * @constructor
> > - */
> > -gadgets.views.View = function(name, opt_isOnlyVisibleGadgetValue) {
> > -  this.name = name;
> > -  this.isOnlyVisibleGadgetValue = !!opt_isOnlyVisibleGadgetValue;
> > -};
> > -
> > -/**
> > - * Returns the name of this view.
> > - *
> > - * @return {gadgets.views.ViewType | String} The view name, usually
> > specified as
> > - * a gadgets.views.ViewType
> > - */
> >  gadgets.views.View.prototype.getName = function() {
> > -  return this.name;
> > +  return this.name_;
> >  };
> >
> > -/**
> > - * Returns true if the gadget is the only visible gadget in this view.
> > - * On a canvas page or in maximize mode this is most likely true; on a
> > profile
> > - * page or in dashboard mode, it is most likely false.
> > - *
> > - * @return {boolean} True if the gadget is the only visible gadget;
> > otherwise, false
> > - */
> >  gadgets.views.View.prototype.isOnlyVisibleGadget = function() {
> > -  return this.isOnlyVisibleGadgetValue;
> > +  return this.isOnlyVisible_;
> >  };
> >
> > -
> > -/**
> > - * @static
> > - * @class
> > - * Used by <a href="gadgets.views.View.html"> View</a>s.
> > - * @name gadgets.views.ViewType
> > - */
> > -gadgets.views.ViewType = {
> > - /**
> > -  * A view where the gadget is displayed in a very large mode. It
> should
> > be the
> > -  * only thing on the page. In a social context, this is usually called
> > the
> > -  * canvas page.
> > -  *
> > -  * @member gadgets.views.ViewType
> > -  */
> > -  FULL_PAGE : 'FULL_PAGE',
> > -
> > - /**
> > -  * A view where the gadget is displayed in a small area usually on a
> > page with
> > -  * other gadgets. In a social context, this is usually called the
> > profile page.
> > -  *
> > -  * @member gadgets.views.ViewType
> > -  */
> > -  DASHBOARD : 'DASHBOARD',
> > -
> > - /**
> > -  * A view where the gadget is displayed in a small separate window by
> > itself.
> > -  *
> > -  * @member gadgets.views.ViewType
> > -  */
> > -  POPUP : 'POPUP'
> > -};
> > +gadgets.views.ViewType = gadgets.util.makeEnum([
> > +  "FULL_PAGE", "DASHBOARD", "POPUP"
> > +]);
> > \ No newline at end of file
> >
> > Modified:
> >
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> > URL:
> >
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java?rev=617472&r1=617471&r2=617472&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> > (original)
> > +++
> >
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> > Fri Feb  1 03:05:01 2008
> > @@ -160,7 +160,7 @@
> >     Node messagesAttr = attrs.getNamedItem("messages");
> >     Node languageAttr = attrs.getNamedItem("lang");
> >     Node countryAttr = attrs.getNamedItem("country");
> > -    Node rtlAttr = attrs.getNamedItem("rtl");
> > +    Node rtlAttr = attrs.getNamedItem("language_direction");
> >
> >     String messages = null;
> >     if (null != messagesAttr) {
> > @@ -181,10 +181,8 @@
> >       language = languageAttr.getNodeValue();
> >     }
> >
> > -    boolean rightToLeft;
> > -    if (null == rtlAttr) {
> > -      rightToLeft = false;
> > -    } else {
> > +    boolean rightToLeft = false;
> > +    if (rtlAttr != null && rtlAttr.getTextContent().equals("rtl")) {
> >       rightToLeft = true;
> >     }
> >
> > @@ -548,19 +546,25 @@
> >     public String getContentData() {
> >       return getContentData(DEFAULT_VIEW);
> >     }
> > -
> > +
> >     public String getContentData(String view) {
> >       Check.is(contentType == ContentType.HTML,
> >                "getContentData() requires contentType HTML");
> >       if (view == null || view == "") {
> >         view = DEFAULT_VIEW;
> >       }
> > -      if (!contentData.containsKey(view)) {
> > -        return null;
> > +
> > +      StringBuilder content = contentData.get(view);
> > +      if (content == null) {
> > +        return "";
> > +      } else {
> > +        return content.toString();
> >       }
> > -      return contentData.get(view).toString();
> >     }
> > -
> > +
> > +    // TODO: Synchronizing this seems unnecessary...a parse job should
> > never
> > +    // happen across threads, and addContent *can't* be called anywhere
> > but
> > +    // within a call to GadgetSpecParser.parse()
> >     public synchronized void addContent(String view, String content) {
> >       if (view == null || view.equals("")) {
> >         view = DEFAULT_VIEW;
> > @@ -569,7 +573,7 @@
> >       if (!contentData.containsKey(view)) {
> >         contentData.put(view, new StringBuilder());
> >       }
> > -
> > +
> >       contentData.get(view).append(content);
> >     }
> >   }
> >
> >
> >
>

Re: Coding Standards

Posted by Kevin Brown <et...@google.com>.
Coding standards will be included in the contributors guide that Dan P and
John H are working on, but they'll differ from language to language. As a
rule, we're trying to follow the most popular community coding conventions
found in other projects (Java = Sun conventions with 2 space indents,
javascript = basically the same as java with some special rules for language
differences). Once this gets published we'll probably have to run over some
files and make some changes.

In any case, any one person's opinion on a style question is irrelevant,
it's up to the project as a whole to decide coding standards. Once we have
the first draft of the document everyone will put their 2 cents in and we'll
come up with a guide that makes as many contributors happy as possible.
Anyone who's making contributions (including committers and people who've
been submitting patches without commit access) will get a say.

On Feb 2, 2008 7:23 AM, Paul Lindner <pl...@hi5.com> wrote:

> On Fri, Feb 01, 2008 at 10:36:34AM -0800, Cassie wrote:
> [....]
> > > +      }
> > > +
> > > +      for (var s in supported) if (supported.hasOwnProperty(s)) {
> >
> >
> > we really need to agree on some code style docs for shindig.
> > this for and if on the same line realllllyyy makes me cringe.
>
> Agreed, here are some apache projects with coding standards:
>
> http://commons.apache.org/net/code-standards.html
> http://geronimo.apache.org/coding-standards.html
> http://portals.apache.org/jetspeed-1/code-standards.html
>
> I tried running Idea's Reformat Code and it changed quite a lot.
>
>
>
> --
> Paul Lindner
> hi5 Architect
> plindner@hi5.com
>

Coding Standards

Posted by Paul Lindner <pl...@hi5.com>.
On Fri, Feb 01, 2008 at 10:36:34AM -0800, Cassie wrote:
[....]
> > +      }
> > +
> > +      for (var s in supported) if (supported.hasOwnProperty(s)) {
> 
> 
> we really need to agree on some code style docs for shindig.
> this for and if on the same line realllllyyy makes me cringe.

Agreed, here are some apache projects with coding standards:

http://commons.apache.org/net/code-standards.html
http://geronimo.apache.org/coding-standards.html
http://portals.apache.org/jetspeed-1/code-standards.html

I tried running Idea's Reformat Code and it changed quite a lot.



-- 
Paul Lindner
hi5 Architect
plindner@hi5.com

Re: svn commit: r617472 - in /incubator/shindig/trunk: features/core-features.txt features/core/util.js features/views/feature.xml features/views/views.js java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java

Posted by Cassie <do...@google.com>.
On Fri, Feb 1, 2008 at 3:05 AM, <et...@apache.org> wrote:

> Author: etnu
> Date: Fri Feb  1 03:05:01 2008
> New Revision: 617472
>
> URL: http://svn.apache.org/viewvc?rev=617472&view=rev
> Log:
> Fixed features/views to actually use the current view data.
> requestNavigateTo is currently a NOOP. It should just use gadgets.rpc, but
> technically this feature should be core-views -- in other words, it should
> not be an explicit feature according to the gadgets API spec. I have
> petitioned for a spec change (the "views" javascript should not be a part of
> core), but if that doesn't happen then this will need to be moved to core,
> which means that IFPC / gadgets.rpc will always be delivered to every
> gadget. This is extraordinarily wasteful. One possible ugly solution would
> be to only serve views when we see an actual reference to "gadgets.views"
> in the content section, but this is hacky, and really there's no reason for
> views to be in the core spec anyway.
>
> Takes data in the following form:
>
> views.getParams() -- Assumes that any parameter named "view-<name>" is a
> key that belongs in this. When Cajoled this won't make sense, so we should
> probably add a check for whether or not we're rendering in an iframe or not
> first.
>
> views.getCurrentView() -- Assumes that the current view is named in the
> "view" query string parameter, which is what GadgetRenderingServlet uses to
> discriminate between views.
>
> May be initialized by calling gadgets.views.init and passing in all of the
> supported views for this container and whether or not they're the "only
> visible" gadget at render time.
>
> Also fixed a possible NPE with GadgetSpecParser and removed a dead file.
>
>
> Removed:
>    incubator/shindig/trunk/features/core-features.txt
> Modified:
>    incubator/shindig/trunk/features/core/util.js
>    incubator/shindig/trunk/features/views/feature.xml
>    incubator/shindig/trunk/features/views/views.js
>
>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
>
> Modified: incubator/shindig/trunk/features/core/util.js
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/core/util.js?rev=617472&r1=617471&r2=617472&view=diff
>
> ==============================================================================
> --- incubator/shindig/trunk/features/core/util.js (original)
> +++ incubator/shindig/trunk/features/core/util.js Fri Feb  1 03:05:01 2008
> @@ -114,6 +114,23 @@
>     },
>
>     /**
> +     * Utility function for generating an "enum" from an array.
> +     *
> +     * @param {Array.<String>} values The values to generate.
> +     * @return {Map&lt;String,String&gt;} An object with member fields to
> handle
> +     *   the enum.
> +     *
> +     * @private Implementation detail.
> +     */
> +    makeEnum : function (values) {
> +      var obj = {};
> +      for (var i = 0, v; v = values[i]; ++i) {
> +        obj[v] = v;
> +      }
> +      return obj;
> +    },
> +
> +    /**
>      * Gets the feature parameters.
>      *
>      * @param {String} feature The feature to get parameters for
>
> Modified: incubator/shindig/trunk/features/views/feature.xml
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/views/feature.xml?rev=617472&r1=617471&r2=617472&view=diff
>
> ==============================================================================
> --- incubator/shindig/trunk/features/views/feature.xml (original)
> +++ incubator/shindig/trunk/features/views/feature.xml Fri Feb  1 03:05:01
> 2008
> @@ -18,7 +18,12 @@
>  -->
>  <feature>
>   <name>views</name>
> +  <!-- <dependency>ifpc</dependency> -->
>   <gadget>
>     <script src="views.js"/>
> +    <!-- TODO: Clean this up and unify the configuration model -->
> +    <script>
> +      gadgets.views.init({"default" : false});
> +    </script>
>   </gadget>
>  </feature>
>
> Modified: incubator/shindig/trunk/features/views/views.js
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/views/views.js?rev=617472&r1=617471&r2=617472&view=diff
>
> ==============================================================================
> --- incubator/shindig/trunk/features/views/views.js (original)
> +++ incubator/shindig/trunk/features/views/views.js Fri Feb  1 03:05:01
> 2008
> @@ -24,126 +24,86 @@
>  var gadgets = gadgets || {};
>
>  /**
> - * @static
> - * @class Provides operations for dealing with Views.
> - * @name gadgets.views
> + * Implements the gadgets.views API spec. See
> + * http://code.google.com/apis/gadgets/docs/reference/gadgets.views.html
>  */
> -gadgets.views = gadgets.views || {};
> +gadgets.views = function() {
>
> -/**
> - * Attempts to navigate to this gadget in a different view. If the
> container
> - * supports parameters will pass the optional parameters along to the
> gadget in
> - * the new view.
> - *
> - * @param {gadgets.views.View} view The view to navigate to
> - * @param {Map.&lt;String, String&gt;} opt_params Parameters to pass to
> the
> - *     gadget after it has been navigated to on the surface
> - *
> - * @member gadgets.views
> - */
> -gadgets.views.requestNavigateTo = function(surface, opt_params) {
> -  return opensocial.Container.get().requestNavigateTo(surface,
> opt_params);
> -};
> -
> -/**
> - * Returns the current view.
> - *
> - * @return {gadgets.views.View} The current view
> - * @member gadgets.views
> - */
> -gadgets.views.getCurrentView = function() {
> -  return this.surface;
> +  /**
> +   * Reference to the current view object.
> +   */
> +  var currentView = null;
> +
> +  /**
> +   * Map of all supported views for this container.
> +   */
> +  var supportedViews = {};
> +
> +  /**
> +   * Map of parameters passed to the current request.
> +   */
> +  var params = {};
> +
> +  return {
> +    requestNavigateTo : function(view, opt_params) {
> +      // TODO: Actually implementing this is going to require gadgets.rpcor
> +      // ifpc or something.
> +    },
> +
> +    getCurrentView : function() {
> +      return currentView;
> +    },
> +
> +    getSupportedViews : function() {
> +      return supportedViews;
> +    },
> +
> +    getParams : function() {
> +      return params;
> +    },
> +
> +    /**
> +     * Initializes views. Assumes that the current view is the "view"
> +     * url parameter (or default if "view" isn't supported), and that
> +     * all view parameters are in the form view-<name>
> +     * TODO: Use unified configuration when it becomes available.
> +     * @param {Map&lt;String, Boolean&gt;} supported The views you
> support,
> +     *   where keys = name of the view and values = isOnlyVisible
> +     */
> +    init : function(supported) {
> +      if (typeof supported["default"] === "undefined") {
> +        throw new Error("default view required!");
> +      }
> +
> +      for (var s in supported) if (supported.hasOwnProperty(s)) {


we really need to agree on some code style docs for shindig.
this for and if on the same line realllllyyy makes me cringe.


>
> +        supportedViews[s] = new gadgets.views.View(s, supported[s]);
> +      }
> +
> +      var urlParams = gadgets.util.getUrlParameters();
> +      // extract all parameters prefixed with "view-".
> +      for (var p in urlParams) if (urlParams.hasOwnProperty(p)) {
> +        if (p.substring(0, 5) === "view-") {
> +          params[p.substr(5)] = urlParams[p];
> +        }
> +      }
> +      currentView = supportedViews[urlParams.view] ||
> supportedViews["default"];
> +    }
> +  };
> +}();
> +
> +gadgets.views.View = function(name, opt_isOnlyVisible) {
> +  this.name_ = name;
> +  this.isOnlyVisible_ = !!opt_isOnlyVisible;
>  };
>
> -/**
> - * Returns a map of all the supported views. Keys each gadgets.view.Viewby
> - * its name.
> - *
> - * @return {Map&lt;gadgets.views.ViewType | String, gadgets.views.View&gt;}
> All
> - *   supported views, keyed by their name attribute.
> - * @member gadgets.views
> - */
> -gadgets.views.getSupportedViews = function() {
> -  return this.supportedSurfaces;
> -};
> -
> -/**
> - * Returns the parameters passed into this gadget for this view. Does not
> - * include all url parameters, only the ones passed into
> - * gadgets.views.requestNavigateTo
> - *
> - * @return {Map.&lt;String, String&gt;} The parameter map
> - * @member gadgets.views
> - */
> -gadgets.views.getParams = function() {
> -  return this.params;
> -};
> -
> -
> -/**
> - * @class Base interface for all view objects.
> - * @name gadgets.views.View
> - */
> -
> -/**
> - * @private
> - * @constructor
> - */
> -gadgets.views.View = function(name, opt_isOnlyVisibleGadgetValue) {
> -  this.name = name;
> -  this.isOnlyVisibleGadgetValue = !!opt_isOnlyVisibleGadgetValue;
> -};
> -
> -/**
> - * Returns the name of this view.
> - *
> - * @return {gadgets.views.ViewType | String} The view name, usually
> specified as
> - * a gadgets.views.ViewType
> - */
>  gadgets.views.View.prototype.getName = function() {
> -  return this.name;
> +  return this.name_;
>  };
>
> -/**
> - * Returns true if the gadget is the only visible gadget in this view.
> - * On a canvas page or in maximize mode this is most likely true; on a
> profile
> - * page or in dashboard mode, it is most likely false.
> - *
> - * @return {boolean} True if the gadget is the only visible gadget;
> otherwise, false
> - */
>  gadgets.views.View.prototype.isOnlyVisibleGadget = function() {
> -  return this.isOnlyVisibleGadgetValue;
> +  return this.isOnlyVisible_;
>  };
>
> -
> -/**
> - * @static
> - * @class
> - * Used by <a href="gadgets.views.View.html"> View</a>s.
> - * @name gadgets.views.ViewType
> - */
> -gadgets.views.ViewType = {
> - /**
> -  * A view where the gadget is displayed in a very large mode. It should
> be the
> -  * only thing on the page. In a social context, this is usually called
> the
> -  * canvas page.
> -  *
> -  * @member gadgets.views.ViewType
> -  */
> -  FULL_PAGE : 'FULL_PAGE',
> -
> - /**
> -  * A view where the gadget is displayed in a small area usually on a
> page with
> -  * other gadgets. In a social context, this is usually called the
> profile page.
> -  *
> -  * @member gadgets.views.ViewType
> -  */
> -  DASHBOARD : 'DASHBOARD',
> -
> - /**
> -  * A view where the gadget is displayed in a small separate window by
> itself.
> -  *
> -  * @member gadgets.views.ViewType
> -  */
> -  POPUP : 'POPUP'
> -};
> +gadgets.views.ViewType = gadgets.util.makeEnum([
> +  "FULL_PAGE", "DASHBOARD", "POPUP"
> +]);
> \ No newline at end of file
>
> Modified:
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java?rev=617472&r1=617471&r2=617472&view=diff
>
> ==============================================================================
> ---
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> (original)
> +++
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
> Fri Feb  1 03:05:01 2008
> @@ -160,7 +160,7 @@
>     Node messagesAttr = attrs.getNamedItem("messages");
>     Node languageAttr = attrs.getNamedItem("lang");
>     Node countryAttr = attrs.getNamedItem("country");
> -    Node rtlAttr = attrs.getNamedItem("rtl");
> +    Node rtlAttr = attrs.getNamedItem("language_direction");
>
>     String messages = null;
>     if (null != messagesAttr) {
> @@ -181,10 +181,8 @@
>       language = languageAttr.getNodeValue();
>     }
>
> -    boolean rightToLeft;
> -    if (null == rtlAttr) {
> -      rightToLeft = false;
> -    } else {
> +    boolean rightToLeft = false;
> +    if (rtlAttr != null && rtlAttr.getTextContent().equals("rtl")) {
>       rightToLeft = true;
>     }
>
> @@ -548,19 +546,25 @@
>     public String getContentData() {
>       return getContentData(DEFAULT_VIEW);
>     }
> -
> +
>     public String getContentData(String view) {
>       Check.is(contentType == ContentType.HTML,
>                "getContentData() requires contentType HTML");
>       if (view == null || view == "") {
>         view = DEFAULT_VIEW;
>       }
> -      if (!contentData.containsKey(view)) {
> -        return null;
> +
> +      StringBuilder content = contentData.get(view);
> +      if (content == null) {
> +        return "";
> +      } else {
> +        return content.toString();
>       }
> -      return contentData.get(view).toString();
>     }
> -
> +
> +    // TODO: Synchronizing this seems unnecessary...a parse job should
> never
> +    // happen across threads, and addContent *can't* be called anywhere
> but
> +    // within a call to GadgetSpecParser.parse()
>     public synchronized void addContent(String view, String content) {
>       if (view == null || view.equals("")) {
>         view = DEFAULT_VIEW;
> @@ -569,7 +573,7 @@
>       if (!contentData.containsKey(view)) {
>         contentData.put(view, new StringBuilder());
>       }
> -
> +
>       contentData.get(view).append(content);
>     }
>   }
>
>
>