From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Mon, 14 May 2018 11:17:18 +0000 (+0100)
Subject: JAL-2989 support value display names for Option or Parameters combo boxes
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=fb9c2cf4eb53fb3100d954934f8a5bfcb5fe5c12;p=jalview.git

JAL-2989 support value display names for Option or Parameters combo boxes
---

diff --git a/src/jalview/gui/OptsAndParamsPage.java b/src/jalview/gui/OptsAndParamsPage.java
index 1843f49..b13d77d 100644
--- a/src/jalview/gui/OptsAndParamsPage.java
+++ b/src/jalview/gui/OptsAndParamsPage.java
@@ -144,12 +144,30 @@ public class OptsAndParamsPage
         add(enabled);
       }
 
-      val = new JComboBox<>();
-      for (String str : opt.getPossibleValues())
+      /*
+       * construct the choice box with possible values, 
+       * or their display names if provided
+       */
+      List<String> displayNames = opt.getDisplayNames();
+      if (displayNames != null)
+      {
+        val = JvSwingUtils.buildComboWithTooltips(displayNames,
+                opt.getPossibleValues());
+      }
+      else
       {
-        val.addItem(str);
+        val = new JComboBox<>();
+        for (String v : opt.getPossibleValues())
+        {
+          val.addItem(v);
+        }
       }
       val.setSelectedItem(opt.getValue());
+
+      /*
+       * only show the choicebox if there is more than one option,
+       * or the option is mandatory
+       */
       if (opt.getPossibleValues().size() > 1 || opt.isRequired())
       {
         val.addActionListener(this);
@@ -228,32 +246,55 @@ public class OptsAndParamsPage
       poparent.argSetModified(this, !notmod);
     }
 
-    public OptionI getOptionIfEnabled()
+    /**
+     * Answers null if the option is not selected, else a new Option holding the
+     * selected value
+     * 
+     * @return
+     */
+    public OptionI getSelectedOption()
     {
       if (!enabled.isSelected())
       {
         return null;
       }
+      String value = getSelectedValue();
       OptionI opt = option.copy();
-      if (opt.getPossibleValues() != null
-              && opt.getPossibleValues().size() == 1)
+      opt.setValue(value);
+      return opt;
+    }
+
+    /*
+     * Answers the value corresponding to the selected item in the choice combo
+     * box. If display names were not provided, this is simply the selected
+     * value. If display names were provided, it is the value corresponding to
+     * the selected item index.
+     * 
+     * @return
+     */
+    protected String getSelectedValue()
+    {
+      List<String> possibleValues = option.getPossibleValues();
+      String value = null;
+      if (possibleValues != null && possibleValues.size() == 1)
       {
         // Hack to make sure the default value for an enabled option with only
-        // one value is actually returned
-        opt.setValue(opt.getPossibleValues().get(0));
-      }
-      if (val.getSelectedItem() != null)
-      {
-        opt.setValue((String) val.getSelectedItem());
+        // one value is actually returned even if this.val is not displayed
+        value = possibleValues.get(0);
       }
       else
       {
-        if (option.getValue() != null)
+        int sel = val.getSelectedIndex();
+        if (sel >= 0 && sel < possibleValues.size())
         {
-          opt.setValue(option.getValue());
+          value = possibleValues.get(sel);
+        }
+        else
+        {
+          value = option.getValue();
         }
       }
-      return opt;
+      return value;
     }
 
     @Override
@@ -618,7 +659,8 @@ public class OptsAndParamsPage
       ParameterI prm = parameter.copy();
       if (isChoiceParameter)
       {
-        prm.setValue((String) choicebox.getSelectedItem());
+        String value = getChoice();
+        prm.setValue(value);
       }
       else
       {
@@ -627,6 +669,24 @@ public class OptsAndParamsPage
       return prm;
     }
 
