From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Mon, 31 Oct 2016 10:25:06 +0000 (+0000)
Subject: Merge branch 'develop' into features/JAL-1723_sequenceReport
X-Git-Tag: Release_2_10_1~11^2^2~3^2~1
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=a86ad83f39c98c9a06d08a1ba1fa8b529d30a63c;p=jalview.git

Merge branch 'develop' into features/JAL-1723_sequenceReport

Conflicts:
	src/jalview/io/SequenceAnnotationReport.java
---

a86ad83f39c98c9a06d08a1ba1fa8b529d30a63c
diff --cc src/jalview/io/SequenceAnnotationReport.java
index 96d71f6,07b88bf..59daa87
--- a/src/jalview/io/SequenceAnnotationReport.java
+++ b/src/jalview/io/SequenceAnnotationReport.java
@@@ -21,16 -21,15 +21,19 @@@
  package jalview.io;
  
  import jalview.datamodel.DBRefEntry;
++import jalview.datamodel.DBRefSource;
  import jalview.datamodel.SequenceFeature;
  import jalview.datamodel.SequenceI;
+ import jalview.io.gff.GffConstants;
+ import jalview.util.DBRefUtils;
++import jalview.util.MessageManager;
  import jalview.util.UrlLink;
  
  import java.util.ArrayList;
 +import java.util.Arrays;
- import java.util.Collections;
 +import java.util.Comparator;
- import java.util.Hashtable;
  import java.util.List;
