ADT GLE2: cleanup some constants and review feedback.

Change-Id: Ib9f3473d49bc27dfb87d29855a832a21bdcf3100

6 files changed, 36 insertions, 18 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/README.txt b/docs/gscripts.txt
index e3f5d18..e3f5d18 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/README.txt
+++ b/docs/gscripts.txt
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseLayout.groovy b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseLayout.groovy
index ef9ea30..98e0b0f 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseLayout.groovy
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseLayout.groovy
@@ -76,7 +76,7 @@ public class BaseLayout extends BaseView {
      * Collect all the "android:id" IDs from the dropped elements.
      *
      * When moving objects within the same canvas, that's all there is to do.
-     * However if the objects are move to a different canvas or are copied
+     * However if the objects are moved to a different canvas or are copied
      * then set createNewIds to true to find the existing IDs under targetNode
      * and create a map with new non-conflicting unique IDs as needed.
      *
@@ -107,7 +107,7 @@ public class BaseLayout extends BaseView {
      */
     protected Map collectIds(Map idMap, IDragElement[] elements) {
         for (element in elements) {
-            def attr = element.getAttribute(ANDROID_URI, "id");
+            def attr = element.getAttribute(ANDROID_URI, ATTR_ID);
             if (attr != null) {
                 String id = attr.getValue();
                 if (id != null && id != "") {
@@ -178,7 +178,7 @@ public class BaseLayout extends BaseView {
             return;
         }
 
-        def id = root.getStringAttr(ANDROID_URI, "id");
+        def id = root.getStringAttr(ANDROID_URI, ATTR_ID);
         if (id != null) {
             id = normalizeId(id);
 
@@ -209,14 +209,14 @@ public class BaseLayout extends BaseView {
      * If filter is non-null, it's a closure that takes for argument:
      *   String attribue-uri (namespace), String attribute-name, String attribute-value
      * The closure should return a valid replacement string.
-     * The closure can reutnr either null, false or an empty string to prevent the attribute
+     * The closure can return either null, false or an empty string to prevent the attribute
      * from being copied into the new node.
      */
     protected void addAttributes(INode newNode, IDragElement oldElement,
                                  Map idMap, Closure filter) {
 
         // A little trick here: when creating new UI widgets by dropping them from
-        // the palette, we compute them a new id and then set the text attribute
+        // the palette, we assign them a new id and then set the text attribute
         // to that id, so for example a Button will have android:text="@+id/Button01".
         // Here we detect if such an id is being remapped to a new id and if there's
         // a text attribute with exactly the same id name, we update it too.
@@ -230,9 +230,9 @@ public class BaseLayout extends BaseView {
             String value = attr.getValue();
 
             if (uri == ANDROID_URI) {
-                if (name == "id") {
+                if (name == ATTR_ID) {
                     oldId = value;
-                } else if (name == "text") {
+                } else if (name == ATTR_TEXT) {
                     oldText = value;
                 }
             }
@@ -253,14 +253,14 @@ public class BaseLayout extends BaseView {
             if (value != null && value != false && value != "") {
                 newNode.setAttribute(uri, name, value);
 
-                if (uri == ANDROID_URI && name == "id" && oldId != null && value != oldId) {
+                if (uri == ANDROID_URI && name == ATTR_ID && oldId != null && value != oldId) {
                     newId = value;
                 }
             }
         }
 
         if (newId != null && oldText == oldId) {
-            newNode.setAttribute(ANDROID_URI, "text", newId);
+            newNode.setAttribute(ANDROID_URI, ATTR_TEXT, newId);
         }
     }
 
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseView.groovy b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseView.groovy
index 352fa26..8a479d0 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseView.groovy
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/BaseView.groovy
@@ -20,11 +20,24 @@ public class BaseView implements IViewRule {
 
     private String mFqcn;
 
+    // Some common Android layout attribute names used by the view rules.
+    // All these belong to the attribute namespace ANDROID_URI.
+    public static String ATTR_ID = "id";
+    public static String ATTR_TEXT = "text";
+    public static String ATTR_LAYOUT_WIDTH = "layout_width";
+    public static String ATTR_LAYOUT_HEIGHT = "layout_height";
+
+    // Some common Android layout attribute values used by the view rules.
+    public static String VALUE_FILL_PARENT = "fill_parent";
+    public static String VALUE_MATCH_PARENT = "match_parent";
+    public static String VALUE_MATCH_CONTENT = "match_content";
+
+
     /**
      * Namespace for the Android resource XML,
      * i.e. "http://schemas.android.com/apk/res/android"
      */
-    public static ANDROID_URI = "http://schemas.android.com/apk/res/android";
+    public static String ANDROID_URI = "http://schemas.android.com/apk/res/android";
 
     public boolean onInitialize(String fqcn) {
         // This base rule can handle any class.
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.LinearLayout.groovy b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.LinearLayout.groovy
index 9b46b9c..1f1b1c6 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.LinearLayout.groovy
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.LinearLayout.groovy
@@ -21,6 +21,9 @@ package com.android.adt.gscripts;
  */
 public class AndroidWidgetLinearLayoutRule extends BaseLayout {
 
+    public static String ATTR_ORIENTATION = "orientation";
+    public static String VALUE_VERTICAL = "vertical";
+
     // ==== Drag'n'drop support ====
 
     DropFeedback onDropEnter(INode targetNode, IDragElement[] elements) {
@@ -34,7 +37,8 @@ public class AndroidWidgetLinearLayoutRule extends BaseLayout {
             return;
         }
 
-        boolean isVertical = targetNode.getStringAttr(ANDROID_URI, "orientation") == "vertical";
+        boolean isVertical =
+            targetNode.getStringAttr(ANDROID_URI, ATTR_ORIENTATION) == VALUE_VERTICAL;
 
         // Prepare a list of insertion points: X coords for horizontal, Y for vertical.
         // Each list is a tuple: 0=pixel coordinate, 1=index of children or -1 for "at end".
@@ -229,10 +233,11 @@ public class AndroidWidgetLinearLayoutRule extends BaseLayout {
                 }
 
                 // Copy all the attributes, modifying them as needed.
+                def attrFilter = getLayoutAttrFilter();
                 addAttributes(newChild, element, idMap) {
                     uri, name, value ->
-                    // TODO exclude original parent attributes
-                    if (name == "layout_x" || name == "layout_y") {
+                    // TODO need a better way to exclude other layout attributes dynamically
+                    if (uri == ANDROID_URI && name in attrFilter) {
                         return false; // don't set these attributes
                     } else {
                         return value;
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.ListView.groovy b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.ListView.groovy
index 6325dfa..b7c00ca 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.ListView.groovy
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.ListView.groovy
@@ -30,8 +30,8 @@ public class AndroidWidgetListViewRule extends BaseView {
      * However ListView is special in that ideally we want fill_parent width by default.
      */
     public Map<?, ?> getDefaultAttributes() {
-        // TODO: find a way to plug in the new value match_parent.
-        return [ "layout_width"  : "fill_parent" ];
+        // TODO: find a way to plug in the new value VALUE_MATCH_PARENT.
+        return [ ATTR_LAYOUT_WIDTH  : VALUE_FILL_PARENT ];
     }
 
 }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.RelativeLayout.groovy b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.RelativeLayout.groovy
index a658bae..02606d9 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.RelativeLayout.groovy
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/gscripts/android.widget.RelativeLayout.groovy
@@ -273,7 +273,7 @@ public class AndroidWidgetRelativeLayoutRule extends BaseLayout {
                 if (child == node) {
                     continue;
                 }
-                def childId = child.getStringAttr(ANDROID_URI, "id");
+                def childId = child.getStringAttr(ANDROID_URI, ATTR_ID);
                 childId = normalizeId(childId);
                 if (id == childId) {
                     def linkedIds = getLinkedIds(child, cachedLinkIds);
@@ -462,7 +462,7 @@ public class AndroidWidgetRelativeLayoutRule extends BaseLayout {
             int h = gc.getFontHeight();
             String id = null;
             if (data.child) {
-                id = data.child.getStringAttr(ANDROID_URI, "id");
+                id = data.child.getStringAttr(ANDROID_URI, ATTR_ID);
             }
             data.curr.attr.each {
                 String s = it;
@@ -555,7 +555,7 @@ public class AndroidWidgetRelativeLayoutRule extends BaseLayout {
 // TODO... seems totally wrong. REVISIT or EXPLAIN
                 String id = null;
                 if (data.child) {
-                    id = data.child.getStringAttr(ANDROID_URI, "id");
+                    id = data.child.getStringAttr(ANDROID_URI, ATTR_ID);
                 }
 
                 data.curr.attr.each {