+    /**
+     * Answers the value corresponding to the selected item in the choice combo
+     * box. If display names were not provided, this is simply the selected
+     * value. If display names were provided, it is the value corresponding to
+     * the selected item index.
+     * 
+     * @return
+     */
+    protected String getChoice()
+    {
+      int sel = choicebox.getSelectedIndex();
+      if (sel < 0 || sel >= parameter.getPossibleValues().size())
+      {
+        return null;
+      }
+      return parameter.getPossibleValues().get(sel);
+    }
+
     public void init()
     {
       // reset the widget's initial value.
@@ -728,11 +788,24 @@ public class OptsAndParamsPage
     {
       adjusting = true;
       boolean init = (choicebox == null && valueField == null);
+      List<String> displayNames = parm.getDisplayNames();
       if (init)
       {
         if (isChoiceParameter)
         {
-          choicebox = new JComboBox<>();
+          if (displayNames != null)
+          {
+            choicebox = JvSwingUtils.buildComboWithTooltips(displayNames,
+                    parm.getPossibleValues());
+          }
+          else
+          {
+            choicebox = new JComboBox<>();
+            for (String val : parm.getPossibleValues())
+            {
+              choicebox.addItem(val);
+            }
+          }
           choicebox.addActionListener(this);
           controlsPanel.add(choicebox, BorderLayout.CENTER);
         }
@@ -765,26 +838,9 @@ public class OptsAndParamsPage
         }
       }
 
-      if (parm != null)
+      if (!isChoiceParameter && parm != null)
       {
-        if (isChoiceParameter)
-        {
-          if (init)
-          {
-            for (String val : parm.getPossibleValues())
-            {
-              choicebox.addItem(val);
-            }
-          }
-          if (parm.getValue() != null)
-          {
-            choicebox.setSelectedItem(parm.getValue());
-          }
-        }
-        else
-        {
-          valueField.setText(parm.getValue());
-        }
+        valueField.setText(parm.getValue());
       }
       lastVal = updateSliderFromValueField();
       adjusting = false;
@@ -810,7 +866,7 @@ public class OptsAndParamsPage
       {
         if (isChoiceParameter)
         {
-          return choicebox.getSelectedItem();
+          return getChoice();
         }
         slider.setVisible(false);
         return valueField.getText().trim();
@@ -1057,7 +1113,7 @@ public class OptsAndParamsPage
     List<ArgumentI> argSet = new ArrayList<>();
     for (OptionBox opts : getOptSet().values())
     {
-      OptionI opt = opts.getOptionIfEnabled();
+      OptionI opt = opts.getSelectedOption();
       if (opt != null)
       {
         argSet.add(opt);
diff --git a/src/jalview/ws/jws2/dm/JabaOption.java b/src/jalview/ws/jws2/dm/JabaOption.java
index cbfbd3b..f1f5014 100644
--- a/src/jalview/ws/jws2/dm/JabaOption.java
+++ b/src/jalview/ws/jws2/dm/JabaOption.java
@@ -117,4 +117,10 @@ public class JabaOption implements jalview.ws.params.OptionI
     return opt;
   }
 
+  @Override
+  public List<String> getDisplayNames()
+  {
+    return null; // not supported for Jaba options
+  }
+
 }
diff --git a/src/jalview/ws/params/OptionI.java b/src/jalview/ws/params/OptionI.java
index 2c5386d..af8ae27 100644
--- a/src/jalview/ws/params/OptionI.java
+++ b/src/jalview/ws/params/OptionI.java
@@ -25,15 +25,49 @@ import java.util.List;
 
 public interface OptionI extends ArgumentI
 {
-
+  /**
+   * Answers a URL with further details for this option, or null if none is
+   * known
+   * 
+   * @return
+   */
   URL getFurtherDetails();
 
+  /**
+   * Answers true if the option is mandatory (a value must be chosen), false if
+   * it is optional
+   * 
+   * @return
+   */
   boolean isRequired();
 
+  /**
+   * Answers the description of the option
+   * 
+   * @return
+   */
   String getDescription();
 
+  /**
+   * Answers a list of possible values that may be chosen for the option (or
+   * null if not applicable)
+   * 
+   * @return
+   */
   List<String> getPossibleValues();
 
-  OptionI copy();
+  /**
+   * Answers a list of display names corresponding to the possible values that
+   * may be chosen for the option (or null if not applicable)
+   * 
+   * @return
+   */
+  List<String> getDisplayNames();
 
+  /**
+   * Answers a new Option with a copy of the settings of this one
+   * 
+   * @return
+   */
+  OptionI copy();
 }
diff --git a/src/jalview/ws/params/simple/Option.java b/src/jalview/ws/params/simple/Option.java
index f0126df..ce5d669 100644
--- a/src/jalview/ws/params/simple/Option.java
+++ b/src/jalview/ws/params/simple/Option.java
@@ -24,7 +24,6 @@ import jalview.ws.params.OptionI;
 
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 
 public class Option implements OptionI
@@ -43,12 +42,107 @@ public class Option implements OptionI
 
   String description;
 
-  ArrayList<String> possibleVals = new ArrayList<>();
+  List<String> possibleVals;
+
+  /*
+   * optional display names corresponding to possibleVals
+   */
+  List<String> displayVals;
 
   boolean required;
 
   URL fdetails;
 
+  /**
+   * Copy constructor
+   * 
+   * @param opt
+   */
+  public Option(Option opt)
+  {
+    name = opt.name;
+    value = opt.value;
+    defvalue = opt.defvalue;
+    description = opt.description;
+    if (opt.possibleVals != null)
+    {
+      possibleVals = new ArrayList<>(opt.possibleVals);
+    }
+    required = opt.required;
+    // URLs are singletons - so we copy by reference. nasty but true.
+    fdetails = opt.fdetails;
+  }
+
+  public Option()
+  {
+  }
+
+  /**
+   * Constructor including display names for possible values
+   * 
+   * @param name2
+   * @param description2
+   * @param isrequired
+   * @param defValue
+   * @param val
+   * @param possibleVals
+   * @param fdetails
+   */
+  public Option(String name2, String description2, boolean isrequired,
+          String defValue, String val, List<String> possibleVals,
+          List<String> displayNames, URL fdetails)
+  {
+    name = name2;
+    description = description2;
+    this.value = val;
+    this.required = isrequired;
+    this.defvalue = defValue;
+    if (possibleVals != null)
+    {
+      this.possibleVals = new ArrayList<>(possibleVals);
+    }
+    if (displayNames != null)
+    {
+      this.displayVals = new ArrayList<>(displayNames);
+    }
+    this.fdetails = fdetails;
+  }
+
+  /**
+   * Constructor
+   * 
+   * @param name2
+   * @param description2
+   * @param isrequired
+   * @param defValue
+   * @param val
+   * @param possibleVals
+   * @param fdetails
+   */
+  public Option(String name2, String description2, boolean isrequired,
+          String defValue, String val, List<String> possibleVals,
+          URL fdetails)
+  {
+    this(name2, description2, isrequired, defValue, val, possibleVals, null,
+            fdetails);
+  }
+
+  @Override
+  public OptionI copy()
+  {
+    Option opt = new Option(this);
+    return opt;
+  }
+
+  /**
+   * toString method to help identify options in the debugger only
+   */
+  @Override
+  public String toString()
+  {
+    return this.getClass().getName() + ":" + name;
+  }
+
   @Override
   public String getName()
   {
@@ -91,64 +185,9 @@ public class Option implements OptionI
     return possibleVals;
   }
 
-  public Option(Option opt)
-  {
-    name = new String(opt.name);
-    if (opt.value != null)
-    {
-      value = new String(opt.value);
-    }
-    if (opt.defvalue != null)
-    {
-      defvalue = new String(opt.defvalue);
-    }
-    if (opt.description != null)
-    {
-      description = new String(opt.description);
-    }
-    if (opt.possibleVals != null)
-    {
-      possibleVals = (ArrayList<String>) opt.possibleVals.clone();
-    }
-    required = opt.required;
-    // URLs are singletons - so we copy by reference. nasty but true.
-    fdetails = opt.fdetails;
-  }
-
-  public Option()
-  {
-  }
-
-  public Option(String name2, String description2, boolean isrequired,
-          String defValue, String value, Collection<String> possibleVals,
-          URL fdetails)
-  {
-    name = name2;
-    description = description2;
-    this.value = value;
-    this.required = isrequired;
-    this.defvalue = defValue;
-    if (possibleVals != null)
-    {
-      this.possibleVals = new ArrayList<>();
-      this.possibleVals.addAll(possibleVals);
-    }
-    this.fdetails = fdetails;
-  }
-
   @Override
-  public OptionI copy()
-  {
-    Option opt = new Option(this);
-    return opt;
-  }
-
-  /**
-   * toString method to help identify options in the debugger only
-   */
-  @Override
-  public String toString()
+  public List<String> getDisplayNames()
   {
-    return this.getClass().getName() + ":" + name;
+    return displayVals;
   }
 }
diff --git a/src/jalview/ws/params/simple/StringParameter.java b/src/jalview/ws/params/simple/StringParameter.java
index b61836a..d3d899c 100644
--- a/src/jalview/ws/params/simple/StringParameter.java
+++ b/src/jalview/ws/params/simple/StringParameter.java
@@ -3,7 +3,7 @@ package jalview.ws.params.simple;
 import jalview.ws.params.ParameterI;
 import jalview.ws.params.ValueConstrainI;
 
-import java.util.Collection;
+import java.util.List;
 
 public class StringParameter extends Option implements ParameterI
 {
@@ -46,6 +46,8 @@ public class StringParameter extends Option implements ParameterI
   {
     this.name = parm.name;
     this.defvalue = parm.defvalue;
+    this.possibleVals = parm.possibleVals;
+    this.displayVals = parm.displayVals;
   }
 
   public StringParameter(String name, String description, boolean required,
@@ -65,7 +67,8 @@ public class StringParameter extends Option implements ParameterI
   }
 
   /**
-   * Constructor for a parameter with a list of possible values
+   * Constructor for a parameter with a list of possible values and (optionally)
+   * corresponding display names
    * 
    * @param name2
    * @param description2
@@ -73,12 +76,13 @@ public class StringParameter extends Option implements ParameterI
    * @param defValue
    * @param value
    * @param possibleVals
+   * @param displayNames
    */
   public StringParameter(String name2, String description2,
           boolean isrequired, String defValue, String value,
-          Collection<String> possibleVals)
+          List<String> possibleVals, List<String> displayNames)
   {
     super(name2, description2, isrequired, defValue, value, possibleVals,
-            null);
+            displayNames, null);
   }
 }