+ import java.util.Map;
  
  /**
   * generate HTML reports for a sequence
@@@ -39,191 -38,195 +42,251 @@@
   */
  public class SequenceAnnotationReport
  {
++  private static final String COMMA = ",";
++
++  private static final String ELLIPSIS = "...";
++
++  private static final int MAX_REFS_PER_SOURCE = 4;
++
++  private static final int MAX_SOURCES = 40;
++
++  private static final String[][] PRIMARY_SOURCES = new String[][] {
++      DBRefSource.CODINGDBS, DBRefSource.DNACODINGDBS,
++      DBRefSource.PROTEINDBS };
++
    final String linkImageURL;
  
 +  /*
 +   * Comparator to order DBRefEntry by Source + accession id (case-insensitive)
 +   */
 +  private static Comparator<DBRefEntry> comparator = new Comparator<DBRefEntry>()
 +  {
++
 +    @Override
 +    public int compare(DBRefEntry ref1, DBRefEntry ref2)
 +    {
 +      String s1 = ref1.getSource();
 +      String s2 = ref2.getSource();
++      boolean s1Primary = isPrimarySource(s1);
++      boolean s2Primary = isPrimarySource(s2);
++      if (s1Primary && !s2Primary)
++      {
++        return -1;
++      }
++      if (!s1Primary && s2Primary)
++      {
++        return 1;
++      }
 +      int comp = s1 == null ? -1 : (s2 == null ? 1 : s1
 +              .compareToIgnoreCase(s2));
 +      if (comp == 0)
 +      {
 +        String a1 = ref1.getAccessionId();
 +        String a2 = ref2.getAccessionId();
 +        comp = a1 == null ? -1 : (a2 == null ? 1 : a1
 +                .compareToIgnoreCase(a2));
 +      }
 +      return comp;
 +    }
++
++    private boolean isPrimarySource(String source)
++    {
++      for (String[] primary : PRIMARY_SOURCES)
++      {
++        for (String s : primary)
++        {
++          if (source.equals(s))
++          {
++            return true;
++          }
++        }
++      }
++      return false;
++    }
 +  };
 +
    public SequenceAnnotationReport(String linkImageURL)
    {
      this.linkImageURL = linkImageURL;
    }
  
    /**
-    * appends the features at rpos to the given stringbuffer ready for display in
-    * a tooltip
+    * Append text for the list of features to the tooltip
     * 
-    * @param tooltipText
-    * @param linkImageURL
 -   * @param tooltipText2
++   * @param sb
     * @param rpos
     * @param features
     * @param minmax
-    *          TODO refactor to Jalview 'utilities' somehow.
     */
-   public void appendFeatures(final StringBuilder tooltipText, int rpos,
-           List<SequenceFeature> features, Hashtable minmax)
 -  public void appendFeatures(final StringBuffer tooltipText2, int rpos,
++  public void appendFeatures(final StringBuilder sb, int rpos,
+           List<SequenceFeature> features, Map<String, float[][]> minmax)
    {
-     String tmpString;
      if (features != null)
      {
        for (SequenceFeature feature : features)
        {
-         if (feature.getType().equals("disulfide bond"))
 -        appendFeature(tooltipText2, rpos, minmax, feature);
++        appendFeature(sb, rpos, minmax, feature);
+       }
+     }
+   }
+ 
+   /**
 -   * Appends text for one sequence feature to the string buffer
++   * Appends the feature at rpos to the given buffer
+    * 
+    * @param sb
+    * @param rpos
+    * @param minmax
 -   *          {{min, max}, {min, max}} positional and non-positional feature
 -   *          scores for this type
+    * @param feature
+    */
 -  void appendFeature(final StringBuffer sb, int rpos,
++  void appendFeature(final StringBuilder sb, int rpos,
+           Map<String, float[][]> minmax, SequenceFeature feature)
+   {
 -    if ("disulfide bond".equals(feature.getType()))
++    String tmpString;
++    if (feature.getType().equals("disulfide bond"))
+     {
+       if (feature.getBegin() == rpos || feature.getEnd() == rpos)
+       {
+         if (sb.length() > 6)
          {
-           if (feature.getBegin() == rpos || feature.getEnd() == rpos)
-           {
-             if (tooltipText.length() > 6)
-             {
-               tooltipText.append("<br>");
-             }
-             tooltipText.append("disulfide bond " + feature.getBegin()
-                     + ":" + feature.getEnd());
-           }
+           sb.append("<br>");
          }
-         else
+         sb.append("disulfide bond ").append(feature.getBegin()).append(":")
+                 .append(feature.getEnd());
+       }
+     }
+     else
+     {
+       if (sb.length() > 6)
+       {
+         sb.append("<br>");
+       }
+       // TODO: remove this hack to display link only features
+       boolean linkOnly = feature.getValue("linkonly") != null;
+       if (!linkOnly)
+       {
+         sb.append(feature.getType()).append(" ");
+         if (rpos != 0)
          {
-           if (tooltipText.length() > 6)
+           // we are marking a positional feature
+           sb.append(feature.begin);
+         }
+         if (feature.begin != feature.end)
+         {
 -          sb.append(" " + feature.end);
++          sb.append(" ").append(feature.end);
+         }
+ 
+         if (feature.getDescription() != null
+                 && !feature.description.equals(feature.getType()))
+         {
 -          String tmpString = feature.getDescription();
++          tmpString = feature.getDescription();
+           String tmp2up = tmpString.toUpperCase();
 -          final int startTag = tmp2up.indexOf("<HTML>");
++          int startTag = tmp2up.indexOf("<HTML>");
+           if (startTag > -1)
            {
-             tooltipText.append("<br>");
+             tmpString = tmpString.substring(startTag + 6);
+             tmp2up = tmp2up.substring(startTag + 6);
            }
-           // TODO: remove this hack to display link only features
-           boolean linkOnly = feature.getValue("linkonly") != null;
-           if (!linkOnly)
 -          // TODO strips off </body> but not <body> - is that intended?
+           int endTag = tmp2up.indexOf("</BODY>");
+           if (endTag > -1)
            {
-             tooltipText.append(feature.getType() + " ");
-             if (rpos != 0)
-             {
-               // we are marking a positional feature
-               tooltipText.append(feature.begin);
-             }
-             if (feature.begin != feature.end)
-             {
-               tooltipText.append(" " + feature.end);
-             }
+             tmpString = tmpString.substring(0, endTag);
+             tmp2up = tmp2up.substring(0, endTag);
+           }
+           endTag = tmp2up.indexOf("</HTML>");
+           if (endTag > -1)
+           {
+             tmpString = tmpString.substring(0, endTag);
+           }
  
-             if (feature.getDescription() != null
-                     && !feature.description.equals(feature.getType()))
+           if (startTag > -1)
+           {
+             sb.append("; ").append(tmpString);
+           }
+           else
+           {
+             if (tmpString.indexOf("<") > -1 || tmpString.indexOf(">") > -1)
              {
-               tmpString = feature.getDescription();
-               String tmp2up = tmpString.toUpperCase();
-               int startTag = tmp2up.indexOf("<HTML>");
-               if (startTag > -1)
-               {
-                 tmpString = tmpString.substring(startTag + 6);
-                 tmp2up = tmp2up.substring(startTag + 6);
-               }
-               int endTag = tmp2up.indexOf("</BODY>");
-               if (endTag > -1)
-               {
-                 tmpString = tmpString.substring(0, endTag);
-                 tmp2up = tmp2up.substring(0, endTag);
-               }
-               endTag = tmp2up.indexOf("</HTML>");
-               if (endTag > -1)
-               {
-                 tmpString = tmpString.substring(0, endTag);
-               }
- 
-               if (startTag > -1)
-               {
-                 tooltipText.append("; " + tmpString);
-               }
-               else
-               {
-                 if (tmpString.indexOf("<") > -1
-                         || tmpString.indexOf(">") > -1)
-                 {
-                   // The description does not specify html is to
-                   // be used, so we must remove < > symbols
-                   tmpString = tmpString.replaceAll("<", "&lt;");
-                   tmpString = tmpString.replaceAll(">", "&gt;");
+               // The description does not specify html is to
+               // be used, so we must remove < > symbols
+               tmpString = tmpString.replaceAll("<", "&lt;");
+               tmpString = tmpString.replaceAll(">", "&gt;");
 -              sb.append("; ").append(tmpString);
 +
-                   tooltipText.append("; ");
-                   tooltipText.append(tmpString);
- 
-                 }
-                 else
-                 {
-                   tooltipText.append("; " + tmpString);
-                 }
-               }
++              sb.append("; ");
++              sb.append(tmpString);
              }
-             // check score should be shown
-             if (!Float.isNaN(feature.getScore()))
+             else
              {
-               float[][] rng = (minmax == null) ? null : ((float[][]) minmax
-                       .get(feature.getType()));
-               if (rng != null && rng[0] != null && rng[0][0] != rng[0][1])
-               {
-                 tooltipText.append(" Score=" + feature.getScore());
-               }
-             }
-             if (feature.getValue("status") != null)
-             {
-               String status = feature.getValue("status").toString();
-               if (status.length() > 0)
-               {
-                 tooltipText.append("; (" + feature.getValue("status")
-                         + ")");
-               }
+               sb.append("; ").append(tmpString);
              }
            }
          }
-         if (feature.links != null)
 -
 -        /*
 -         * score should be shown if there is one, and min != max
 -         * for this feature type (e.g. not all 0)
 -         */
++        // check score should be shown
+         if (!Float.isNaN(feature.getScore()))
          {
-           if (linkImageURL != null)
 -          float[][] rng = (minmax == null) ? null : minmax.get(feature
 -                  .getType());
++          float[][] rng = (minmax == null) ? null : ((float[][]) minmax
++                  .get(feature.getType()));
+           if (rng != null && rng[0] != null && rng[0][0] != rng[0][1])
            {
-             tooltipText.append(" <img src=\"" + linkImageURL + "\">");
 -            sb.append(" Score=").append(String.valueOf(feature.getScore()));
++            sb.append(" Score=" + feature.getScore());
            }
-           else
+         }
+         String status = (String) feature.getValue("status");
+         if (status != null && status.length() > 0)
+         {
+           sb.append("; (").append(status).append(")");
+         }
+         String clinSig = (String) feature
+                 .getValue(GffConstants.CLINICAL_SIGNIFICANCE);
+         if (clinSig != null)
+         {
+           sb.append("; ").append(clinSig);
+         }
+       }
+     }
 -    appendLinks(sb, feature);
+   }
+ 
+   /**
+    * Format and appends any hyperlinks for the sequence feature to the string
+    * buffer
+    * 
+    * @param sb
+    * @param feature
+    */
+   void appendLinks(final StringBuffer sb, SequenceFeature feature)
+   {
+     if (feature.links != null)
+     {
+       if (linkImageURL != null)
+       {
+         sb.append(" <img src=\"" + linkImageURL + "\">");
+       }
+       else
+       {
+         for (String urlstring : feature.links)
+         {
+           try
            {
-             for (String urlstring : feature.links)
+             for (String[] urllink : createLinksFrom(null, urlstring))
              {
-               try
-               {
-                 for (String[] urllink : createLinksFrom(null, urlstring))
-                 {
-                   tooltipText.append("<br/> <a href=\""
-                           + urllink[3]
-                           + "\" target=\""
-                           + urllink[0]
-                           + "\">"
-                           + (urllink[0].toLowerCase().equals(
-                                   urllink[1].toLowerCase()) ? urllink[0]
-                                   : (urllink[0] + ":" + urllink[1]))
-                           + "</a></br>");
-                 }
-               } catch (Exception x)
-               {
-                 System.err.println("problem when creating links from "
-                         + urlstring);
-                 x.printStackTrace();
-               }
+               sb.append("<br/> <a href=\""
+                       + urllink[3]
+                       + "\" target=\""
+                       + urllink[0]
+                       + "\">"
+                       + (urllink[0].toLowerCase().equals(
+                               urllink[1].toLowerCase()) ? urllink[0]
+                               : (urllink[0] + ":" + urllink[1]))
+                       + "</a></br>");
              }
+           } catch (Exception x)
+           {
+             System.err.println("problem when creating links from "
+                     + urlstring);
+             x.printStackTrace();
            }
- 
          }
        }
+ 
      }
    }
  
@@@ -244,53 -247,62 +307,63 @@@
        System.err.println(urlLink.getInvalidMessage());
        return null;
      }
 +    final String target = urlLink.getTarget(); // link.substring(0,
 +    // link.indexOf("|"));
 +    final String label = urlLink.getLabel();
      if (seq != null && urlLink.isDynamic())
      {
- 
-       // collect matching db-refs
-       DBRefEntry[] dbr = jalview.util.DBRefUtils.selectRefs(seq.getDBRefs(),
-               new String[] { target });
-       // collect id string too
-       String id = seq.getName();
-       String descr = seq.getDescription();
-       if (descr != null && descr.length() < 1)
+       urlSets.addAll(createDynamicLinks(seq, urlLink, uniques));
+     }
+     else
+     {
 -      String target = urlLink.getTarget();
 -      String label = urlLink.getLabel();
+       String unq = label + "|" + urlLink.getUrl_prefix();
+       if (!uniques.contains(unq))
        {
-         descr = null;
+         uniques.add(unq);
+         urlSets.add(new String[] { target, label, null,
+             urlLink.getUrl_prefix() });
        }
-       if (dbr != null)
+     }
+ 
+     return urlSets.toArray(new String[][] {});
+   }
+ 
+   /**
+    * Formats and returns a list of dynamic href links
+    * 
+    * @param seq
+    * @param urlLink
+    * @param uniques
+    */
+   List<String[]> createDynamicLinks(SequenceI seq, UrlLink urlLink,
+           List<String> uniques)
+   {
+     List<String[]> result = new ArrayList<String[]>();
+     final String target = urlLink.getTarget();
+     final String label = urlLink.getLabel();
+ 
+     // collect matching db-refs
+     DBRefEntry[] dbr = DBRefUtils.selectRefs(seq.getDBRefs(),
+             new String[] { target });
+     // collect id string too
+     String id = seq.getName();
+     String descr = seq.getDescription();
+     if (descr != null && descr.length() < 1)
+     {
+       descr = null;
+     }
+     if (dbr != null)
+     {
+       for (int r = 0; r < dbr.length; r++)
        {
-         for (int r = 0; r < dbr.length; r++)
+         if (id != null && dbr[r].getAccessionId().equals(id))
          {
-           if (id != null && dbr[r].getAccessionId().equals(id))
-           {
-             // suppress duplicate link creation for the bare sequence ID
-             // string with this link
-             id = null;
-           }
-           // create Bare ID link for this RUL
-           String[] urls = urlLink.makeUrls(dbr[r].getAccessionId(), true);
-           if (urls != null)
-           {
-             for (int u = 0; u < urls.length; u += 2)
-             {
-               String unq = urls[u] + "|" + urls[u + 1];
-               if (!uniques.contains(unq))
-               {
-                 urlSets.add(new String[] { target, label, urls[u],
-                     urls[u + 1] });
-                 uniques.add(unq);
-               }
-             }
-           }
+           // suppress duplicate link creation for the bare sequence ID
+           // string with this link
+           id = null;
          }
-       }
-       if (id != null)
-       {
-         // create Bare ID link for this RUL
-         String[] urls = urlLink.makeUrls(id, true);
+         // create Bare ID link for this URL
+         String[] urls = urlLink.makeUrls(dbr[r].getAccessionId(), true);
          if (urls != null)
          {
            for (int u = 0; u < urls.length; u += 2)
@@@ -323,66 -333,41 +394,57 @@@
            }
          }
        }
- 
      }
-     else
+     if (descr != null && urlLink.getRegexReplace() != null)
      {
-       String unq = label + "|" + urlLink.getUrl_prefix();
-       if (!uniques.contains(unq))
+       // create link for this URL from description only if regex matches
+       String[] urls = urlLink.makeUrls(descr, true);
+       if (urls != null)
        {
-         uniques.add(unq);
-         // Add a non-dynamic link
-         urlSets.add(new String[] { target, label, null,
-             urlLink.getUrl_prefix() });
+         for (int u = 0; u < urls.length; u += 2)
+         {
+           String unq = urls[u] + "|" + urls[u + 1];
+           if (!uniques.contains(unq))
+           {
+             result.add(new String[] { target, label, urls[u], urls[u + 1] });
+             uniques.add(unq);
+           }
+         }
        }
      }
- 
-     return urlSets.toArray(new String[][] {});
+     return result;
    }
  
-   public void createTooltipAnnotationReport(final StringBuilder tip,
 -  public void createSequenceAnnotationReport(final StringBuffer tip,
++  public void createSequenceAnnotationReport(final StringBuilder tip,
            SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
-           Hashtable minmax)
+           Map<String, float[][]> minmax)
    {
-     int maxWidth = createSequenceAnnotationReport(tip, sequence,
-             showDbRefs, showNpFeats, minmax, true);
- 
-     if (maxWidth > 60)
-     {
-       tip.insert(0, "<table width=350 border=0><tr><td><i>");
-       tip.append("</i></td></tr></table>");
-     }
-   }
- 
-   public int createSequenceAnnotationReport(final StringBuilder tip,
-           SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
-           Hashtable minmax)
-   {
-     return createSequenceAnnotationReport(tip, sequence, showDbRefs,
-             showNpFeats, minmax, false);
+     createSequenceAnnotationReport(tip, sequence, showDbRefs, showNpFeats,
 -            true, minmax);
++            minmax, false);
    }
  
 -  public void createSequenceAnnotationReport(final StringBuffer tip,
 +  /**
-    * Adds an html-formatted sequence annotation report to the provided string
-    * buffer, and returns the longest line length added
++   * Builds an html formatted report of sequence details and appends it to the
++   * provided buffer.
 +   * 
 +   * @param sb
++   *          buffer to append report to
 +   * @param sequence
++   *          the sequence the report is for
 +   * @param showDbRefs
-    *          if true, include database references
++   *          whether to include database references for the sequence
 +   * @param showNpFeats
-    *          if true, include non-positional sequence features
++   *          whether to include non-positional sequence features
 +   * @param minmax
 +   * @param summary
-    *          if true, build a shortened summary report (for tooltip)
 +   * @return
 +   */
 +  int createSequenceAnnotationReport(final StringBuilder sb,
            SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
-           Hashtable minmax, boolean summary)
 -          boolean tableWrap, Map<String, float[][]> minmax)