diff --git a/src/jalview/ws/rest/InputType.java b/src/jalview/ws/rest/InputType.java
index 88431a6..c83879a 100644
--- a/src/jalview/ws/rest/InputType.java
+++ b/src/jalview/ws/rest/InputType.java
@@ -30,7 +30,6 @@ import jalview.ws.params.simple.Option;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -58,9 +57,9 @@ public abstract class InputType
   {
     NUC, PROT, MIX;
 
-    public static Collection<String> toStringValues()
+    public static List<String> toStringValues()
     {
-      Collection<String> c = new ArrayList<String>();
+      List<String> c = new ArrayList<>();
       for (molType type : values())
       {
         c.add(type.toString());
@@ -75,7 +74,7 @@ public abstract class InputType
 
   public int max = 0; // unbounded
 
-  protected ArrayList<Class> inputData = new ArrayList<Class>();
+  protected List<Class> inputData = new ArrayList<>();
 
   /**
    * initialise the InputType with a list of jalview data classes that the
@@ -104,7 +103,9 @@ public abstract class InputType
   public boolean validFor(RestJob restJob)
   {
     if (!validFor(restJob.rsd))
+    {
       return false;
+    }
     for (Class cl : inputData)
     {
       if (!restJob.hasDataOfType(cl))
@@ -118,7 +119,9 @@ public abstract class InputType
   public boolean validFor(RestServiceDescription restServiceDescription)
   {
     if (!restServiceDescription.inputParams.values().contains(this))
+    {
       return false;
+    }
 
     return true;
   }
@@ -270,7 +273,7 @@ public abstract class InputType
 
   public List<OptionI> getBaseOptions()
   {
-    ArrayList<OptionI> opts = new ArrayList<OptionI>();
+    ArrayList<OptionI> opts = new ArrayList<>();
     opts.add(new IntegerParameter("min",
             "Minimum number of data of this type", true, 1, min, 0, -1));
     opts.add(new IntegerParameter("max",
@@ -295,7 +298,7 @@ public abstract class InputType
   public void configureFromArgumentI(List<ArgumentI> currentSettings)
           throws InvalidArgumentException
   {
-    ArrayList<String> urltoks = new ArrayList<String>();
+    List<String> urltoks = new ArrayList<>();
     String rg;
     for (ArgumentI arg : currentSettings)
     {