++          Map<String, float[][]> minmax, boolean summary)
    {
      String tmp;
 -    tip.append("<i>");
 +    sb.append("<i>");
  
      int maxWidth = 0;
      if (sequence.getDescription() != null)
@@@ -397,59 -382,18 +459,83 @@@
        ds = ds.getDatasetSequence();
      }
      DBRefEntry[] dbrefs = ds.getDBRefs();
-     Arrays.sort(dbrefs, comparator);
      if (showDbRefs && dbrefs != null)
      {
 -      for (int i = 0; i < dbrefs.length; i++)
++      // note this sorts the refs held on the sequence!
++      Arrays.sort(dbrefs, comparator);
 +      boolean ellipsis = false;
++      String source = null;
 +      String lastSource = null;
 +      int countForSource = 0;
++      int sourceCount = 0;
++      boolean moreSources = false;
++      int lineLength = 0;
++
 +      for (DBRefEntry ref : dbrefs)
 +      {
-         String source = ref.getSource();
++        source = ref.getSource();
 +        if (source == null)
 +        {
 +          // shouldn't happen
 +          continue;
 +        }
 +        boolean sourceChanged = !source.equals(lastSource);
 +        if (sourceChanged)
 +        {
++          lineLength = 0;
 +          countForSource = 0;
++          sourceCount++;
++        }
++        if (sourceCount > MAX_SOURCES && summary)
++        {
++          ellipsis = true;
++          moreSources = true;
++          break;
 +        }
 +        lastSource = source;
 +        countForSource++;
 +        if (countForSource == 1 || !summary)
 +        {
 +          sb.append("<br>");
 +        }
-         if (countForSource < 3 || !summary)
++        if (countForSource <= MAX_REFS_PER_SOURCE || !summary)
 +        {
 +          String accessionId = ref.getAccessionId();
-           int len = accessionId.length() + 1;
++          lineLength += accessionId.length() + 1;
 +          if (countForSource > 1 && summary)
 +          {
 +            sb.append(", ").append(accessionId);
-             len++;
++            lineLength++;
 +          }
 +          else
 +          {
 +            sb.append(source).append(" ").append(accessionId);
-             len += source.length();
++            lineLength += source.length();
 +          }
-           maxWidth = Math.max(maxWidth, len);
++          maxWidth = Math.max(maxWidth, lineLength);
 +        }
-         if (countForSource == 3 && summary)
++        if (countForSource == MAX_REFS_PER_SOURCE && summary)
 +        {
-           sb.append(", ...");
++          sb.append(COMMA).append(ELLIPSIS);
 +          ellipsis = true;
 +        }
 +      }
-       if (ellipsis) {
-         sb.append("<br>(Output Sequence Details to list all database references)");
++      if (moreSources)
+       {
 -        tip.append("<br>");
 -        tmp = dbrefs[i].getSource() + " " + dbrefs[i].getAccessionId();
 -        tip.append(tmp);
 -        maxWidth = Math.max(maxWidth, tmp.length());
++        sb.append("<br>").append(ELLIPSIS).append(COMMA).append(source)
++                .append(COMMA).append(ELLIPSIS);
++      }
++      if (ellipsis)
++      {
++        sb.append("<br>(");
++        sb.append(MessageManager.getString("label.output_seq_details"));
++        sb.append(")");
        }
      }
  
--    // ADD NON POSITIONAL SEQUENCE INFO
++    /*
++     * add non-positional features if wanted
++     */
      SequenceFeature[] features = sequence.getSequenceFeatures();
      if (showNpFeats && features != null)
      {
@@@ -457,15 -401,21 +543,29 @@@
        {
          if (features[i].begin == 0 && features[i].end == 0)
          {
 -          int sz = -tip.length();
 -          List<SequenceFeature> tfeat = new ArrayList<SequenceFeature>();
 -          tfeat.add(features[i]);
 -          appendFeatures(tip, 0, tfeat, minmax);
 -          sz += tip.length();
 +          int sz = -sb.length();
-           List<SequenceFeature> tfeat = Collections
-                   .singletonList(features[i]);
-           appendFeatures(sb, 0, tfeat, minmax);
++          appendFeature(sb, 0, minmax, features[i]);
 +          sz += sb.length();
            maxWidth = Math.max(maxWidth, sz);
          }
        }
      }
++    sb.append("</i>");
 +    return maxWidth;
 +  }
++
++  public void createTooltipAnnotationReport(final StringBuilder tip,
++          SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
++          Map<String, float[][]> minmax)
++  {
++    int maxWidth = createSequenceAnnotationReport(tip, sequence,
++            showDbRefs, showNpFeats, minmax, true);
+ 
 -    if (tableWrap && maxWidth > 60)
++    if (maxWidth > 60)
+     {
 -      tip.insert(0, "<table width=350 border=0><tr><td><i>");
 -      tip.append("</i></td></tr></table>");
++      // ? not sure this serves any useful purpose
++      // tip.insert(0, "<table width=350 border=0><tr><td>");
++      // tip.append("</td></tr></table>");
+     }
 -
+   }
  }
diff --cc test/jalview/io/SequenceAnnotationReportTest.java
index 0000000,a96a2a8..1392157
mode 000000,100644..100644
--- a/test/jalview/io/SequenceAnnotationReportTest.java
+++ b/test/jalview/io/SequenceAnnotationReportTest.java
@@@ -1,0 -1,185 +1,185 @@@
+ /*
+  * Jalview - A Sequence Alignment Editor and Viewer ($$Version-Rel$$)
+  * Copyright (C) $$Year-Rel$$ The Jalview Authors
+  * 
+  * This file is part of Jalview.
+  * 
+  * Jalview is free software: you can redistribute it and/or
+  * modify it under the terms of the GNU General Public License 
+  * as published by the Free Software Foundation, either version 3
+  * of the License, or (at your option) any later version.
+  *  
+  * Jalview is distributed in the hope that it will be useful, but 
+  * WITHOUT ANY WARRANTY; without even the implied warranty 
+  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR 
+  * PURPOSE.  See the GNU General Public License for more details.
+  * 
+  * You should have received a copy of the GNU General Public License
+  * along with Jalview.  If not, see <http://www.gnu.org/licenses/>.
+  * The Jalview Authors are detailed in the 'AUTHORS' file.
+  */
+ package jalview.io;
+ 
+ import static org.testng.AssertJUnit.assertEquals;
+ 
+ import jalview.datamodel.SequenceFeature;
+ 
+ import java.util.Hashtable;
+ import java.util.Map;
+ 
+ import org.testng.annotations.Test;
+ 
+ public class SequenceAnnotationReportTest
+ {
+   @Test(groups = "Functional")
+   public void testAppendFeature_disulfideBond()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     sb.append("123456");
+     SequenceFeature sf = new SequenceFeature("disulfide bond", "desc", 1,
+             3, 1.2f, "group");
+ 
+     // residuePos == 2 does not match start or end of feature, nothing done:
+     sar.appendFeature(sb, 2, null, sf);
+     assertEquals("123456", sb.toString());
+ 
+     // residuePos == 1 matches start of feature, text appended (but no <br>)
+     // feature score is not included
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("123456disulfide bond 1:3", sb.toString());
+ 
+     // residuePos == 3 matches end of feature, text appended
+     // <br> is prefixed once sb.length() > 6
+     sar.appendFeature(sb, 3, null, sf);
+     assertEquals("123456disulfide bond 1:3<br>disulfide bond 1:3",
+             sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_status()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3,
+             Float.NaN, "group");
+     sf.setStatus("Confirmed");
+ 
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Fe2-S; (Confirmed)", sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_withScore()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3, 1.3f,
+             "group");
+ 
+     Map<String, float[][]> minmax = new Hashtable<String, float[][]>();
+     sar.appendFeature(sb, 1, minmax, sf);
+     /*
+      * map has no entry for this feature type - score is not shown:
+      */
+     assertEquals("METAL 1 3; Fe2-S", sb.toString());
+ 
+     /*
+      * map has entry for this feature type - score is shown:
+      */
+     minmax.put("METAL", new float[][] { { 0f, 1f }, null });
+     sar.appendFeature(sb, 1, minmax, sf);
+     // <br> is appended to a buffer > 6 in length
+     assertEquals("METAL 1 3; Fe2-S<br>METAL 1 3; Fe2-S Score=1.3",
+             sb.toString());
+ 
+     /*
+      * map has min == max for this feature type - score is not shown:
+      */
+     minmax.put("METAL", new float[][] { { 2f, 2f }, null });
+     sb.setLength(0);
+     sar.appendFeature(sb, 1, minmax, sf);
+     assertEquals("METAL 1 3; Fe2-S", sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_noScore()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3,
+             Float.NaN, "group");
+ 
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Fe2-S", sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_clinicalSignificance()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3,
+             Float.NaN, "group");
+     sf.setValue("clinical_significance", "Benign");
+ 
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Fe2-S; Benign", sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_withScoreStatusClinicalSignificance()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3, 1.3f,
+             "group");
+     sf.setStatus("Confirmed");
+     sf.setValue("clinical_significance", "Benign");
+     Map<String, float[][]> minmax = new Hashtable<String, float[][]>();
+     minmax.put("METAL", new float[][] { { 0f, 1f }, null });
+     sar.appendFeature(sb, 1, minmax, sf);
+ 
+     assertEquals("METAL 1 3; Fe2-S Score=1.3; (Confirmed); Benign",
+             sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_DescEqualsType()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "METAL", 1, 3,
+             Float.NaN, "group");
+ 
+     // description is not included if it duplicates type:
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3", sb.toString());
+ 
+     sb.setLength(0);
+     sf.setDescription("Metal");
+     // test is case-sensitive:
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Metal", sb.toString());
+   }
+ 
+   @Test(groups = "Functional")
+   public void testAppendFeature_stripHtml()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL",
+             "<html><body>hello<em>world</em></body></html>", 1, 3,
+             Float.NaN, "group");
+ 
+     sar.appendFeature(sb, 1, null, sf);
+     // !! strips off </body> but not <body> ??
+     assertEquals("METAL 1 3; <body>hello<em>world</em>", sb.toString());
+ 
+     sb.setLength(0);
+     sf.setDescription("<br>&kHD>6");
+     sar.appendFeature(sb, 1, null, sf);
+     // if no <html> tag, html-encodes > and < (only):
+     assertEquals("METAL 1 3; &lt;br&gt;&kHD&gt;6", sb.toString());
+   }
+ }