Merge branch 'features/pca_jaxb_datasetrefs_JAL-3171_JAL-3063_JAL-1767' into develop
authorJim Procter <jprocter@issues.jalview.org>
Wed, 16 Jan 2019 11:38:15 +0000 (11:38 +0000)
committerJim Procter <jprocter@issues.jalview.org>
Wed, 16 Jan 2019 11:38:15 +0000 (11:38 +0000)
43 files changed:
.gitignore
help/html/features/featuresFormat.html
help/html/releases.html
resources/lang/Messages.properties
resources/lang/Messages_es.properties
src/jalview/api/FeatureRenderer.java
src/jalview/appletgui/AppletJmolBinding.java
src/jalview/appletgui/ExtJmol.java
src/jalview/appletgui/OverviewCanvas.java
src/jalview/appletgui/OverviewPanel.java
src/jalview/commands/EditCommand.java
src/jalview/datamodel/Sequence.java
src/jalview/datamodel/SequenceGroup.java
src/jalview/datamodel/SequenceI.java
src/jalview/datamodel/features/FeatureStore.java
src/jalview/datamodel/features/SequenceFeatures.java
src/jalview/datamodel/features/SequenceFeaturesI.java
src/jalview/ext/jmol/JalviewJmolBinding.java
src/jalview/gui/FeatureSettings.java
src/jalview/gui/FeatureTypeSettings.java
src/jalview/gui/OverviewCanvas.java
src/jalview/gui/ScalePanel.java
src/jalview/schemes/FeatureColour.java
src/jalview/ws/DBRefFetcher.java
test/jalview/commands/EditCommandTest.java
test/jalview/datamodel/SequenceTest.java
test/jalview/datamodel/features/FeatureStoreTest.java
test/jalview/datamodel/features/SequenceFeaturesTest.java
test/jalview/ext/htsjdk/TestHtsContigDb.java
test/jalview/ext/jmol/JmolParserTest.java
test/jalview/ext/rbvi/chimera/ChimeraConnect.java
test/jalview/gui/AlignViewportTest.java
test/jalview/gui/AlignmentPanelTest.java
test/jalview/gui/FreeUpMemoryTest.java
test/jalview/gui/ScalePanelTest.java [new file with mode: 0644]
test/jalview/io/IdentifyFileTest.java
test/jalview/io/cache/JvCacheableInputBoxTest.java
test/jalview/schemes/FeatureColourTest.java
test/jalview/viewmodel/styles/ViewStyleTest.java
utils/checkstyle/README.txt
utils/checkstyle/checkstyle-suppress.xml
utils/checkstyle/checkstyle.xml
utils/checkstyle/import-control.xml

index 86637df..d355a80 100644 (file)
@@ -14,3 +14,4 @@ TESTNG
 /jalviewApplet.jar
 /benchmarking/lib
 *.class
+/site
index fd6b99f..4d13dcd 100755 (executable)
     contains tab separated text fields. <strong>No comments are
       allowed</strong>.
   </p>
-  <p>The first set of lines contain type definitions:
+  <p>
+    <strong>Feature Colours</strong>
+  </p>
+  <p>The first set of lines contain feature type definitions and their colours:
   <pre>
 <strong><em>Feature label</em>&#9;<em>Feature Colour</em>
 <!-- &#9;<em>Feature links</em>  --></strong>
   <ul>
     <li>A single colour specified as either a red,green,blue 24 bit
       triplet in hexadecimal (eg. 00ff00) or as comma separated numbers
-      (ranging from 0 to 255))</li>
+      (ranging from 0 to 255))<br>
+      (For help with colour values, see <a href="https://www.w3schools.com/colors/colors_converter.asp">https://www.w3schools.com/colors/colors_converter.asp</a>.)</li>
 
     <li>A <a href="featureschemes.html">graduated colourscheme</a>
       specified as a "|" separated list of fields: <pre>
-[label|]&lt;mincolor&gt;|&lt;maxcolor&gt;|[absolute|]&lt;minvalue&gt;|&lt;maxvalue&gt;[|&lt;thresholdtype&gt;|[&lt;threshold value&gt;]]
+[label <em>or</em> score<em> or</em> attribute|attName|]&lt;mincolor&gt;|&lt;maxcolor&gt;|[absolute|]&lt;minvalue&gt;|&lt;maxvalue&gt;[|&lt;novalue&gt;][|&lt;thresholdtype&gt;|[&lt;threshold value&gt;]]
 </pre> The fields are as follows:
 
       <ul>
-        <li><em>label</em><br> Indicate that the feature
+        <li><em>label</em><br> Indicates that the feature
           description should be used to create a colour for features of
           this type.<br> <em>Note: if no threshold value is
-            needed then the final '|' may be omitted.<br> This
+            needed then only 'label' is required.<br> This
             keyword was added in Jalview 2.6
         </em></li>
 
+        <li><em>score</em><br> Indicates that the feature
+          score should be used to create a graduated colour for features of
+          this type, in conjunction with mincolor, maxcolor.<br><em>This keyword was added in Jalview 2.11.
+          It may be omitted (score is the default) if mincolor and maxcolor are specified.
+        </em></li>
+
+        <li><em>attribute|attName</em><br> Indicates that the value of feature
+          attribute 'attName' should be used to create a colour for features of
+          this type.
+          <br>For example, <em>attribute|clinical_significance</em> to colour by clinical_significance.
+          <br>To colour by range of a numeric attribute, include <em>mincolor</em> and <em>maxcolor</em>, or omit to colour by text (category).
+          <br>(Note: the value of the attribute used for colouring will also be shown in the tooltip as you mouse over features.)
+          <br>A sub-attribute should be written as, for example, CSQ:IMPACT.
+          <br><em>This keyword was added in Jalview 2.11</em></li>
+
         <li><em>mincolor</em> and <em>maxcolor</em><br> Colour
           triplets specified as hexadecimal or comma separated values
           (may be left blank for a <em>label</em> style colourscheme,
 
         <li><em>minvalue</em> and <em>maxvalue</em><br>
           Minimum and maximum values defining the range of scores for
-          which the colour range will be defined over. If minvalue is
+          which the colour range will be defined over.<br>If minvalue is
           greater than maxvalue then the linear mapping will have
           negative gradient.</li>
 
+        <li><em>novalue</em> <br>
+          Specifies the colour to use if colouring by attribute, when the attribute is absent.
+          Valid options are <em>novaluemin, novaluemax, novaluenone</em>, to use mincolor, maxcolor, or no colour.
+          <br>If not specified this will default to novaluemin.</li>
+
         <li><em>thresholdtype</em><br> Either
           &quot;none&quot;, &quot;below&quot;, or &quot;above&quot;. <em>below</em>
           and <em>above</em> require an additional <em>threshold
   </ul>
   </p>
 
+  <p>
+    <strong>Feature Filters</strong>
+  </p>
+  <p>This section is optional, and allows one or more filters to be defined for each feature type.
+     <br>Only features that satisfy the filter conditions will be displayed.
+     <br>Begin with a line which is just STARTFILTERS, and end with a line which is just ENDFILTERS.
+     <br>Each line has the format:
+     <pre>featureType <em>&lt;tab&gt;</em> (filtercondition1) [and|or] (filtercondition2) [and|or]...<br></pre>
+     The parentheses are not needed if there is only one condition. 
+     Combine multiple conditions with either <em>and</em> or <em>or</em> (but not a mixture).
+     <br>Each condition is written as:
+     <pre>Label <em>or</em> Score <em>or</em> AttributeName condition [value]</pre>
+     where either the label (description), (numeric) score, or (text or numeric) attribute is tested against the condition.
+     <br><em>condition</em> is not case sensitive, and should be one of
+     <ul>
+     <li><em>Contains</em> - description (or attribute) should contain the given value (not case sensitive); example <em>clinical_significance contains Pathogenic</em></li> 
+     <li><em>NotContains</em> - description (or attribute) should not contain the given value</li> 
+     <li><em>Matches</em> - description (or attribute)  should match the given value (not case sensitive)</li> 
+     <li><em>NotMatches</em> - description (or attribute) should not match the given value (not case sensitive)</li> 
+     <li><em>Present</em> - attribute is present on the feature (no value required); example <em>CSQ:SIFT present</em></li> 
+     <li><em>NotPresent</em> - attribute is not present on the feature (no value required)</li> 
+     <li><em>EQ</em> - feature score, or specified attribute, is equal to the (numeric) value</li> 
+     <li><em>NE, LT, LE, GT, GE</em> - tests for not equal to / less than / less than or equal to / greater than / greater than or equal to the value</li> 
+     </ul>
+     A non-numeric value always fails a numeric test.<br>If either attribute name, or value to compare, contains spaces, then enclose in single quotes:
+     <em>'mutagenesis site' contains 'decreased affinity'</em>
+     <br>Tip: some examples of filter syntax are given below; or to see more, first configure colours and filters in Jalview, then <em>File | Export Features</em> to Textbox in Jalview Format.
+     <br><em>Feature filters were added in Jalview 2.11</em>
+  </p>
+
+  <p>
+    <strong>Feature Instances</strong>
+  </p>
+
   <p>The remaining lines in the file are the sequence annotation
     definitions, where the now defined features are attached to regions
     on particular sequences. Each feature can optionally include some
     descriptive text which is displayed in a tooltip when the mouse is
-    near the feature on that sequence (and can also be used to generate
-    a colour the feature).</p>
+    near the feature on that sequence (and may also be used to generate
+    a colour for the feature).</p>
 
   <p>
     If your sequence annotation is already available in <a href="http://gmod.org/wiki/GFF2">GFF2</a> (http://gmod.org/wiki/GFF2) or
@@ -204,6 +262,13 @@ signal peptide&#9;0,155,165
 helix&#9;ff0000
 strand&#9;00ff00
 coil&#9;cccccc
+kdHydrophobicity&#9;ccffcc|333300|-3.9|4.5|above|-2.0
+
+STARTFILTERS
+metal ion-binding site&#9;Label Contains sulfur
+kdHydrophobicity&#9;(Score LT 1.5) OR (Score GE 2.8)
+ENDFILTERS
+
 Your Own description here&#9;FER_CAPAA&#9;-1&#9;3&#9;93&#9;domain
 Your Own description here&#9;FER_CAPAN&#9;-1&#9;48&#9;144&#9;chain
 Your Own description here&#9;FER_CAPAN&#9;-1&#9;50&#9;140&#9;domain
@@ -211,10 +276,16 @@ Your Own description here&#9;FER_CAPAN&#9;-1&#9;136&#9;136&#9;modified residue
 Your Own description here&#9;FER1_LYCES&#9;-1&#9;1&#9;47&#9;transit peptide
 Your Own description here&#9;Q93XJ9_SOLTU&#9;-1&#9;1&#9;48&#9;signal peptide
 Your Own description here&#9;Q93XJ9_SOLTU&#9;-1&#9;49&#9;144&#9;chain
-startgroup&#9;secondarystucture
+
+STARTGROUP&#9;secondarystucture
 PDB secondary structure annotation&#9;FER1_SPIOL&#9;-1&#9;52&#9;59&#9;strand
 PDB secondary structure annotation&#9;FER1_SPIOL&#9;-1&#9;74&#9;80&#9;helix
-endgroup&#9;secondarystructure
+ENDGROUP&#9;secondarystructure
+
+STARTGROUP&#9;kd
+Hydrophobicity score by kD     Q93XJ9_SOLTU    -1      48      48      kdHydrophobicity        1.8
+ENDGROUP&#9;kd
+
 GFF
 FER_CAPAA&#9;GffGroup&#9;domain&#9;3&#9;93&#9;.&#9;.
 </pre>
index 3eaf234..0fba08a 100755 (executable)
@@ -71,25 +71,64 @@ li:before {
       <td width="60" nowrap>
         <div align="center">
           <strong><a name="Jalview.2.11.0">2.11.0</a><br />
-            <em>8/09/2018</em></strong>
+            <em>29/01/2019</em></strong>
         </div>
       </td>
-      <td><div align="left">
-          <em></em>
-          <ul>
-            <li>
-              <!--  -->
-            </li>
-          </ul>
-        </div></td>
-      <td><div align="left">
-          <em></em>
-          <ul>
-            <li>
-              <!-- JAL-3035 -->DAS sequence retrieval and annotation capabilities removed from the Jalview Desktop
-            </li>
-          </ul>
-        </div></td>
+    <td><div align="left">
+        <em>Deprecations</em>
+        <ul>
+          <li>
+            <!-- JAL-3035 -->DAS sequence retrieval and annotation
+            capabilities removed from the Jalview Desktop
+          </li>
+        </ul>
+        <em>Release Processes</em>
+        <ul>
+        <li>Atlassian Bamboo continuous integration server for unattended Test Suite execution</li>
+        <li><!-- JAL-2864 -->Memory test suite to detect leaks in common operations</li> 
+        </ul>
+      </div></td>
+    <td><div align="left">
+        <em></em>
+        <ul>
+          <li>
+            <!-- JAL-2865 -->Jalview hangs when closing windows
+            or the overview updates with large alignments.
+          </li>
+          <li>
+            <!-- JAL-2865 -->Tree and PCA calculation fails for selected
+            region if columns were selected by dragging right-to-left
+            and the mouse moved to the left of the first column.
+          </li>
+          <li>
+            <!-- JAL-2846 -->Error message for trying to load in invalid
+            URLs doesn't tell users the invalid URL
+          </li>
+        </ul>
+        <em>Editing</em>
+        <ul>
+          <li>
+            <!-- JAL-2822 -->Start and End should be updated when
+            sequence data at beginning or end of alignment added/removed
+            via 'Edit' sequence
+          </li>
+          <li>
+            <!-- JAL-2541 -->Delete/Cut selection doesn't relocate
+            sequence features correctly when start of sequence is
+            removed (Known defect since 2.10)
+          </li>
+          <li>
+            <!-- JAL- -->
+          </li>
+        </ul>
+        <em>New Known Defects</em>
+        <ul>
+          <li>
+            <!-- JAL-3178 -->Nonpositional features lose feature group
+            on export as jalview features file
+          </li>
+        </ul>
+      </div></td>
     </tr>
     <tr>
     <td width="60" nowrap>
index 449c9b6..43e055d 100644 (file)
@@ -415,7 +415,7 @@ label.input_alignment_from_url = Input Alignment From URL
 label.input_alignment = Input Alignment
 label.couldnt_import_as_vamsas_session = Couldn't import {0} as a new vamsas session.
 label.vamsas_document_import_failed = Vamsas Document Import Failed
-label.couldnt_locate = Couldn't locate {0}
+label.couldnt_locate = Could not locate {0}
 label.url_not_found = URL not found
 label.new_sequence_url_link = New sequence URL link
 label.cannot_edit_annotations_in_wrapped_view = Cannot edit annotations in wrapped view
@@ -1282,7 +1282,6 @@ label.SEQUENCE_ID_for_DB_ACCESSION1 = Please review your URL links in the 'Conne
 label.SEQUENCE_ID_for_DB_ACCESSION2 = URL links using '$SEQUENCE_ID$' for DB accessions now use '$DB_ACCESSION$'.
 label.do_not_display_again = Do not display this message again
 exception.url_cannot_have_duplicate_id = {0} cannot be used as a label for more than one line
-label.filter = Filter text:
 action.customfilter = Custom only
 action.showall = Show All
 label.insert = Insert:
@@ -1350,7 +1349,6 @@ label.colour_by_text = Colour by text
 label.graduated_colour = Graduated Colour
 label.by_text_of = By text of
 label.by_range_of = By range of
-label.filters_tooltip = Click to set or amend filters
 label.or = Or
 label.and = And
 label.sequence_feature_colours = Sequence Feature Colours
@@ -1361,3 +1359,5 @@ label.most_bound_molecules = Most Bound Molecules
 label.most_polymer_residues = Most Polymer Residues
 label.cached_structures = Cached Structures
 label.free_text_search = Free Text Search
+label.configuration = Configuration
+label.configure_feature_tooltip = Click to configure variable colour or filters
index 3c82386..5aed0cb 100644 (file)
@@ -1283,7 +1283,6 @@ label.SEQUENCE_ID_for_DB_ACCESSION1 = Por favor, revise sus URLs en la pesta
 label.SEQUENCE_ID_for_DB_ACCESSION2 = URL enlaza usando '$SEQUENCE_ID$' para accesiones DB ahora usar '$DB_ACCESSION$'.
 label.do_not_display_again = No mostrar este mensaje de nuevo
 exception.url_cannot_have_duplicate_id = {0} no puede ser usada como etiqueta en más de un enlace
-label.filter = Filtrar texto:
 action.customfilter = Sólo personalizado
 action.showall = Mostrar todo
 label.insert = Insertar:
@@ -1351,7 +1350,6 @@ label.colour_by_text = Colorear por texto
 label.graduated_colour = Color graduado
 label.by_text_of = Por texto de
 label.by_range_of = Por rango de
-label.filters_tooltip = Haga clic para configurar o modificar los filtros
 label.or = O
 label.and = Y
 label.sequence_feature_colours = Colores de características de las secuencias
@@ -1362,3 +1360,5 @@ label.most_bound_molecules = M
 label.most_polymer_residues = Más Residuos de Polímeros
 label.cached_structures = Estructuras en Caché
 label.free_text_search = Búsqueda de texto libre
+label.configuration = Configuración
+label.configure_feature_tooltip = Haga clic para configurar el color o los filtros
index cf3c8da..868f196 100644 (file)
@@ -255,11 +255,11 @@ public interface FeatureRenderer
    * <p>
    * Returns null if
    * <ul>
-   * <li>feature type is not visible, or</li>
    * <li>feature group is not visible, or</li>
    * <li>feature values lie outside any colour threshold, or</li>
    * <li>feature is excluded by filter conditions</li>
    * </ul>
+   * This method does not check feature type visibility.
    * 
    * @param feature
    * @return
index 2f61b24..e5767b6 100644 (file)
@@ -24,7 +24,6 @@ import jalview.api.AlignmentViewPanel;
 import jalview.datamodel.PDBEntry;
 import jalview.datamodel.SequenceI;
 import jalview.ext.jmol.JalviewJmolBinding;
-import jalview.gui.IProgressIndicator;
 import jalview.io.DataSourceType;
 import jalview.structure.StructureSelectionManager;
 
@@ -174,21 +173,12 @@ class AppletJmolBinding extends JalviewJmolBinding
   @Override
   public int[] resizeInnerPanel(String data)
   {
-    // TODO Auto-generated method stub
     return null;
   }
 
   @Override
   public Map<String, Object> getJSpecViewProperty(String arg0)
   {
-    // TODO Auto-generated method stub
-    return null;
-  }
-
-  @Override
-  protected IProgressIndicator getIProgressIndicator()
-  {
-    // no progress indicators on the applet
     return null;
   }
 }
index 89228d5..b0d3f7a 100644 (file)
@@ -26,7 +26,6 @@ import jalview.api.SequenceRenderer;
 import jalview.datamodel.PDBEntry;
 import jalview.datamodel.SequenceI;
 import jalview.ext.jmol.JalviewJmolBinding;
-import jalview.gui.IProgressIndicator;
 import jalview.io.DataSourceType;
 
 import java.awt.Container;
@@ -66,18 +65,8 @@ public class ExtJmol extends JalviewJmolBinding
   }
 
   @Override
-  protected IProgressIndicator getIProgressIndicator()
-  {
-    // no progress indicators on applet (could access javascript for this)
-    return null;
-  }
-
-  @Override
   public void updateColours(Object source)
   {
-
-    // TODO Auto-generated method stub
-
   }
 
   @Override
@@ -190,7 +179,6 @@ public class ExtJmol extends JalviewJmolBinding
   protected JmolAppConsoleInterface createJmolConsole(
           Container consolePanel, String buttonsToShow)
   {
-    // TODO Auto-generated method stub
     return null;
   }
 
@@ -205,14 +193,11 @@ public class ExtJmol extends JalviewJmolBinding
   @Override
   public void releaseReferences(Object svl)
   {
-    // TODO Auto-generated method stub
-
   }
 
   @Override
   public Map<String, Object> getJSpecViewProperty(String arg0)
   {
-    // TODO Auto-generated method stub
     return null;
   }
 
index e99c021..07f5919 100644 (file)
@@ -162,4 +162,12 @@ public class OverviewCanvas extends Component
     }
   }
 
+  /**
+   * Nulls references to protect against potential memory leaks
+   */
+  void dispose()
+  {
+    od = null;
+  }
+
 }
index 3bbbe95..5081509 100755 (executable)
@@ -334,6 +334,10 @@ public class OverviewPanel extends Panel implements Runnable,
     } finally
     {
       av = null;
+      if (oviewCanvas != null)
+      {
+        oviewCanvas.dispose();
+      }
       oviewCanvas = null;
       ap = null;
       od = null;
index cac843f..1a227c5 100644 (file)
  */
 package jalview.commands;
 
+import jalview.analysis.AlignSeq;
 import jalview.datamodel.AlignmentAnnotation;
 import jalview.datamodel.AlignmentI;
 import jalview.datamodel.Annotation;
+import jalview.datamodel.Range;
 import jalview.datamodel.Sequence;
 import jalview.datamodel.SequenceFeature;
 import jalview.datamodel.SequenceI;
+import jalview.datamodel.features.SequenceFeaturesI;
+import jalview.util.Comparison;
 import jalview.util.ReverseListIterator;
 import jalview.util.StringUtils;
 
@@ -114,7 +118,7 @@ public class EditCommand implements CommandI
     public abstract Action getUndoAction();
   };
 
-  private List<Edit> edits = new ArrayList<Edit>();
+  private List<Edit> edits = new ArrayList<>();
 
   String description;
 
@@ -330,20 +334,8 @@ public class EditCommand implements CommandI
           int position, int number, AlignmentI al, boolean performEdit,
           AlignmentI[] views)
   {
-    Edit edit = new Edit(command, seqs, position, number,
-            al.getGapCharacter());
-    if (al.getHeight() == seqs.length)
-    {
-      edit.al = al;
-      edit.fullAlignmentHeight = true;
-    }
-
-    addEdit(edit);
-
-    if (performEdit)
-    {
-      performEdit(edit, views);
-    }
+    Edit edit = new Edit(command, seqs, position, number, al);
+    appendEdit(edit, al, performEdit, views);
   }
 
   /**
@@ -534,39 +526,62 @@ public class EditCommand implements CommandI
         command.string[i] = sequence.getSequence(command.position,
                 command.position + command.number);
         SequenceI oldds = sequence.getDatasetSequence();
-        if (command.oldds != null && command.oldds[i] != null)
-        {
-          // we are redoing an undone cut.
-          sequence.setDatasetSequence(null);
-        }
-        sequence.deleteChars(command.position,
+        Range cutPositions = sequence.findPositions(command.position + 1,
                 command.position + command.number);
+        boolean cutIsInternal = cutPositions != null
+                && sequence.getStart() != cutPositions
+                .getBegin() && sequence.getEnd() != cutPositions.getEnd();
+
+        /*
+         * perform the cut; if this results in a new dataset sequence, add
+         * that to the alignment dataset
+         */
+        SequenceI ds = sequence.getDatasetSequence();
+        sequence.deleteChars(command.position, command.position
+                + command.number);
+
         if (command.oldds != null && command.oldds[i] != null)
         {
-          // oldds entry contains the cut dataset sequence.
+          /*
+           * we are Redoing a Cut, or Undoing a Paste - so
+           * oldds entry contains the cut dataset sequence,
+           * with sequence features in expected place
+           */
           sequence.setDatasetSequence(command.oldds[i]);
           command.oldds[i] = oldds;
         }
         else
         {
-          // modify the oldds if necessary
+          /* 
+           * new cut operation: save the dataset sequence 
+           * so it can be restored in an Undo
+           */
+          if (command.oldds == null)
+          {
+            command.oldds = new SequenceI[command.seqs.length];
+          }
+          command.oldds[i] = oldds;// todo not if !cutIsInternal?
+
+          // do we need to edit sequence features for new sequence ?
           if (oldds != sequence.getDatasetSequence()
-                  || sequence.getFeatures().hasFeatures())
+                  || (cutIsInternal
+                          && sequence.getFeatures().hasFeatures()))
+          // todo or just test cutIsInternal && cutPositions != null ?
           {
-            if (command.oldds == null)
+            if (cutPositions != null)
             {
-              command.oldds = new SequenceI[command.seqs.length];
+              cutFeatures(command, sequence, cutPositions.getBegin(),
+                              cutPositions.getEnd(), cutIsInternal);
             }
-            command.oldds[i] = oldds;
-            // FIXME JAL-2541 JAL-2526 get correct positions if on a gap
-            adjustFeatures(
-                    command,
-                    i,
-                    sequence.findPosition(command.position),
-                    sequence.findPosition(command.position + command.number),
-                    false);
           }
         }
+        SequenceI newDs = sequence.getDatasetSequence();
+        if (newDs != ds && command.al != null
+                && command.al.getDataset() != null
+                && !command.al.getDataset().getSequences().contains(newDs))
+        {
+          command.al.getDataset().addSequence(newDs);
+        }
       }
 
       if (sequence.getLength() < 1)
@@ -588,21 +603,19 @@ public class EditCommand implements CommandI
    */
   static void paste(Edit command, AlignmentI[] views)
   {
-    StringBuffer tmp;
-    boolean newDSNeeded;
-    boolean newDSWasNeeded;
-    int newstart, newend;
     boolean seqWasDeleted = false;
-    int start = 0, end = 0;
 
     for (int i = 0; i < command.seqs.length; i++)
     {
-      newDSNeeded = false;
-      newDSWasNeeded = command.oldds != null && command.oldds[i] != null;
-      if (command.seqs[i].getLength() < 1)
+      boolean newDSNeeded = false;
+      boolean newDSWasNeeded = command.oldds != null
+              && command.oldds[i] != null;
+      SequenceI sequence = command.seqs[i];
+      if (sequence.getLength() < 1)
       {
-        // ie this sequence was deleted, we need to
-        // readd it to the alignment
+        /*
+         * sequence was deleted; re-add it to the alignment
+         */
         if (command.alIndex[i] < command.al.getHeight())
         {
           List<SequenceI> sequences;
@@ -610,68 +623,78 @@ public class EditCommand implements CommandI
           {
             if (!(command.alIndex[i] < 0))
             {
-              sequences.add(command.alIndex[i], command.seqs[i]);
+              sequences.add(command.alIndex[i], sequence);
             }
           }
         }
         else
         {
-          command.al.addSequence(command.seqs[i]);
+          command.al.addSequence(sequence);
         }
         seqWasDeleted = true;
       }
-      newstart = command.seqs[i].getStart();
-      newend = command.seqs[i].getEnd();
+      int newStart = sequence.getStart();
+      int newEnd = sequence.getEnd();
 
-      tmp = new StringBuffer();
-      tmp.append(command.seqs[i].getSequence());
+      StringBuilder tmp = new StringBuilder();
+      tmp.append(sequence.getSequence());
       // Undo of a delete does not replace original dataset sequence on to
       // alignment sequence.
 
+      int start = 0;
+      int length = 0;
+
       if (command.string != null && command.string[i] != null)
       {
         if (command.position >= tmp.length())
         {
           // This occurs if padding is on, and residues
           // are removed from end of alignment
-          int length = command.position - tmp.length();
-          while (length > 0)
+          int len = command.position - tmp.length();
+          while (len > 0)
           {
             tmp.append(command.gapChar);
-            length--;
+            len--;
           }
         }
         tmp.insert(command.position, command.string[i]);
         for (int s = 0; s < command.string[i].length; s++)
         {
-          if (jalview.schemes.ResidueProperties.aaIndex[command.string[i][s]] != 23)
+          if (!Comparison.isGap(command.string[i][s]))
           {
+            length++;
             if (!newDSNeeded)
             {
               newDSNeeded = true;
-              start = command.seqs[i].findPosition(command.position);
-              end = command.seqs[i]
-                      .findPosition(command.position + command.number);
+              start = sequence.findPosition(command.position);
+              // end = sequence
+              // .findPosition(command.position + command.number);
             }
-            if (command.seqs[i].getStart() == start)
+            if (sequence.getStart() == start)
             {
-              newstart--;
+              newStart--;
             }
             else
             {
-              newend++;
+              newEnd++;
             }
           }
         }
         command.string[i] = null;
       }
 
-      command.seqs[i].setSequence(tmp.toString());
-      command.seqs[i].setStart(newstart);
-      command.seqs[i].setEnd(newend);
+      sequence.setSequence(tmp.toString());
+      sequence.setStart(newStart);
+      sequence.setEnd(newEnd);
+
+      /*
+       * command and Undo share the same dataset sequence if cut was
+       * at start or end of sequence
+       */
+      boolean sameDatasetSequence = false;
       if (newDSNeeded)
       {
-        if (command.seqs[i].getDatasetSequence() != null)
+        if (sequence.getDatasetSequence() != null)
         {
           SequenceI ds;
           if (newDSWasNeeded)
@@ -682,21 +705,29 @@ public class EditCommand implements CommandI
           {
             // make a new DS sequence
             // use new ds mechanism here
-            ds = new Sequence(command.seqs[i].getName(),
-                    jalview.analysis.AlignSeq.extractGaps(
-                            jalview.util.Comparison.GapChars,
-                            command.seqs[i].getSequenceAsString()),
-                    command.seqs[i].getStart(), command.seqs[i].getEnd());
-            ds.setDescription(command.seqs[i].getDescription());
+            String ungapped = AlignSeq.extractGaps(Comparison.GapChars,
+                    sequence.getSequenceAsString());
+            ds = new Sequence(sequence.getName(), ungapped,
+                    sequence.getStart(), sequence.getEnd());
+            ds.setDescription(sequence.getDescription());
           }
           if (command.oldds == null)
           {
             command.oldds = new SequenceI[command.seqs.length];
           }
-          command.oldds[i] = command.seqs[i].getDatasetSequence();
-          command.seqs[i].setDatasetSequence(ds);
+          command.oldds[i] = sequence.getDatasetSequence();
+          sameDatasetSequence = ds == sequence.getDatasetSequence();
+          ds.setSequenceFeatures(sequence.getSequenceFeatures());
+          if (!sameDatasetSequence && command.al.getDataset() != null)
+          {
+            // delete 'undone' sequence from alignment dataset
+            command.al.getDataset()
+                    .deleteSequence(sequence.getDatasetSequence());
+          }
+          sequence.setDatasetSequence(ds);
         }
-        adjustFeatures(command, i, start, end, true);
+        undoCutFeatures(command, command.seqs[i], start, length,
+                sameDatasetSequence);
       }
     }
     adjustAnnotations(command, true, seqWasDeleted, views);
@@ -706,7 +737,7 @@ public class EditCommand implements CommandI
 
   static void replace(Edit command)
   {
-    StringBuffer tmp;
+    StringBuilder tmp;
     String oldstring;
     int start = command.position;
     int end = command.number;
@@ -719,6 +750,7 @@ public class EditCommand implements CommandI
     {
       boolean newDSWasNeeded = command.oldds != null
               && command.oldds[i] != null;
+      boolean newStartEndWasNeeded = command.oldStartEnd!=null && command.oldStartEnd[i]!=null;
 
       /**
        * cut addHistoryItem(new EditCommand("Cut Sequences", EditCommand.CUT,
@@ -731,49 +763,147 @@ public class EditCommand implements CommandI
        * EditCommand.PASTE, sequences, 0, alignment.getWidth(), alignment) );
        * 
        */
+
+      Range beforeEditedPositions = command.seqs[i].findPositions(1, start);
+      Range afterEditedPositions = command.seqs[i]
+              .findPositions(end + 1, command.seqs[i].getLength());
+      
       oldstring = command.seqs[i].getSequenceAsString();
-      tmp = new StringBuffer(oldstring.substring(0, start));
+      tmp = new StringBuilder(oldstring.substring(0, start));
       tmp.append(command.string[i]);
-      String nogaprep = jalview.analysis.AlignSeq.extractGaps(
-              jalview.util.Comparison.GapChars,
+      String nogaprep = AlignSeq.extractGaps(Comparison.GapChars,
               new String(command.string[i]));
-      int ipos = command.seqs[i].findPosition(start)
-              - command.seqs[i].getStart();
-      tmp.append(oldstring.substring(end));
+      if (end < oldstring.length())
+      {
+        tmp.append(oldstring.substring(end));
+      }
+      // stash end prior to updating the sequence object so we can save it if
+      // need be.
+      Range oldstartend = new Range(command.seqs[i].getStart(),
+              command.seqs[i].getEnd());
       command.seqs[i].setSequence(tmp.toString());
-      command.string[i] = oldstring.substring(start, end).toCharArray();
-      String nogapold = jalview.analysis.AlignSeq.extractGaps(
-              jalview.util.Comparison.GapChars,
+      command.string[i] = oldstring
+              .substring(start, Math.min(end, oldstring.length()))
+              .toCharArray();
+      String nogapold = AlignSeq.extractGaps(Comparison.GapChars,
               new String(command.string[i]));
+
       if (!nogaprep.toLowerCase().equals(nogapold.toLowerCase()))
       {
-        if (newDSWasNeeded)
+        // we may already have dataset and limits stashed...
+        if (newDSWasNeeded || newStartEndWasNeeded)
         {
+          if (newDSWasNeeded)
+          {
+          // then just switch the dataset sequence
           SequenceI oldds = command.seqs[i].getDatasetSequence();
           command.seqs[i].setDatasetSequence(command.oldds[i]);
           command.oldds[i] = oldds;
+          }
+          if (newStartEndWasNeeded)
+          {
+            Range newStart = command.oldStartEnd[i];
+            command.oldStartEnd[i] = oldstartend;
+            command.seqs[i].setStart(newStart.getBegin());
+            command.seqs[i].setEnd(newStart.getEnd());
+          }
         }
-        else
+        else         
         {
-          if (command.oldds == null)
+          // decide if we need a new dataset sequence or modify start/end
+          // first edit the original dataset sequence string
+          SequenceI oldds = command.seqs[i].getDatasetSequence();
+          String osp = oldds.getSequenceAsString();
+          int beforeStartOfEdit = -oldds.getStart() + 1
+                  + (beforeEditedPositions == null
+                          ? ((afterEditedPositions != null)
+                                  ? afterEditedPositions.getBegin() - 1
+                                  : oldstartend.getBegin()
+                                          + nogapold.length())
+                          : beforeEditedPositions.getEnd()
+                  );
+          int afterEndOfEdit = -oldds.getStart() + 1
+                  + ((afterEditedPositions == null)
+                  ? oldstartend.getEnd()
+                          : afterEditedPositions.getBegin() - 1);
+          String fullseq = osp.substring(0,
+                  beforeStartOfEdit)
+                  + nogaprep
+                  + osp.substring(afterEndOfEdit);
+
+          // and check if new sequence data is different..
+          if (!fullseq.equalsIgnoreCase(osp))
           {
-            command.oldds = new SequenceI[command.seqs.length];
-          }
-          command.oldds[i] = command.seqs[i].getDatasetSequence();
-          SequenceI newds = new Sequence(
-                  command.seqs[i].getDatasetSequence());
-          String fullseq, osp = newds.getSequenceAsString();
-          fullseq = osp.substring(0, ipos) + nogaprep
-                  + osp.substring(ipos + nogaprep.length());
-          newds.setSequence(fullseq.toUpperCase());
-          // TODO: JAL-1131 ensure newly created dataset sequence is added to
-          // the set of
-          // dataset sequences associated with the alignment.
-          // TODO: JAL-1131 fix up any annotation associated with new dataset
-          // sequence to ensure that original sequence/annotation relationships
-          // are preserved.
-          command.seqs[i].setDatasetSequence(newds);
+            // old ds and edited ds are different, so
+            // create the new dataset sequence
+            SequenceI newds = new Sequence(oldds);
+            newds.setSequence(fullseq);
+
+            if (command.oldds == null)
+            {
+              command.oldds = new SequenceI[command.seqs.length];
+            }
+            command.oldds[i] = command.seqs[i].getDatasetSequence();
+
+            // And preserve start/end for good-measure
 
+            if (command.oldStartEnd == null)
+            {
+              command.oldStartEnd = new Range[command.seqs.length];
+            }
+            command.oldStartEnd[i] = oldstartend;
+            // TODO: JAL-1131 ensure newly created dataset sequence is added to
+            // the set of
+            // dataset sequences associated with the alignment.
+            // TODO: JAL-1131 fix up any annotation associated with new dataset
+            // sequence to ensure that original sequence/annotation
+            // relationships
+            // are preserved.
+            command.seqs[i].setDatasetSequence(newds);
+          }
+          else
+          {
+            if (command.oldStartEnd == null)
+            {
+              command.oldStartEnd = new Range[command.seqs.length];
+            }
+            command.oldStartEnd[i] = new Range(command.seqs[i].getStart(),
+                    command.seqs[i].getEnd());
+            if (beforeEditedPositions != null
+                    && afterEditedPositions == null)
+            {
+              // modification at end
+              command.seqs[i].setEnd(
+                      beforeEditedPositions.getEnd() + nogaprep.length()
+                              - nogapold.length());
+            }
+            else if (afterEditedPositions != null
+                    && beforeEditedPositions == null)
+            {
+              // modification at start
+              command.seqs[i].setStart(
+                      afterEditedPositions.getBegin() - nogaprep.length());
+            }
+            else
+            {
+              // edit covered both start and end. Here we can only guess the
+              // new
+              // start/end
+              String nogapalseq = AlignSeq.extractGaps(Comparison.GapChars,
+                      command.seqs[i].getSequenceAsString().toUpperCase());
+              int newStart = command.seqs[i].getDatasetSequence()
+                      .getSequenceAsString().indexOf(nogapalseq);
+              if (newStart == -1)
+              {
+                throw new Error(
+                        "Implementation Error: could not locate start/end "
+                                + "in dataset sequence after an edit of the sequence string");
+              }
+              int newEnd = newStart + nogapalseq.length() - 1;
+              command.seqs[i].setStart(newStart);
+              command.seqs[i].setEnd(newEnd);
+            }
+          }
         }
       }
       tmp = null;
@@ -789,7 +919,7 @@ public class EditCommand implements CommandI
     if (modifyVisibility && !insert)
     {
       // only occurs if a sequence was added or deleted.
-      command.deletedAnnotationRows = new Hashtable<SequenceI, AlignmentAnnotation[]>();
+      command.deletedAnnotationRows = new Hashtable<>();
     }
     if (command.fullAlignmentHeight)
     {
@@ -892,8 +1022,7 @@ public class EditCommand implements CommandI
                 }
                 // and then duplicate added annotation on every other alignment
                 // view
-                for (int vnum = 0; views != null
-                        && vnum < views.length; vnum++)
+                for (int vnum = 0; views != null && vnum < views.length; vnum++)
                 {
                   if (views[vnum] != command.al)
                   {
@@ -948,7 +1077,7 @@ public class EditCommand implements CommandI
 
     if (!insert)
     {
-      command.deletedAnnotations = new Hashtable<String, Annotation[]>();
+      command.deletedAnnotations = new Hashtable<>();
     }
 
     int aSize;
@@ -1109,98 +1238,97 @@ public class EditCommand implements CommandI
     }
   }
 
-  final static void adjustFeatures(Edit command, int index, final int i,
-          final int j, boolean insert)
+  /**
+   * Restores features to the state before a Cut.
+   * <ul>
+   * <li>re-add any features deleted by the cut</li>
+   * <li>remove any truncated features created by the cut</li>
+   * <li>shift right any features to the right of the cut</li>
+   * </ul>
+   * 
+   * @param command
+   *          the Cut command
+   * @param seq
+   *          the sequence the Cut applied to
+   * @param start
+   *          the start residue position of the cut
+   * @param length
+   *          the number of residues cut
+   * @param sameDatasetSequence
+   *          true if dataset sequence and frame of reference were left
+   *          unchanged by the Cut
+   */
+  final static void undoCutFeatures(Edit command, SequenceI seq,
+          final int start, final int length, boolean sameDatasetSequence)
   {
-    SequenceI seq = command.seqs[index];
     SequenceI sequence = seq.getDatasetSequence();
     if (sequence == null)
     {
       sequence = seq;
     }
 
-    if (insert)
+    /*
+     * shift right features that lie to the right of the restored cut (but not 
+     * if dataset sequence unchanged - so coordinates were changed by Cut)
+     */
+    if (!sameDatasetSequence)
     {
-      if (command.editedFeatures != null
-              && command.editedFeatures.containsKey(seq))
+      /*
+       * shift right all features right of and not 
+       * contiguous with the cut position
+       */
+      seq.getFeatures().shiftFeatures(start + 1, length);
+
+      /*
+       * shift right any features that start at the cut position,
+       * unless they were truncated
+       */
+      List<SequenceFeature> sfs = seq.getFeatures().findFeatures(start,
+              start);
+      for (SequenceFeature sf : sfs)
       {
-        sequence.setSequenceFeatures(command.editedFeatures.get(seq));
+        if (sf.getBegin() == start)
+        {
+          if (!command.truncatedFeatures.containsKey(seq)
+                  || !command.truncatedFeatures.get(seq).contains(sf))
+          {
+            /*
+             * feature was shifted left to cut position (not truncated),
+             * so shift it back right
+             */
+            SequenceFeature shifted = new SequenceFeature(sf, sf.getBegin()
+                    + length, sf.getEnd() + length, sf.getFeatureGroup(),
+                    sf.getScore());
+            seq.addSequenceFeature(shifted);
+            seq.deleteFeature(sf);
+          }
+        }
       }
-
-      return;
     }
 
-    List<SequenceFeature> sf = sequence.getFeatures()
-            .getPositionalFeatures();
-
-    if (sf.isEmpty())
-    {
-      return;
-    }
-
-    List<SequenceFeature> oldsf = new ArrayList<SequenceFeature>();
-
-    int cSize = j - i;
-
-    for (SequenceFeature feature : sf)
+    /*
+     * restore any features that were deleted or truncated
+     */
+    if (command.deletedFeatures != null
+            && command.deletedFeatures.containsKey(seq))
     {
-      SequenceFeature copy = new SequenceFeature(feature);
-
-      oldsf.add(copy);
-
-      if (feature.getEnd() < i)
-      {
-        continue;
-      }
-
-      if (feature.getBegin() > j)
-      {
-        int newBegin = copy.getBegin() - cSize;
-        int newEnd = copy.getEnd() - cSize;
-        SequenceFeature newSf = new SequenceFeature(feature, newBegin,
-                newEnd, feature.getFeatureGroup(), feature.getScore());
-        sequence.deleteFeature(feature);
-        sequence.addSequenceFeature(newSf);
-        // feature.setBegin(newBegin);
-        // feature.setEnd(newEnd);
-        continue;
-      }
-
-      int newBegin = feature.getBegin();
-      int newEnd = feature.getEnd();
-      if (newBegin >= i)
-      {
-        newBegin = i;
-        // feature.setBegin(i);
-      }
-
-      if (newEnd < j)
+      for (SequenceFeature deleted : command.deletedFeatures.get(seq))
       {
-        newEnd = j - 1;
-        // feature.setEnd(j - 1);
+        sequence.addSequenceFeature(deleted);
       }
-      newEnd = newEnd - cSize;
-      // feature.setEnd(feature.getEnd() - (cSize));
-
-      sequence.deleteFeature(feature);
-      if (newEnd >= newBegin)
-      {
-        sequence.addSequenceFeature(new SequenceFeature(feature, newBegin,
-                newEnd, feature.getFeatureGroup(), feature.getScore()));
-      }
-      // if (feature.getBegin() > feature.getEnd())
-      // {
-      // sequence.deleteFeature(feature);
-      // }
     }
 
-    if (command.editedFeatures == null)
+    /*
+     * delete any truncated features
+     */
+    if (command.truncatedFeatures != null
+            && command.truncatedFeatures.containsKey(seq))
     {
-      command.editedFeatures = new Hashtable<SequenceI, List<SequenceFeature>>();
+      for (SequenceFeature amended : command.truncatedFeatures.get(seq))
+      {
+        sequence.deleteFeature(amended);
+      }
     }
-
-    command.editedFeatures.put(seq, oldsf);
-
   }
 
   /**
@@ -1233,7 +1361,7 @@ public class EditCommand implements CommandI
    */
   public Map<SequenceI, SequenceI> priorState(boolean forUndo)
   {
-    Map<SequenceI, SequenceI> result = new HashMap<SequenceI, SequenceI>();
+    Map<SequenceI, SequenceI> result = new HashMap<>();
     if (getEdits() == null)
     {
       return result;
@@ -1266,7 +1394,7 @@ public class EditCommand implements CommandI
      * Work backwards through the edit list, deriving the sequences before each
      * was applied. The final result is the sequence set before any edits.
      */
-    Iterator<Edit> editList = new ReverseListIterator<Edit>(getEdits());
+    Iterator<Edit> editList = new ReverseListIterator<>(getEdits());
     while (editList.hasNext())
     {
       Edit oldEdit = editList.next();
@@ -1315,29 +1443,45 @@ public class EditCommand implements CommandI
 
   public class Edit
   {
-    public SequenceI[] oldds;
+    private SequenceI[] oldds;
+
+    /**
+     * start and end of sequence prior to edit
+     */
+    private Range[] oldStartEnd;
+
+    private boolean fullAlignmentHeight = false;
 
-    boolean fullAlignmentHeight = false;
+    private Map<SequenceI, AlignmentAnnotation[]> deletedAnnotationRows;
 
-    Hashtable<SequenceI, AlignmentAnnotation[]> deletedAnnotationRows;
+    private Map<String, Annotation[]> deletedAnnotations;
 
-    Hashtable<String, Annotation[]> deletedAnnotations;
+    /*
+     * features deleted by the cut (re-add on Undo)
+     * (including the original of any shortened features)
+     */
+    private Map<SequenceI, List<SequenceFeature>> deletedFeatures;
 
-    Hashtable<SequenceI, List<SequenceFeature>> editedFeatures;
+    /*
+     * shortened features added by the cut (delete on Undo)
+     */
+    private Map<SequenceI, List<SequenceFeature>> truncatedFeatures;
 
-    AlignmentI al;
+    private AlignmentI al;
 
-    Action command;
+    final private Action command;
 
     char[][] string;
 
     SequenceI[] seqs;
 
-    int[] alIndex;
+    private int[] alIndex;
 
-    int position, number;
+    private int position;
 
-    char gapChar;
+    private int number;
+
+    private char gapChar;
 
     public Edit(Action cmd, SequenceI[] sqs, int pos, int count,
             char gap)
@@ -1352,11 +1496,8 @@ public class EditCommand implements CommandI
     Edit(Action cmd, SequenceI[] sqs, int pos, int count,
             AlignmentI align)
     {
-      this.gapChar = align.getGapCharacter();
-      this.command = cmd;
-      this.seqs = sqs;
-      this.position = pos;
-      this.number = count;
+      this(cmd, sqs, pos, count, align.getGapCharacter());
+
       this.al = align;
 
       alIndex = new int[sqs.length];
@@ -1368,22 +1509,26 @@ public class EditCommand implements CommandI
       fullAlignmentHeight = (align.getHeight() == sqs.length);
     }
 
+    /**
+     * Constructor given a REPLACE command and the replacement string
+     * 
+     * @param cmd
+     * @param sqs
+     * @param pos
+     * @param count
+     * @param align
+     * @param replace
+     */
     Edit(Action cmd, SequenceI[] sqs, int pos, int count,
             AlignmentI align, String replace)
     {
-      this.command = cmd;
-      this.seqs = sqs;
-      this.position = pos;
-      this.number = count;
-      this.al = align;
-      this.gapChar = align.getGapCharacter();
+      this(cmd, sqs, pos, count, align);
+
       string = new char[sqs.length][];
       for (int i = 0; i < sqs.length; i++)
       {
         string[i] = replace.toCharArray();
       }
-
-      fullAlignmentHeight = (align.getHeight() == sqs.length);
     }
 
     public SequenceI[] getSequences()
@@ -1427,7 +1572,163 @@ public class EditCommand implements CommandI
     }
     else
     {
-      return new ReverseListIterator<Edit>(getEdits());
+      return new ReverseListIterator<>(getEdits());
+    }
+  }
+
+  /**
+   * Adjusts features for Cut, and saves details of changes made to allow Undo
+   * <ul>
+   * <li>features left of the cut are unchanged</li>
+   * <li>features right of the cut are shifted left</li>
+   * <li>features internal to the cut region are deleted</li>
+   * <li>features that overlap or span the cut are shortened</li>
+   * <li>the originals of any deleted or shortened features are saved, to re-add
+   * on Undo</li>
+   * <li>any added (shortened) features are saved, to delete on Undo</li>
+   * </ul>
+   * 
+   * @param command
+   * @param seq
+   * @param fromPosition
+   * @param toPosition
+   * @param cutIsInternal
+   */
+  protected static void cutFeatures(Edit command, SequenceI seq,
+          int fromPosition, int toPosition, boolean cutIsInternal)
+  {
+    /* 
+     * if the cut is at start or end of sequence
+     * then we don't modify the sequence feature store
+     */
+    if (!cutIsInternal)
+    {
+      return;
+    }
+    List<SequenceFeature> added = new ArrayList<>();
+    List<SequenceFeature> removed = new ArrayList<>();
+  
+    SequenceFeaturesI featureStore = seq.getFeatures();
+    if (toPosition < fromPosition || featureStore == null)
+    {
+      return;
+    }
+  
+    int cutStartPos = fromPosition;
+    int cutEndPos = toPosition;
+    int cutWidth = cutEndPos - cutStartPos + 1;
+  
+    synchronized (featureStore)
+    {
+      /*
+       * get features that overlap the cut region
+       */
+      List<SequenceFeature> toAmend = featureStore.findFeatures(
+              cutStartPos, cutEndPos);
+  
+      /*
+       * add any contact features that span the cut region
+       * (not returned by findFeatures)
+       */
+      for (SequenceFeature contact : featureStore.getContactFeatures())
+      {
+        if (contact.getBegin() < cutStartPos
+                && contact.getEnd() > cutEndPos)
+        {
+          toAmend.add(contact);
+        }
+      }
+
+      /*
+       * adjust start-end of overlapping features;
+       * delete features enclosed by the cut;
+       * delete partially overlapping contact features
+       */
+      for (SequenceFeature sf : toAmend)
+      {
+        int sfBegin = sf.getBegin();
+        int sfEnd = sf.getEnd();
+        int newBegin = sfBegin;
+        int newEnd = sfEnd;
+        boolean toDelete = false;
+        boolean follows = false;
+        
+        if (sfBegin >= cutStartPos && sfEnd <= cutEndPos)
+        {
+          /*
+           * feature lies within cut region - delete it
+           */
+          toDelete = true;
+        }
+        else if (sfBegin < cutStartPos && sfEnd > cutEndPos)
+        {
+          /*
+           * feature spans cut region - left-shift the end
+           */
+          newEnd -= cutWidth;
+        }
+        else if (sfEnd <= cutEndPos)
+        {
+          /*
+           * feature overlaps left of cut region - truncate right
+           */
+          newEnd = cutStartPos - 1;
+          if (sf.isContactFeature())
+          {
+            toDelete = true;
+          }
+        }
+        else if (sfBegin >= cutStartPos)
+        {
+          /*
+           * remaining case - feature overlaps right
+           * truncate left, adjust end of feature
+           */
+          newBegin = cutIsInternal ? cutStartPos : cutEndPos + 1;
+          newEnd = newBegin + sfEnd - cutEndPos - 1;
+          if (sf.isContactFeature())
+          {
+            toDelete = true;
+          }
+        }
+  
+        seq.deleteFeature(sf);
+        if (!follows)
+        {
+          removed.add(sf);
+        }
+        if (!toDelete)
+        {
+          SequenceFeature copy = new SequenceFeature(sf, newBegin, newEnd,
+                  sf.getFeatureGroup(), sf.getScore());
+          seq.addSequenceFeature(copy);
+          if (!follows)
+          {
+            added.add(copy);
+          }
+        }
+      }
+  
+      /*
+       * and left shift any features lying to the right of the cut region
+       */
+
+      featureStore.shiftFeatures(cutEndPos + 1, -cutWidth);
+    }
+
+    /*
+     * save deleted and amended features, so that Undo can 
+     * re-add or delete them respectively
+     */
+    if (command.deletedFeatures == null)
+    {
+      command.deletedFeatures = new HashMap<>();
+    }
+    if (command.truncatedFeatures == null)
+    {
+      command.truncatedFeatures = new HashMap<>();
     }
+    command.deletedFeatures.put(seq, removed);
+    command.truncatedFeatures.put(seq, added);
   }
 }
index 33de452..f555855 100755 (executable)
@@ -43,10 +43,7 @@ import fr.orsay.lri.varna.models.rna.RNA;
 
 /**
  * 
- * Implements the SequenceI interface for a char[] based sequence object.
- * 
- * @author $author$
- * @version $Revision$
+ * Implements the SequenceI interface for a char[] based sequence object
  */
 public class Sequence extends ASequence implements SequenceI
 {
@@ -797,6 +794,7 @@ public class Sequence extends ASequence implements SequenceI
      * preserve end residue column provided cursor was valid
      */
     int endColumn = isValidCursor(cursor) ? cursor.lastColumnPosition : 0;
+
     if (residuePos == this.end)
     {
       endColumn = column;
@@ -833,8 +831,7 @@ public class Sequence extends ASequence implements SequenceI
     /*
      * move left or right to find pos from hint.position
      */
-    int col = curs.columnPosition - 1; // convert from base 1 to 0-based array
-                                       // index
+    int col = curs.columnPosition - 1; // convert from base 1 to base 0
     int newPos = curs.residuePosition;
     int delta = newPos > pos ? -1 : 1;
 
@@ -1263,12 +1260,13 @@ public class Sequence extends ASequence implements SequenceI
     boolean createNewDs = false;
     // TODO: take a (second look) at the dataset creation validation method for
     // the very large sequence case
+
     int startIndex = findIndex(start) - 1;
     int endIndex = findIndex(end) - 1;
     int startDeleteColumn = -1; // for dataset sequence deletions
     int deleteCount = 0;
 
-    for (int s = i; s < j; s++)
+    for (int s = i; s < j && s < sequence.length; s++)
     {
       if (Comparison.isGap(sequence[s]))
       {
@@ -1913,6 +1911,15 @@ public class Sequence extends ASequence implements SequenceI
 
     List<SequenceFeature> result = getFeatures().findFeatures(startPos,
             endPos, types);
+    if (datasetSequence != null)
+    {
+      result = datasetSequence.getFeatures().findFeatures(startPos, endPos,
+              types);
+    }
+    else
+    {
+      result = sequenceFeatureStore.findFeatures(startPos, endPos, types);
+    }
 
     /*
      * if end column is gapped, endPos may be to the right, 
index 944f263..fc8ac49 100755 (executable)
@@ -102,11 +102,15 @@ public class SequenceGroup implements AnnotatedCollectionI
    */
   public ResidueShaderI cs;
 
-  // start column (base 0)
-  int startRes = 0;
+  /**
+   * start column (base 0)
+   */
+  private int startRes = 0;
 
-  // end column (base 0)
-  int endRes = 0;
+  /**
+   *  end column (base 0)
+   */
+  private int endRes = 0;
 
   public Color outlineColour = Color.black;
 
@@ -209,6 +213,7 @@ public class SequenceGroup implements AnnotatedCollectionI
       displayBoxes = seqsel.displayBoxes;
       displayText = seqsel.displayText;
       colourText = seqsel.colourText;
+      
       startRes = seqsel.startRes;
       endRes = seqsel.endRes;
       cs = new ResidueShader((ResidueShader) seqsel.cs);
@@ -752,13 +757,16 @@ public class SequenceGroup implements AnnotatedCollectionI
   /**
    * Set the first column selected by this group. Runs from 0<=i<N_cols
    * 
-   * @param i
+   * @param newStart
    */
-  public void setStartRes(int i)
+  public void setStartRes(int newStart)
   {
     int before = startRes;
-    startRes = i;
-    changeSupport.firePropertyChange(SEQ_GROUP_CHANGED, before, startRes);
+   startRes= Math.max(0,newStart); // sanity check for negative start column positions
+   changeSupport.firePropertyChange(SEQ_GROUP_CHANGED, before, startRes);
+    
+
+
   }
 
   /**
index 8dce31e..48615f0 100755 (executable)
@@ -206,11 +206,14 @@ public interface SequenceI extends ASequenceI
   public int findPosition(int i);
 
   /**
-   * Returns the from-to sequence positions (start..) for the given column
-   * positions (1..), or null if no residues are included in the range
+   * Returns the sequence positions for first and last residues lying within the
+   * given column positions [fromColum,toColumn] (where columns are numbered
+   * from 1), or null if no residues are included in the range
    * 
    * @param fromColum
+   *          - first column base 1
    * @param toColumn
+   *          - last column, base 1
    * @return
    */
   public Range findPositions(int fromColum, int toColumn);
index 02ce1c5..951b7a5 100644 (file)
@@ -1047,13 +1047,15 @@ public class FeatureStore
   }
 
   /**
-   * Adds the shift value to the start and end of all positional features.
-   * Returns true if at least one feature was updated, else false.
+   * Adds the shift amount to the start and end of all positional features whose
+   * start position is at or after fromPosition. Returns true if at least one
+   * feature was shifted, else false.
    * 
-   * @param shift
+   * @param fromPosition
+   * @param shiftBy
    * @return
    */
-  public synchronized boolean shiftFeatures(int shift)
+  public synchronized boolean shiftFeatures(int fromPosition, int shiftBy)
   {
     /*
      * Because begin and end are final fields (to ensure the data store's
@@ -1063,21 +1065,24 @@ public class FeatureStore
     boolean modified = false;
     for (SequenceFeature sf : getPositionalFeatures())
     {
-      modified = true;
-      int newBegin = sf.getBegin() + shift;
-      int newEnd = sf.getEnd() + shift;
-
-      /*
-       * sanity check: don't shift left of the first residue
-       */
-      if (newEnd > 0)
+      if (sf.getBegin() >= fromPosition)
       {
-        newBegin = Math.max(1, newBegin);
-        SequenceFeature sf2 = new SequenceFeature(sf, newBegin, newEnd,
-                sf.getFeatureGroup(), sf.getScore());
-        addFeature(sf2);
+        modified = true;
+        int newBegin = sf.getBegin() + shiftBy;
+        int newEnd = sf.getEnd() + shiftBy;
+
+        /*
+         * sanity check: don't shift left of the first residue
+         */
+        if (newEnd > 0)
+        {
+          newBegin = Math.max(1, newBegin);
+          SequenceFeature sf2 = new SequenceFeature(sf, newBegin, newEnd,
+                  sf.getFeatureGroup(), sf.getScore());
+          addFeature(sf2);
+        }
+        delete(sf);
       }
-      delete(sf);
     }
     return modified;
   }
index 727d3ef..8f965b4 100644 (file)
@@ -472,13 +472,22 @@ public class SequenceFeatures implements SequenceFeaturesI
    * {@inheritDoc}
    */
   @Override
-  public boolean shiftFeatures(int shift)
+  public boolean shiftFeatures(int fromPosition, int shiftBy)
   {
     boolean modified = false;
     for (FeatureStore fs : featureStore.values())
     {
-      modified |= fs.shiftFeatures(shift);
+      modified |= fs.shiftFeatures(fromPosition, shiftBy);
     }
     return modified;
   }
-}
\ No newline at end of file
+
+  /**
+   * {@inheritDoc}
+   */
+  @Override
+  public void deleteAll()
+  {
+    featureStore.clear();
+  }
+}
index 31712b9..ca0283e 100644 (file)
@@ -215,10 +215,17 @@ public interface SequenceFeaturesI
   float getMaximumScore(String type, boolean positional);
 
   /**
-   * Adds the shift amount to the start and end of all positional features,
-   * returning true if at least one feature was shifted, else false
+   * Adds the shift amount to the start and end of all positional features whose
+   * start position is at or after fromPosition. Returns true if at least one
+   * feature was shifted, else false.
    * 
-   * @param shift
+   * @param fromPosition
+   * @param shiftBy
    */
-  abstract boolean shiftFeatures(int shift);
-}
\ No newline at end of file
+  boolean shiftFeatures(int fromPosition, int shiftBy);
+
+  /**
+   * Deletes all positional and non-positional features
+   */
+  void deleteAll();
+}
index 8832278..a5b1110 100644 (file)
@@ -1254,7 +1254,10 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel
     return chainNames;
   }
 
-  protected abstract IProgressIndicator getIProgressIndicator();
+  protected IProgressIndicator getIProgressIndicator()
+  {
+    return null;
+  }
 
   public void notifyNewPickingModeMeasurement(int iatom, String strMeasure)
   {
index 90d7c35..dbe3317 100644 (file)
@@ -79,7 +79,6 @@ import javax.swing.BorderFactory;
 import javax.swing.Icon;
 import javax.swing.JButton;
 import javax.swing.JCheckBox;
-import javax.swing.JCheckBoxMenuItem;
 import javax.swing.JColorChooser;
 import javax.swing.JDialog;
 import javax.swing.JInternalFrame;
@@ -219,7 +218,8 @@ public class FeatureSettings extends JPanel
           FeatureMatcherSet o = (FeatureMatcherSet) table.getValueAt(row,
                   column);
           tip = o.isEmpty()
-                  ? MessageManager.getString("label.filters_tooltip")
+                  ? MessageManager
+                          .getString("label.configure_feature_tooltip")
                   : o.toString();
           break;
         default:
@@ -415,69 +415,6 @@ public class FeatureSettings extends JPanel
     });
     men.add(dens);
 
-    /*
-     * variable colour options include colour by label, by score,
-     * by selected attribute text, or attribute value
-     */
-    final JCheckBoxMenuItem mxcol = new JCheckBoxMenuItem(
-            MessageManager.getString("label.variable_colour"));
-    mxcol.setSelected(!featureColour.isSimpleColour());
-    men.add(mxcol);
-    mxcol.addActionListener(new ActionListener()
-    {
-      JColorChooser colorChooser;
-
-      @Override
-      public void actionPerformed(ActionEvent e)
-      {
-        if (e.getSource() == mxcol)
-        {
-          if (featureColour.isSimpleColour())
-          {
-            FeatureTypeSettings fc = new FeatureTypeSettings(me.fr, type);
-            fc.addActionListener(this);
-          }
-          else
-          {
-            // bring up simple color chooser
-            colorChooser = new JColorChooser();
-            String title = MessageManager
-                    .getString("label.select_colour");
-            JDialog dialog = JColorChooser.createDialog(me,
-                    title, true, // modal
-                    colorChooser, this, // OK button handler
-                    null); // no CANCEL button handler
-            colorChooser.setColor(featureColour.getMaxColour());
-            dialog.setVisible(true);
-          }
-        }
-        else
-        {
-          if (e.getSource() instanceof FeatureTypeSettings)
-          {
-            /*
-             * update after OK in feature colour dialog; the updated
-             * colour will have already been set in the FeatureRenderer
-             */
-            FeatureColourI fci = fr.getFeatureColours().get(type);
-            table.setValueAt(fci, rowSelected, 1);
-            table.validate();
-          }
-          else
-          {
-            // probably the color chooser!
-            table.setValueAt(new FeatureColour(colorChooser.getColor()),
-                    rowSelected, 1);
-            table.validate();
-            me.updateFeatureRenderer(
-                    ((FeatureTableModel) table.getModel()).getData(),
-                    false);
-          }
-        }
-      }
-
-    });
-
     JMenuItem selCols = new JMenuItem(
             MessageManager.getString("label.select_columns_containing"));
     selCols.addActionListener(new ActionListener()
@@ -1354,7 +1291,7 @@ public class FeatureSettings extends JPanel
     private String[] columnNames = {
         MessageManager.getString("label.feature_type"),
         MessageManager.getString("action.colour"),
-        MessageManager.getString("label.filter"),
+        MessageManager.getString("label.configuration"),
         MessageManager.getString("label.show") };
 
     private Object[][] data;
@@ -1690,14 +1627,17 @@ public class FeatureSettings extends JPanel
         {
           // bring up graduated chooser.
           chooser = new FeatureTypeSettings(me.fr, type);
-          chooser.setRequestFocusEnabled(true);
-          chooser.requestFocus();
+          /**
+           * @j2sNative
+           */
+          {
+            chooser.setRequestFocusEnabled(true);
+            chooser.requestFocus();
+          }
           chooser.addActionListener(this);
-          chooser.showTab(true);
+          // Make the renderer reappear.
+          fireEditingStopped();
         }
-        // Make the renderer reappear.
-        fireEditingStopped();
-
       }
       else
       {
@@ -1819,7 +1759,6 @@ public class FeatureSettings extends JPanel
                   chooser.getWidth(), chooser.getHeight());
           chooser.validate();
         }
-        chooser.showTab(false);
         fireEditingStopped();
       }
       else if (e.getSource() instanceof Component)
index e13f6ee..58bbbe8 100644 (file)
@@ -62,7 +62,6 @@ import javax.swing.JLabel;
 import javax.swing.JPanel;
 import javax.swing.JRadioButton;
 import javax.swing.JSlider;
-import javax.swing.JTabbedPane;
 import javax.swing.JTextField;
 import javax.swing.SwingConstants;
 import javax.swing.border.LineBorder;
@@ -107,7 +106,8 @@ public class FeatureTypeSettings extends JalviewDialog
   /*
    * FeatureRenderer holds colour scheme and filters for feature types
    */
-  private final FeatureRenderer fr; // todo refactor to allow interface type here
+  private final FeatureRenderer fr; // todo refactor to allow interface type
+                                    // here
 
   /*
    * the view panel to update when settings change
@@ -155,7 +155,12 @@ public class FeatureTypeSettings extends JalviewDialog
 
   private JRadioButton graduatedColour = new JRadioButton();
 
-  private JPanel singleColour = new JPanel();
+  /**
+   * colours and filters are shown in tabbed view or single content pane
+   */
+  JPanel coloursPanel, filtersPanel;
+
+  JPanel singleColour = new JPanel();
 
   private JPanel minColour = new JPanel();
 
@@ -199,13 +204,8 @@ public class FeatureTypeSettings extends JalviewDialog
    */
   private List<FeatureMatcherI> filters;
 
-  // set white normally, black to debug layout
-  private Color debugBorderColour = Color.white;
-
   private JPanel chooseFiltersPanel;
 
-  private JTabbedPane tabbedPane;
-
   /**
    * Constructor
    * 
@@ -214,28 +214,14 @@ public class FeatureTypeSettings extends JalviewDialog
    */
   public FeatureTypeSettings(FeatureRenderer frender, String theType)
   {
-    this(frender, false, theType);
-  }
-
-  /**
-   * Constructor, with option to make a blocking dialog (has to complete in the
-   * AWT event queue thread). Currently this option is always set to false.
-   * 
-   * @param frender
-   * @param blocking
-   * @param theType
-   */
-  FeatureTypeSettings(FeatureRenderer frender, boolean blocking,
-          String theType)
-  {
     this.fr = frender;
     this.featureType = theType;
     ap = fr.ap;
     originalFilter = fr.getFeatureFilter(theType);
     originalColour = fr.getFeatureColours().get(theType);
-
+    
     adjusting = true;
-
+    
     try
     {
       initialise();
@@ -244,20 +230,19 @@ public class FeatureTypeSettings extends JalviewDialog
       ex.printStackTrace();
       return;
     }
-
+    
     updateColoursTab();
-
+    
     updateFiltersTab();
-
+    
     adjusting = false;
-
+    
     colourChanged(false);
-
+    
     String title = MessageManager
             .formatMessage("label.display_settings_for", new String[]
             { theType });
-    initDialogFrame(this, true, blocking, title, 600, 360);
-
+    initDialogFrame(this, true, false, title, 580, 500);
     waitForInput();
   }
 
@@ -280,9 +265,9 @@ public class FeatureTypeSettings extends JalviewDialog
        */
       if (fc.isSimpleColour())
       {
-        simpleColour.setSelected(true);
         singleColour.setBackground(fc.getColour());
         singleColour.setForeground(fc.getColour());
+        simpleColour.setSelected(true);
       }
 
       /*
@@ -380,7 +365,7 @@ public class FeatureTypeSettings extends JalviewDialog
                         : BELOW_THRESHOLD_OPTION);
         slider.setEnabled(true);
         slider.setValue((int) (fc.getThreshold() * scaleFactor));
-        thresholdValue.setText(String.valueOf(getRoundedSliderValue()));
+        thresholdValue.setText(String.valueOf(fc.getThreshold()));
         thresholdValue.setEnabled(true);
         thresholdIsMin.setEnabled(true);
       }
@@ -403,8 +388,6 @@ public class FeatureTypeSettings extends JalviewDialog
   private void initialise()
   {
     this.setLayout(new BorderLayout());
-    tabbedPane = new JTabbedPane();
-    this.add(tabbedPane, BorderLayout.CENTER);
 
     /*
      * an ActionListener that applies colour changes
@@ -419,18 +402,16 @@ public class FeatureTypeSettings extends JalviewDialog
     };
 
     /*
-     * first tab: colour options
+     * first panel/tab: colour options
      */
     JPanel coloursPanel = initialiseColoursPanel();
-    tabbedPane.addTab(MessageManager.getString("action.colour"),
-            coloursPanel);
+    this.add(coloursPanel, BorderLayout.NORTH);
 
     /*
-     * second tab: filter options
+     * second panel/tab: filter options
      */
     JPanel filtersPanel = initialiseFiltersPanel();
-    tabbedPane.addTab(MessageManager.getString("label.filters"),
-            filtersPanel);
+    this.add(filtersPanel, BorderLayout.CENTER);
 
     JPanel okCancelPanel = initialiseOkCancelPanel();
 
@@ -581,12 +562,11 @@ public class FeatureTypeSettings extends JalviewDialog
     maxColour.setBorder(new LineBorder(Color.black));
 
     /*
-     * default max colour to current colour (if a plain colour),
-     * or to Black if colour by label;  make min colour a pale
-     * version of max colour
+     * default max colour to last plain colour;
+     * make min colour a pale version of max colour
      */
     FeatureColourI fc = fr.getFeatureColours().get(featureType);
-    Color bg = fc.isSimpleColour() ? fc.getColour() : Color.BLACK;
+    Color bg = fc.getColour() == null ? Color.BLACK : fc.getColour();
     maxColour.setBackground(bg);
     minColour.setBackground(ColorUtils.bleachColour(bg, 0.9f));
 
@@ -671,6 +651,7 @@ public class FeatureTypeSettings extends JalviewDialog
         {
           thresholdValue
                   .setText(String.valueOf(slider.getValue() / scaleFactor));
+          thresholdValue.setBackground(Color.white); // to reset red for invalid
           sliderValueChanged();
         }
       }
@@ -736,15 +717,16 @@ public class FeatureTypeSettings extends JalviewDialog
   private JPanel initialiseColoursPanel()
   {
     JPanel colourByPanel = new JPanel();
+    colourByPanel.setBackground(Color.white);
     colourByPanel.setLayout(new BoxLayout(colourByPanel, BoxLayout.Y_AXIS));
+    JvSwingUtils.createTitledBorder(colourByPanel,
+            MessageManager.getString("action.colour"), true);
 
     /*
      * simple colour radio button and colour picker
      */
     JPanel simpleColourPanel = new JPanel(new FlowLayout(FlowLayout.LEFT));
     simpleColourPanel.setBackground(Color.white);
-    JvSwingUtils.createTitledBorder(simpleColourPanel,
-            MessageManager.getString("label.simple"), true);
     colourByPanel.add(simpleColourPanel);
 
     simpleColour = new JRadioButton(
@@ -757,15 +739,24 @@ public class FeatureTypeSettings extends JalviewDialog
       {
         if (simpleColour.isSelected() && !adjusting)
         {
-          showColourChooser(singleColour, "label.select_colour");
+          colourChanged(true);
         }
       }
-
     });
-    
+
     singleColour.setFont(JvSwingUtils.getLabelFont());
     singleColour.setBorder(BorderFactory.createLineBorder(Color.black));
     singleColour.setPreferredSize(new Dimension(40, 20));
+    if (originalColour.isGraduatedColour())
+    {
+      singleColour.setBackground(originalColour.getMaxColour());
+      singleColour.setForeground(originalColour.getMaxColour());
+    }
+    else
+    {
+      singleColour.setBackground(originalColour.getColour());
+      singleColour.setForeground(originalColour.getColour());
+    }
     singleColour.addMouseListener(new MouseAdapter()
     {
       @Override
@@ -849,9 +840,9 @@ public class FeatureTypeSettings extends JalviewDialog
   }
 
   /**
-   * Constructs and sets the selected colour options as the colour for the feature
-   * type, and repaints the alignment, and optionally the Overview and/or
-   * structure viewer if open
+   * Constructs and sets the selected colour options as the colour for the
+   * feature type, and repaints the alignment, and optionally the Overview
+   * and/or structure viewer if open
    * 
    * @param updateStructsAndOverview
    */
@@ -902,8 +893,8 @@ public class FeatureTypeSettings extends JalviewDialog
      */
     if (byCategory.isSelected())
     {
-      Color c = this.getBackground();
-      FeatureColourI fc = new FeatureColour(c, c, null, 0f, 0f);
+      Color c = singleColour.getBackground();
+      FeatureColourI fc = new FeatureColour(c);
       fc.setColourByLabel(true);
       String byWhat = (String) colourByTextCombo.getSelectedItem();
       if (!LABEL_18N.equals(byWhat))
@@ -1028,21 +1019,23 @@ public class FeatureTypeSettings extends JalviewDialog
   {
     try
     {
+      /*
+       * set 'adjusting' flag while moving the slider, so it 
+       * doesn't then in turn change the value (with rounding)
+       */
       adjusting = true;
       float f = Float.parseFloat(thresholdValue.getText());
+      f = Float.max(f,  this.min);
+      f = Float.min(f, this.max);
+      thresholdValue.setText(String.valueOf(f));
       slider.setValue((int) (f * scaleFactor));
       threshline.value = f;
       thresholdValue.setBackground(Color.white); // ok
-
-      /*
-       * force repaint of any Overview window or structure
-       */
-      ap.paintAlignment(true, true);
+      adjusting = false;
+      colourChanged(true);
     } catch (NumberFormatException ex)
     {
       thresholdValue.setBackground(Color.red); // not ok
-    } finally
-    {
       adjusting = false;
     }
   }
@@ -1065,8 +1058,8 @@ public class FeatureTypeSettings extends JalviewDialog
 
   /**
    * Converts the slider value to its absolute value by dividing by the
-   * scaleFactor. Rounding errors are squashed by forcing min/max of slider range
-   * to the actual min/max of feature score range
+   * scaleFactor. Rounding errors are squashed by forcing min/max of slider
+   * range to the actual min/max of feature score range
    * 
    * @return
    */
@@ -1089,11 +1082,11 @@ public class FeatureTypeSettings extends JalviewDialog
   }
 
   /**
-   * A helper method to build the drop-down choice of attributes for a feature. If
-   * 'withRange' is true, then Score, and any attributes with a min-max range, are
-   * added. If 'withText' is true, Label and any known attributes are added. This
-   * allows 'categorical numerical' attributes e.g. codon position to be coloured
-   * by text.
+   * A helper method to build the drop-down choice of attributes for a feature.
+   * If 'withRange' is true, then Score, and any attributes with a min-max
+   * range, are added. If 'withText' is true, Label and any known attributes are
+   * added. This allows 'categorical numerical' attributes e.g. codon position
+   * to be coloured by text.
    * <p>
    * Where metadata is available with a description for an attribute, that is
    * added as a tooltip.
@@ -1188,7 +1181,6 @@ public class FeatureTypeSettings extends JalviewDialog
   {
     JPanel andOrPanel = new JPanel(new FlowLayout(FlowLayout.LEFT));
     andOrPanel.setBackground(Color.white);
-    andOrPanel.setBorder(BorderFactory.createLineBorder(debugBorderColour));
     andFilters = new JRadioButton(MessageManager.getString("label.and"));
     orFilters = new JRadioButton(MessageManager.getString("label.or"));
     ActionListener actionListener = new ActionListener()
@@ -1271,7 +1263,6 @@ public class FeatureTypeSettings extends JalviewDialog
     for (FeatureMatcherI filter : filters)
     {
       JPanel row = addFilter(filter, attNames, filterIndex);
-      row.setBorder(BorderFactory.createLineBorder(debugBorderColour));
       chooseFiltersPanel.add(row);
       filterIndex++;
     }
@@ -1283,7 +1274,8 @@ public class FeatureTypeSettings extends JalviewDialog
   /**
    * A helper method that constructs a row (panel) with one filter condition:
    * <ul>
-   * <li>a drop-down list of Label, Score and attribute names to choose from</li>
+   * <li>a drop-down list of Label, Score and attribute names to choose
+   * from</li>
    * <li>a drop-down list of conditions to choose from</li>
    * <li>a text field for input of a match pattern</li>
    * <li>optionally, a 'remove' button</li>
@@ -1481,8 +1473,8 @@ public class FeatureTypeSettings extends JalviewDialog
    * @param selectedCondition
    * @param patternField
    */
-  private void setNumericHints(String attName,
-          Condition selectedCondition, JTextField patternField)
+  private void setNumericHints(String attName, Condition selectedCondition,
+          JTextField patternField)
   {
     patternField.setToolTipText("");
 
@@ -1516,11 +1508,11 @@ public class FeatureTypeSettings extends JalviewDialog
   }
 
   /**
-   * Populates the drop-down list of comparison conditions for the given attribute
-   * name. The conditions added depend on the datatype of the attribute values.
-   * The supplied condition is set as the selected item in the list, provided it
-   * is in the list. If the pattern is now invalid (non-numeric pattern for a
-   * numeric condition), it is cleared.
+   * Populates the drop-down list of comparison conditions for the given
+   * attribute name. The conditions added depend on the datatype of the
+   * attribute values. The supplied condition is set as the selected item in the
+   * list, provided it is in the list. If the pattern is now invalid
+   * (non-numeric pattern for a numeric condition), it is cleared.
    * 
    * @param attName
    * @param cond
@@ -1599,12 +1591,12 @@ public class FeatureTypeSettings extends JalviewDialog
   }
 
   /**
-   * Answers true unless a numeric condition has been selected with a non-numeric
-   * value. Sets the value field to RED with a tooltip if in error.
+   * Answers true unless a numeric condition has been selected with a
+   * non-numeric value. Sets the value field to RED with a tooltip if in error.
    * <p>
-   * If the pattern is expected but is empty, this method returns false, but does
-   * not mark the field as invalid. This supports selecting an attribute for a new
-   * condition before a match pattern has been entered.
+   * If the pattern is expected but is empty, this method returns false, but
+   * does not mark the field as invalid. This supports selecting an attribute
+   * for a new condition before a match pattern has been entered.
    * 
    * @param value
    * @param condCombo
@@ -1650,14 +1642,15 @@ public class FeatureTypeSettings extends JalviewDialog
 
   /**
    * Constructs a filter condition from the given input fields, and replaces the
-   * condition at filterIndex with the new one. Does nothing if the pattern field
-   * is blank (unless the match condition is one that doesn't require a pattern,
-   * e.g. 'Is present'). Answers true if the filter was updated, else false.
+   * condition at filterIndex with the new one. Does nothing if the pattern
+   * field is blank (unless the match condition is one that doesn't require a
+   * pattern, e.g. 'Is present'). Answers true if the filter was updated, else
+   * false.
    * <p>
    * This method may update the tooltip on the filter value field to show the
-   * value range, if a numeric condition is selected. This ensures the tooltip is
-   * updated when a numeric valued attribute is chosen on the last 'add a filter'
-   * row.
+   * value range, if a numeric condition is selected. This ensures the tooltip
+   * is updated when a numeric valued attribute is chosen on the last 'add a
+   * filter' row.
    * 
    * @param attCombo
    * @param condCombo
@@ -1705,17 +1698,6 @@ public class FeatureTypeSettings extends JalviewDialog
   }
 
   /**
-   * Makes the dialog visible, at the Feature Colour tab or at the Filters tab
-   * 
-   * @param coloursTab
-   */
-  public void showTab(boolean coloursTab)
-  {
-    setVisible(true);
-    tabbedPane.setSelectedIndex(coloursTab ? 0 : 1);
-  }
-
-  /**
    * Action on any change to feature filtering, namely
    * <ul>
    * <li>change of selected attribute</li>
@@ -1723,8 +1705,8 @@ public class FeatureTypeSettings extends JalviewDialog
    * <li>change of match pattern</li>
    * <li>removal of a condition</li>
    * </ul>
-   * The inputs are parsed into a combined filter and this is set for the feature
-   * type, and the alignment redrawn.
+   * The inputs are parsed into a combined filter and this is set for the
+   * feature type, and the alignment redrawn.
    */
   protected void filtersChanged()
   {
index cc361a5..89088b8 100644 (file)
@@ -257,6 +257,7 @@ public class OverviewCanvas extends JComponent
   public void dispose()
   {
     dispose = true;
+    od = null;
     synchronized (this)
     {
       restart = true;
index e6bba02..6abee08 100755 (executable)
@@ -276,7 +276,9 @@ public class ScalePanel extends JPanel
   {
     mouseDragging = false;
 
-    int res = (evt.getX() / av.getCharWidth())
+    int xCords = Math.max(0, evt.getX()); // prevent negative X coordinates
+
+    int res = (xCords / av.getCharWidth())
             + av.getRanges().getStartRes();
 
     if (av.hasHiddenColumns())
index 7d14662..51e7645 100644 (file)
@@ -321,8 +321,8 @@ public class FeatureColour implements FeatureColourI
       } catch (Exception e)
       {
         throw new IllegalArgumentException(
-                "Couldn't parse the minimum value for graduated colour ("
-                        + descriptor + ")");
+                "Couldn't parse the minimum value for graduated colour ('"
+                        + minval + "')");
       }
       try
       {
index 061e70c..41e4d0d 100644 (file)
@@ -670,7 +670,8 @@ public class DBRefFetcher implements Runnable
             int startShift = absStart - sequenceStart + 1;
             if (startShift != 0)
             {
-              modified |= sequence.getFeatures().shiftFeatures(startShift);
+              modified |= sequence.getFeatures().shiftFeatures(1,
+                      startShift);
             }
           }
         }
index 155f00e..2160657 100644 (file)
@@ -21,8 +21,8 @@
 package jalview.commands;
 
 import static org.testng.AssertJUnit.assertEquals;
-import static org.testng.AssertJUnit.assertNull;
 import static org.testng.AssertJUnit.assertSame;
+import static org.testng.AssertJUnit.assertTrue;
 
 import jalview.commands.EditCommand.Action;
 import jalview.commands.EditCommand.Edit;
@@ -34,6 +34,8 @@ import jalview.datamodel.SequenceI;
 import jalview.datamodel.features.SequenceFeatures;
 import jalview.gui.JvOptionPane;
 
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 
@@ -50,6 +52,22 @@ import org.testng.annotations.Test;
  */
 public class EditCommandTest
 {
+  private static Comparator<SequenceFeature> BY_DESCRIPTION = new Comparator<SequenceFeature>()
+  {
+
+    @Override
+    public int compare(SequenceFeature o1, SequenceFeature o2)
+    {
+      return o1.getDescription().compareTo(o2.getDescription());
+    }
+  };
+
+  private EditCommand testee;
+
+  private SequenceI[] seqs;
+
+  private Alignment al;
+
   /*
    * compute n(n+1)/2 e.g. 
    * func(5) = 5 + 4 + 3 + 2 + 1 = 15
@@ -66,12 +84,6 @@ public class EditCommandTest
     JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION);
   }
 
-  private EditCommand testee;
-
-  private SequenceI[] seqs;
-
-  private Alignment al;
-
   @BeforeMethod(alwaysRun = true)
   public void setUp()
   {
@@ -141,18 +153,23 @@ public class EditCommandTest
   }
 
   /**
-   * Test a Paste action, where this adds sequences to an alignment.
+   * Test a Paste action, followed by Undo and Redo
    */
   @Test(groups = { "Functional" }, enabled = false)
-  // TODO fix so it works
-  public void testPaste_addToAlignment()
+  public void testPaste_undo_redo()
   {
+    // TODO code this test properly, bearing in mind that:
+    // Paste action requires something on the clipboard (Cut/Copy)
+    // - EditCommand.paste doesn't add sequences to the alignment
+    // ... that is done in AlignFrame.paste()
+    // ... unless as a Redo
+    // ...
+
     SequenceI[] newSeqs = new SequenceI[2];
     newSeqs[0] = new Sequence("newseq0", "ACEFKL");
     newSeqs[1] = new Sequence("newseq1", "JWMPDH");
 
-    Edit ec = testee.new Edit(Action.PASTE, newSeqs, 0, al.getWidth(), al);
-    EditCommand.paste(ec, new AlignmentI[] { al });
+    new EditCommand("Paste", Action.PASTE, newSeqs, 0, al.getWidth(), al);
     assertEquals(6, al.getSequences().size());
     assertEquals("1234567890", seqs[3].getSequenceAsString());
     assertEquals("ACEFKL", seqs[4].getSequenceAsString());
@@ -262,12 +279,123 @@ public class EditCommandTest
   {
     // seem to need a dataset sequence on the edited sequence here
     seqs[1].createDatasetSequence();
-    new EditCommand("", Action.REPLACE, "ZXY", new SequenceI[] { seqs[1] },
+    assertEquals("fghjklmnopq", seqs[1].getSequenceAsString());
+    // NB command.number holds end position for a Replace command
+    new EditCommand("", Action.REPLACE, "Z-xY", new SequenceI[] { seqs[1] },
             4, 8, al);
     assertEquals("abcdefghjk", seqs[0].getSequenceAsString());
+    assertEquals("fghjZ-xYopq", seqs[1].getSequenceAsString());
+    assertEquals("fghjZxYopq",
+            seqs[1].getDatasetSequence().getSequenceAsString());
     assertEquals("qrstuvwxyz", seqs[2].getSequenceAsString());
     assertEquals("1234567890", seqs[3].getSequenceAsString());
-    seqs[1] = new Sequence("seq1", "fghjZXYnopq");
+  }
+
+  /**
+   * Test the replace command (used to manually edit a sequence)
+   */
+  @Test(groups = { "Functional" })
+  public void testReplace_withGaps()
+  {
+    SequenceI seq = new Sequence("seq", "ABC--DEF");
+    seq.createDatasetSequence();
+    assertEquals("ABCDEF", seq.getDatasetSequence().getSequenceAsString());
+    assertEquals(1, seq.getStart());
+    assertEquals(6, seq.getEnd());
+
+    /*
+     * replace C- with XYZ
+     * NB arg4 = start column of selection for edit (base 0)
+     * arg5 = column after end of selection for edit
+     */
+    EditCommand edit = new EditCommand("", Action.REPLACE, "xyZ",
+            new SequenceI[]
+            { seq }, 2,
+            4, al);
+    assertEquals("ABxyZ-DEF", seq.getSequenceAsString());
+    assertEquals(1, seq.getStart());
+    assertEquals(8, seq.getEnd());
+    assertEquals("ABxyZDEF",
+            seq.getDatasetSequence().getSequenceAsString());
+    assertEquals(8, seq.getDatasetSequence().getEnd());
+
+    /*
+     * undo the edit
+     */
+    AlignmentI[] views = new AlignmentI[]
+    { new Alignment(new SequenceI[] { seq }) };
+    edit.undoCommand(views);
+
+    assertEquals("ABC--DEF", seq.getSequenceAsString());
+    assertEquals("ABCDEF", seq.getDatasetSequence().getSequenceAsString());
+    assertEquals(1, seq.getStart());
+    assertEquals(6, seq.getEnd());
+    assertEquals(6, seq.getDatasetSequence().getEnd());
+
+    /*
+     * redo the edit
+     */
+    edit.doCommand(views);
+
+    assertEquals("ABxyZ-DEF", seq.getSequenceAsString());
+    assertEquals(1, seq.getStart());
+    assertEquals(8, seq.getEnd());
+    assertEquals("ABxyZDEF",
+            seq.getDatasetSequence().getSequenceAsString());
+    assertEquals(8, seq.getDatasetSequence().getEnd());
+
+  }
+
+  /**
+   * Test replace command when it doesn't cause a sequence edit (see comment in
+   */
+  @Test(groups = { "Functional" })
+  public void testReplaceFirstResiduesWithGaps()
+  {
+    // test replace when gaps are inserted at start. Start/end should change
+    // w.r.t. original edited sequence.
+    SequenceI dsseq = seqs[1].getDatasetSequence();
+    EditCommand edit = new EditCommand("", Action.REPLACE, "----",
+            new SequenceI[]
+            { seqs[1] }, 0, 4, al);
+
+    // trimmed start
+    assertEquals("----klmnopq", seqs[1].getSequenceAsString());
+    // and ds is preserved
+    assertTrue(dsseq == seqs[1].getDatasetSequence());
+    // and it is unchanged
+    assertEquals("fghjklmnopq", dsseq.getSequenceAsString());
+    // and that alignment sequence start has been adjusted
+    assertEquals(5, seqs[1].getStart());
+    assertEquals(11, seqs[1].getEnd());
+
+    AlignmentI[] views = new AlignmentI[] { new Alignment(seqs) };
+    // and undo
+    edit.undoCommand(views);
+
+    // dataset sequence unchanged
+    assertTrue(dsseq == seqs[1].getDatasetSequence());
+    // restore sequence
+    assertEquals("fghjklmnopq", seqs[1].getSequenceAsString());
+    // and start/end numbering also restored
+    assertEquals(1, seqs[1].getStart());
+    assertEquals(11, seqs[1].getEnd());
+
+    // now redo
+    edit.undoCommand(views);
+
+    // and repeat asserts for the original edit
+
+    // trimmed start
+    assertEquals("----klmnopq", seqs[1].getSequenceAsString());
+    // and ds is preserved
+    assertTrue(dsseq == seqs[1].getDatasetSequence());
+    // and it is unchanged
+    assertEquals("fghjklmnopq", dsseq.getSequenceAsString());
+    // and that alignment sequence start has been adjusted
+    assertEquals(5, seqs[1].getStart());
+    assertEquals(11, seqs[1].getEnd());
+
   }
 
   /**
@@ -663,7 +791,7 @@ public class EditCommandTest
      * create sequence features before, after and overlapping
      * a cut of columns/residues 4-7
      */
-    SequenceI seq0 = seqs[0];
+    SequenceI seq0 = seqs[0]; // abcdefghjk/1-10
     seq0.addSequenceFeature(new SequenceFeature("before", "", 1, 3, 0f,
             null));
     seq0.addSequenceFeature(new SequenceFeature("overlap left", "", 2, 6,
@@ -675,29 +803,52 @@ public class EditCommandTest
     seq0.addSequenceFeature(new SequenceFeature("after", "", 8, 10, 0f,
             null));
 
+    /*
+     * add some contact features
+     */
+    SequenceFeature internalContact = new SequenceFeature("disulphide bond", "", 5,
+            6, 0f, null);
+    seq0.addSequenceFeature(internalContact); // should get deleted
+    SequenceFeature overlapLeftContact = new SequenceFeature(
+            "disulphide bond", "", 2, 6, 0f, null);
+    seq0.addSequenceFeature(overlapLeftContact); // should get deleted
+    SequenceFeature overlapRightContact = new SequenceFeature(
+            "disulphide bond", "", 5, 8, 0f, null);
+    seq0.addSequenceFeature(overlapRightContact); // should get deleted
+    SequenceFeature spanningContact = new SequenceFeature(
+            "disulphide bond", "", 2, 9, 0f, null);
+    seq0.addSequenceFeature(spanningContact); // should get shortened 3'
+
+    /*
+     * cut columns 3-6 (base 0), residues d-g 4-7
+     */
     Edit ec = testee.new Edit(Action.CUT, seqs, 3, 4, al); // cols 3-6 base 0
     EditCommand.cut(ec, new AlignmentI[] { al });
 
     List<SequenceFeature> sfs = seq0.getSequenceFeatures();
     SequenceFeatures.sortFeatures(sfs, true);
 
-    assertEquals(4, sfs.size()); // feature internal to cut has been deleted
+    assertEquals(5, sfs.size()); // features internal to cut were deleted
     SequenceFeature sf = sfs.get(0);
     assertEquals("before", sf.getType());
     assertEquals(1, sf.getBegin());
     assertEquals(3, sf.getEnd());
     sf = sfs.get(1);
+    assertEquals("disulphide bond", sf.getType());
+    assertEquals(2, sf.getBegin());
+    assertEquals(5, sf.getEnd()); // truncated by cut
+    sf = sfs.get(2);
     assertEquals("overlap left", sf.getType());
     assertEquals(2, sf.getBegin());
     assertEquals(3, sf.getEnd()); // truncated by cut
-    sf = sfs.get(2);
-    assertEquals("overlap right", sf.getType());
-    assertEquals(4, sf.getBegin()); // shifted left by cut
-    assertEquals(5, sf.getEnd()); // truncated by cut
     sf = sfs.get(3);
     assertEquals("after", sf.getType());
     assertEquals(4, sf.getBegin()); // shifted left by cut
     assertEquals(6, sf.getEnd()); // shifted left by cut
+    sf = sfs.get(4);
+    assertEquals("overlap right", sf.getType());
+    assertEquals(4, sf.getBegin()); // shifted left by cut
+    assertEquals(4, sf.getEnd()); // truncated by cut
   }
 
   /**
@@ -711,16 +862,28 @@ public class EditCommandTest
      * create a sequence features on each subrange of 1-5
      */
     SequenceI seq0 = new Sequence("seq", "ABCDE");
+    int start = 8;
+    int end = 12;
+    seq0.setStart(start);
+    seq0.setEnd(end);
     AlignmentI alignment = new Alignment(new SequenceI[] { seq0 });
     alignment.setDataset(null);
-    for (int from = 1; from <= seq0.getLength(); from++)
+
+    /*
+     * create a new alignment with shared dataset sequence
+     */
+    AlignmentI copy = new Alignment(
+            new SequenceI[]
+            { alignment.getDataset().getSequenceAt(0).deriveSequence() });
+    SequenceI copySeq0 = copy.getSequenceAt(0);
+
+    for (int from = start; from <= end; from++)
     {
-      for (int to = from; to <= seq0.getLength(); to++)
+      for (int to = from; to <= end; to++)
       {
         String desc = String.format("%d-%d", from, to);
         SequenceFeature sf = new SequenceFeature("test", desc, from, to,
-                0f,
-                null);
+                0f, null);
         sf.setValue("from", Integer.valueOf(from));
         sf.setValue("to", Integer.valueOf(to));
         seq0.addSequenceFeature(sf);
@@ -729,109 +892,234 @@ public class EditCommandTest
     // sanity check
     List<SequenceFeature> sfs = seq0.getSequenceFeatures();
     assertEquals(func(5), sfs.size());
+    assertEquals(sfs, copySeq0.getSequenceFeatures());
+    String copySequenceFeatures = copySeq0.getSequenceFeatures().toString();
 
     /*
-     * now perform all possible cuts of subranges of 1-5 (followed by Undo)
+     * now perform all possible cuts of subranges of columns 1-5
      * and validate the resulting remaining sequence features!
      */
     SequenceI[] sqs = new SequenceI[] { seq0 };
 
-    // goal is to have this passing for all from/to values!!
-    // for (int from = 0; from < seq0.getLength(); from++)
-    // {
-    // for (int to = from; to < seq0.getLength(); to++)
-    for (int from = 1; from < 3; from++)
+    for (int from = 0; from < seq0.getLength(); from++)
     {
-      for (int to = 2; to < 3; to++)
+      for (int to = from; to < seq0.getLength(); to++)
       {
-        testee.appendEdit(Action.CUT, sqs, from, (to - from + 1),
-                alignment, true);
+        EditCommand ec = new EditCommand("Cut", Action.CUT, sqs, from, (to
+                - from + 1), alignment);
+        final String msg = String.format("Cut %d-%d ", from + 1, to + 1);
+        boolean newDatasetSequence = copySeq0.getDatasetSequence() != seq0
+                .getDatasetSequence();
+
+        verifyCut(seq0, from, to, msg, start);
+
+        /*
+         * verify copy alignment dataset sequence unaffected
+         */
+        assertEquals("Original dataset sequence was modified",
+                copySequenceFeatures,
+                copySeq0.getSequenceFeatures().toString());
 
+        /*
+         * verify any new dataset sequence was added to the
+         * alignment dataset
+         */
+        assertEquals("Wrong Dataset size after " + msg,
+                newDatasetSequence ? 2 : 1,
+                alignment.getDataset().getHeight());
+
+        /*
+         * undo and verify all restored
+         */
+        AlignmentI[] views = new AlignmentI[] { alignment };
+        ec.undoCommand(views);
         sfs = seq0.getSequenceFeatures();
+        assertEquals("After undo of " + msg, func(5), sfs.size());
+        verifyUndo(from, to, sfs);
+
+        /*
+         * verify copy alignment dataset sequence still unaffected
+         * and alignment dataset has shrunk (if it was added to)
+         */
+        assertEquals("Original dataset sequence was modified",
+                copySequenceFeatures,
+                copySeq0.getSequenceFeatures().toString());
+        assertEquals("Wrong Dataset size after Undo of " + msg, 1,
+                alignment.getDataset().getHeight());
 
         /*
-         * confirm the number of features has reduced by the
-         * number of features within the cut region i.e. by
-         * func(length of cut)
+         * redo and verify
          */
-        String msg = String.format("Cut %d-%d ", from, to);
-        if (to - from == 4)
-        {
-          // all columns cut
-          assertNull(sfs);
-        }
-        else
-        {
-          assertEquals(msg + "wrong number of features left", func(5)
-                  - func(to - from + 1), sfs.size());
-        }
+        ec.doCommand(views);
+        verifyCut(seq0, from, to, msg, start);
 
         /*
-         * inspect individual features
+         * verify copy alignment dataset sequence unaffected
+         * and any new dataset sequence readded to alignment dataset
          */
-        if (sfs != null)
-        {
-          for (SequenceFeature sf : sfs)
-          {
-            checkFeatureRelocation(sf, from + 1, to + 1);
-          }
-        }
+        assertEquals("Original dataset sequence was modified",
+                copySequenceFeatures,
+                copySeq0.getSequenceFeatures().toString());
+        assertEquals("Wrong Dataset size after Redo of " + msg,
+                newDatasetSequence ? 2 : 1,
+                alignment.getDataset().getHeight());
+
         /*
          * undo ready for next cut
          */
-        testee.undoCommand(new AlignmentI[] { alignment });
-        assertEquals(func(5), seq0.getSequenceFeatures().size());
+        ec.undoCommand(views);
+
+        /*
+         * final verify that copy alignment dataset sequence is still unaffected
+         * and that alignment dataset has shrunk
+         */
+        assertEquals("Original dataset sequence was modified",
+                copySequenceFeatures,
+                copySeq0.getSequenceFeatures().toString());
+        assertEquals("Wrong Dataset size after final Undo of " + msg, 1,
+                alignment.getDataset().getHeight());
       }
     }
   }
 
   /**
+   * Verify by inspection that the sequence features left on the sequence after
+   * a cut match the expected results. The trick to this is that we can parse
+   * each feature's original start-end positions from its description.
+   * 
+   * @param seq0
+   * @param from
+   * @param to
+   * @param msg
+   * @param seqStart
+   */
+  protected void verifyCut(SequenceI seq0, int from, int to,
+          final String msg, int seqStart)
+  {
+    List<SequenceFeature> sfs;
+    sfs = seq0.getSequenceFeatures();
+
+    Collections.sort(sfs, BY_DESCRIPTION);
+
+    /*
+     * confirm the number of features has reduced by the
+     * number of features within the cut region i.e. by
+     * func(length of cut); exception is a cut at start or end of sequence, 
+     * which retains the original coordinates, dataset sequence 
+     * and all its features
+     */
+    boolean datasetRetained = from == 0 || to == 4;
+    if (datasetRetained)
+    {
+      // dataset and all features retained
+      assertEquals(msg, func(5), sfs.size());
+    }
+    else if (to - from == 4)
+    {
+      // all columns were cut
+      assertTrue(sfs.isEmpty());
+    }
+    else
+    {
+      // failure in checkFeatureRelocation is more informative!
+      assertEquals(msg + "wrong number of features left", func(5)
+              - func(to - from + 1), sfs.size());
+    }
+
+    /*
+     * inspect individual features
+     */
+    for (SequenceFeature sf : sfs)
+    {
+      verifyFeatureRelocation(sf, from + 1, to + 1, !datasetRetained,
+              seqStart);
+    }
+  }
+
+  /**
+   * Check that after Undo, every feature has start/end that match its original
+   * "start" and "end" properties
+   * 
+   * @param from
+   * @param to
+   * @param sfs
+   */
+  protected void verifyUndo(int from, int to, List<SequenceFeature> sfs)
+  {
+    for (SequenceFeature sf : sfs)
+    {
+      final int oldFrom = ((Integer) sf.getValue("from")).intValue();
+      final int oldTo = ((Integer) sf.getValue("to")).intValue();
+      String msg = String.format(
+              "Undo cut of [%d-%d], feature at [%d-%d] ", from + 1, to + 1,
+              oldFrom, oldTo);
+      assertEquals(msg + "start", oldFrom, sf.getBegin());
+      assertEquals(msg + "end", oldTo, sf.getEnd());
+    }
+  }
+
+  /**
    * Helper method to check a feature has been correctly relocated after a cut
    * 
    * @param sf
    * @param from
-   *          start of cut (first residue cut)
+   *          start of cut (first residue cut 1..)
    * @param to
-   *          end of cut (last residue cut)
+   *          end of cut (last residue cut 1..)
+   * @param newDataset
+   * @param seqStart
    */
-  private void checkFeatureRelocation(SequenceFeature sf, int from, int to)
+  private void verifyFeatureRelocation(SequenceFeature sf, int from, int to,
+ boolean newDataset, int seqStart)
   {
     // TODO handle the gapped sequence case as well
     int cutSize = to - from + 1;
-    int oldFrom = ((Integer) sf.getValue("from")).intValue();
-    int oldTo = ((Integer) sf.getValue("to")).intValue();
+    final int oldFrom = ((Integer) sf.getValue("from")).intValue();
+    final int oldTo = ((Integer) sf.getValue("to")).intValue();
+    final int oldFromPosition = oldFrom - seqStart + 1; // 1..
+    final int oldToPosition = oldTo - seqStart + 1; // 1..
 
     String msg = String.format(
             "Feature %s relocated to %d-%d after cut of %d-%d",
             sf.getDescription(), sf.getBegin(), sf.getEnd(), from, to);
-    if (oldTo < from)
+    if (!newDataset)
+    {
+      // dataset retained with all features unchanged
+      assertEquals("0: " + msg, oldFrom, sf.getBegin());
+      assertEquals("0: " + msg, oldTo, sf.getEnd());
+    }
+    else if (oldToPosition < from)
     {
       // before cut region so unchanged
       assertEquals("1: " + msg, oldFrom, sf.getBegin());
       assertEquals("2: " + msg, oldTo, sf.getEnd());
     }
-    else if (oldFrom > to)
+    else if (oldFromPosition > to)
     {
       // follows cut region - shift by size of cut
-      assertEquals("3: " + msg, oldFrom - cutSize, sf.getBegin());
-      assertEquals("4: " + msg, oldTo - cutSize, sf.getEnd());
+      assertEquals("3: " + msg, newDataset ? oldFrom - cutSize : oldFrom,
+              sf.getBegin());
+      assertEquals("4: " + msg, newDataset ? oldTo - cutSize : oldTo,
+              sf.getEnd());
     }
-    else if (oldFrom < from && oldTo > to)
+    else if (oldFromPosition < from && oldToPosition > to)
     {
       // feature encloses cut region - shrink it right
       assertEquals("5: " + msg, oldFrom, sf.getBegin());
       assertEquals("6: " + msg, oldTo - cutSize, sf.getEnd());
     }
-    else if (oldFrom < from)
+    else if (oldFromPosition < from)
     {
       // feature overlaps left side of cut region - truncated right
-      assertEquals("7: " + msg, from - 1, sf.getEnd());
+      assertEquals("7: " + msg, from - 1 + seqStart - 1, sf.getEnd());
     }
-    else if (oldTo > to)
+    else if (oldToPosition > to)
     {
       // feature overlaps right side of cut region - truncated left
-      assertEquals("8: " + msg, from, sf.getBegin());
-      assertEquals("9: " + msg, from + oldTo - to - 1, sf.getEnd());
+      assertEquals("8: " + msg, newDataset ? from + seqStart - 1 : to + 1,
+              sf.getBegin());
+      assertEquals("9: " + msg, newDataset ? from + oldTo - to - 1 : oldTo,
+              sf.getEnd());
     }
     else
     {
@@ -844,30 +1132,32 @@ public class EditCommandTest
    * Test a cut action's relocation of sequence features
    */
   @Test(groups = { "Functional" })
-  public void testCut_gappedWithFeatures()
+  public void testCut_withFeatures5prime()
   {
+    SequenceI seq0 = new Sequence("seq/8-11", "A-BCC");
+    seq0.createDatasetSequence();
+    assertEquals(8, seq0.getStart());
+    seq0.addSequenceFeature(new SequenceFeature("", "", 10, 11, 0f,
+            null));
+    SequenceI[] seqsArray = new SequenceI[] { seq0 };
+    AlignmentI alignment = new Alignment(seqsArray);
+
     /*
-     * create sequence features before, after and overlapping
-     * a cut of columns/residues 4-7
+     * cut columns of A-B; same dataset sequence is retained, aligned sequence
+     * start becomes 10
      */
-    SequenceI seq0 = new Sequence("seq", "A-BCC");
-    seq0.addSequenceFeature(new SequenceFeature("", "", 3, 4, 0f,
-            null));
-    AlignmentI alignment = new Alignment(new SequenceI[] { seq0 });
-    // cut columns of A-B
-    Edit ec = testee.new Edit(Action.CUT, seqs, 0, 3, alignment); // cols 0-3
-                                                                  // base 0
+    Edit ec = testee.new Edit(Action.CUT, seqsArray, 0, 3, alignment);
     EditCommand.cut(ec, new AlignmentI[] { alignment });
   
     /*
-     * feature on CC(3-4) should now be on CC(1-2)
+     * feature on CC(10-11) should still be on CC(10-11)
      */
+    assertSame(seq0, alignment.getSequenceAt(0));
+    assertEquals(10, seq0.getStart());
     List<SequenceFeature> sfs = seq0.getSequenceFeatures();
     assertEquals(1, sfs.size());
     SequenceFeature sf = sfs.get(0);
-    assertEquals(1, sf.getBegin());
-    assertEquals(2, sf.getEnd());
-
-    // TODO add further cases including Undo - see JAL-2541
+    assertEquals(10, sf.getBegin());
+    assertEquals(11, sf.getEnd());
   }
 }
index 79bb2bb..9629b6f 100644 (file)
@@ -290,6 +290,61 @@ public class SequenceTest
     assertEquals(0, sq.findIndex(2));
   }
 
+  @Test(groups = { "Functional" })
+  public void testFindPositions()
+  {
+    SequenceI sq = new Sequence("test/8-13", "-ABC---DE-F--");
+
+    /*
+     * invalid inputs
+     */
+    assertNull(sq.findPositions(6, 5));
+    assertNull(sq.findPositions(0, 5));
+    assertNull(sq.findPositions(-1, 5));
+
+    /*
+     * all gapped ranges
+     */
+    assertNull(sq.findPositions(1, 1)); // 1-based columns
+    assertNull(sq.findPositions(5, 5));
+    assertNull(sq.findPositions(5, 6));
+    assertNull(sq.findPositions(5, 7));
+
+    /*
+     * all ungapped ranges
+     */
+    assertEquals(new Range(8, 8), sq.findPositions(2, 2)); // A
+    assertEquals(new Range(8, 9), sq.findPositions(2, 3)); // AB
+    assertEquals(new Range(8, 10), sq.findPositions(2, 4)); // ABC
+    assertEquals(new Range(9, 10), sq.findPositions(3, 4)); // BC
+
+    /*
+     * gap to ungapped range
+     */
+    assertEquals(new Range(8, 10), sq.findPositions(1, 4)); // ABC
+    assertEquals(new Range(11, 12), sq.findPositions(6, 9)); // DE
+
+    /*
+     * ungapped to gapped range
+     */
+    assertEquals(new Range(10, 10), sq.findPositions(4, 5)); // C
+    assertEquals(new Range(9, 13), sq.findPositions(3, 11)); // BCDEF
+
+    /*
+     * ungapped to ungapped enclosing gaps
+     */
+    assertEquals(new Range(10, 11), sq.findPositions(4, 8)); // CD
+    assertEquals(new Range(8, 13), sq.findPositions(2, 11)); // ABCDEF
+
+    /*
+     * gapped to gapped enclosing ungapped
+     */
+    assertEquals(new Range(8, 10), sq.findPositions(1, 5)); // ABC
+    assertEquals(new Range(11, 12), sq.findPositions(5, 10)); // DE
+    assertEquals(new Range(8, 13), sq.findPositions(1, 13)); // the lot
+    assertEquals(new Range(8, 13), sq.findPositions(1, 99));
+  }
+
   /**
    * Tests for the method that returns a dataset sequence position (start..) for
    * an aligned column position (base 0).
@@ -465,8 +520,7 @@ public class SequenceTest
     assertEquals("test:Pos13:Col10:startCol3:endCol10:tok0",
             PA.getValue(sq, "cursor").toString());
     sq.sequenceChanged();
-    assertEquals(12, sq.findPosition(8));
-    cursor = (SequenceCursor) PA.getValue(sq, "cursor");
+    assertEquals(12, sq.findPosition(8)); // E12
     // sequenceChanged() invalidates cursor.lastResidueColumn
     cursor = (SequenceCursor) PA.getValue(sq, "cursor");
     assertEquals("test:Pos12:Col9:startCol3:endCol0:tok1",
@@ -505,6 +559,13 @@ public class SequenceTest
     assertEquals(6, sq.getEnd());
     assertNull(PA.getValue(sq, "datasetSequence"));
 
+    sq = new Sequence("test", "ABCDE");
+    sq.deleteChars(0, 3);
+    assertEquals("DE", sq.getSequenceAsString());
+    assertEquals(4, sq.getStart());
+    assertEquals(5, sq.getEnd());
+    assertNull(PA.getValue(sq, "datasetSequence"));
+
     /*
      * delete at end
      */
@@ -514,6 +575,21 @@ public class SequenceTest
     assertEquals(1, sq.getStart());
     assertEquals(4, sq.getEnd());
     assertNull(PA.getValue(sq, "datasetSequence"));
+
+    /*
+     * delete more positions than there are
+     */
+    sq = new Sequence("test/8-11", "ABCD");
+    sq.deleteChars(0, 99);
+    assertEquals("", sq.getSequenceAsString());
+    assertEquals(12, sq.getStart()); // = findPosition(99) ?!?
+    assertEquals(11, sq.getEnd());
+
+    sq = new Sequence("test/8-11", "----");
+    sq.deleteChars(0, 99); // ArrayIndexOutOfBoundsException <= 2.10.2
+    assertEquals("", sq.getSequenceAsString());
+    assertEquals(8, sq.getStart());
+    assertEquals(11, sq.getEnd());
   }
 
   @Test(groups = { "Functional" })
@@ -1614,6 +1690,10 @@ public class SequenceTest
     // cursor should now be at [D 6]
     cursor = (SequenceCursor) PA.getValue(sq, "cursor");
     assertEquals(new SequenceCursor(sq, 11, 6, ++token), cursor);
+    assertEquals(0, cursor.lastColumnPosition); // not yet found
+    assertEquals(13, sq.findPosition(8)); // E13
+    cursor = (SequenceCursor) PA.getValue(sq, "cursor");
+    assertEquals(9, cursor.lastColumnPosition); // found
 
     /*
      * deleteChars should invalidate the cached cursor
@@ -1693,61 +1773,6 @@ public class SequenceTest
   }
 
   @Test(groups = { "Functional" })
-  public void testFindPositions()
-  {
-    SequenceI sq = new Sequence("test/8-13", "-ABC---DE-F--");
-
-    /*
-     * invalid inputs
-     */
-    assertNull(sq.findPositions(6, 5));
-    assertNull(sq.findPositions(0, 5));
-    assertNull(sq.findPositions(-1, 5));
-
-    /*
-     * all gapped ranges
-     */
-    assertNull(sq.findPositions(1, 1)); // 1-based columns
-    assertNull(sq.findPositions(5, 5));
-    assertNull(sq.findPositions(5, 6));
-    assertNull(sq.findPositions(5, 7));
-
-    /*
-     * all ungapped ranges
-     */
-    assertEquals(new Range(8, 8), sq.findPositions(2, 2)); // A
-    assertEquals(new Range(8, 9), sq.findPositions(2, 3)); // AB
-    assertEquals(new Range(8, 10), sq.findPositions(2, 4)); // ABC
-    assertEquals(new Range(9, 10), sq.findPositions(3, 4)); // BC
-
-    /*
-     * gap to ungapped range
-     */
-    assertEquals(new Range(8, 10), sq.findPositions(1, 4)); // ABC
-    assertEquals(new Range(11, 12), sq.findPositions(6, 9)); // DE
-
-    /*
-     * ungapped to gapped range
-     */
-    assertEquals(new Range(10, 10), sq.findPositions(4, 5)); // C
-    assertEquals(new Range(9, 13), sq.findPositions(3, 11)); // BCDEF
-
-    /*
-     * ungapped to ungapped enclosing gaps
-     */
-    assertEquals(new Range(10, 11), sq.findPositions(4, 8)); // CD
-    assertEquals(new Range(8, 13), sq.findPositions(2, 11)); // ABCDEF
-
-    /*
-     * gapped to gapped enclosing ungapped
-     */
-    assertEquals(new Range(8, 10), sq.findPositions(1, 5)); // ABC
-    assertEquals(new Range(11, 12), sq.findPositions(5, 10)); // DE
-    assertEquals(new Range(8, 13), sq.findPositions(1, 13)); // the lot
-    assertEquals(new Range(8, 13), sq.findPositions(1, 99));
-  }
-
-  @Test(groups = { "Functional" })
   public void testGapBitset()
   {
     SequenceI sq = new Sequence("test/8-13", "-ABC---DE-F--");
index db21c2f..6e7dd02 100644 (file)
@@ -2,6 +2,7 @@ package jalview.datamodel.features;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertSame;
 import static org.testng.Assert.assertTrue;
 
 import jalview.datamodel.SequenceFeature;
@@ -774,7 +775,7 @@ public class FeatureStoreTest
   public void testShiftFeatures()
   {
     FeatureStore fs = new FeatureStore();
-    assertFalse(fs.shiftFeatures(1));
+    assertFalse(fs.shiftFeatures(0, 1)); // nothing to do
 
     SequenceFeature sf1 = new SequenceFeature("Cath", "", 2, 5, 0f, null);
     fs.addFeature(sf1);
@@ -790,9 +791,9 @@ public class FeatureStoreTest
     fs.addFeature(sf4);
 
     /*
-     * shift features right by 5
+     * shift all features right by 5
      */
-    assertTrue(fs.shiftFeatures(5));
+    assertTrue(fs.shiftFeatures(0, 5));
 
     // non-positional features untouched:
     List<SequenceFeature> nonPos = fs.getNonPositionalFeatures();
@@ -818,7 +819,7 @@ public class FeatureStoreTest
      * feature at [7-10] should be removed
      * feature at [13-19] should become [1-4] 
      */
-    assertTrue(fs.shiftFeatures(-15));
+    assertTrue(fs.shiftFeatures(0, -15));
     pos = fs.getPositionalFeatures();
     assertEquals(pos.size(), 2);
     SequenceFeatures.sortFeatures(pos, true);
@@ -826,6 +827,32 @@ public class FeatureStoreTest
     assertEquals(pos.get(0).getEnd(), 4);
     assertEquals(pos.get(1).getBegin(), 13);
     assertEquals(pos.get(1).getEnd(), 22);
+
+    /*
+     * shift right by 4 from position 2 onwards
+     * feature at [1-4] unchanged, feature at [13-22] shifts
+     */
+    assertTrue(fs.shiftFeatures(2, 4));
+    pos = fs.getPositionalFeatures();
+    assertEquals(pos.size(), 2);
+    SequenceFeatures.sortFeatures(pos, true);
+    assertEquals(pos.get(0).getBegin(), 1);
+    assertEquals(pos.get(0).getEnd(), 4);
+    assertEquals(pos.get(1).getBegin(), 17);
+    assertEquals(pos.get(1).getEnd(), 26);
+
+    /*
+     * shift right by 4 from position 18 onwards
+     * should be no change
+     */
+    SequenceFeature f1 = pos.get(0);
+    SequenceFeature f2 = pos.get(1);
+    assertFalse(fs.shiftFeatures(18, 4)); // no update
+    pos = fs.getPositionalFeatures();
+    assertEquals(pos.size(), 2);
+    SequenceFeatures.sortFeatures(pos, true);
+    assertSame(pos.get(0), f1);
+    assertSame(pos.get(1), f2);
   }
 
   @Test(groups = "Functional")
index 32987b0..29e76bb 100644 (file)
@@ -1161,7 +1161,7 @@ public class SequenceFeaturesTest
   public void testShiftFeatures()
   {
     SequenceFeatures store = new SequenceFeatures();
-    assertFalse(store.shiftFeatures(1));
+    assertFalse(store.shiftFeatures(0, 1));
 
     SequenceFeature sf1 = new SequenceFeature("Cath", "", 2, 5, 0f, null);
     store.add(sf1);
@@ -1179,7 +1179,7 @@ public class SequenceFeaturesTest
     /*
      * shift features right by 5
      */
-    assertTrue(store.shiftFeatures(5));
+    assertTrue(store.shiftFeatures(0, 5));
   
     // non-positional features untouched:
     List<SequenceFeature> nonPos = store.getNonPositionalFeatures();
@@ -1208,7 +1208,7 @@ public class SequenceFeaturesTest
      * feature at [7-10] should be removed
      * feature at [13-19] should become [1-4] 
      */
-    assertTrue(store.shiftFeatures(-15));
+    assertTrue(store.shiftFeatures(0, -15));
     pos = store.getPositionalFeatures();
     assertEquals(pos.size(), 2);
     SequenceFeatures.sortFeatures(pos, true);
@@ -1218,6 +1218,35 @@ public class SequenceFeaturesTest
     assertEquals(pos.get(1).getBegin(), 13);
     assertEquals(pos.get(1).getEnd(), 22);
     assertEquals(pos.get(1).getType(), "Disulfide bond");
+
+    /*
+     * shift right by 4 from column 2
+     * feature at [1-4] should be unchanged
+     * feature at [13-22] should become [17-26] 
+     */
+    assertTrue(store.shiftFeatures(2, 4));
+    pos = store.getPositionalFeatures();
+    assertEquals(pos.size(), 2);
+    SequenceFeatures.sortFeatures(pos, true);
+    assertEquals(pos.get(0).getBegin(), 1);
+    assertEquals(pos.get(0).getEnd(), 4);
+    assertEquals(pos.get(0).getType(), "Metal");
+    assertEquals(pos.get(1).getBegin(), 17);
+    assertEquals(pos.get(1).getEnd(), 26);
+    assertEquals(pos.get(1).getType(), "Disulfide bond");
+
+    /*
+     * shift right from column 18
+     * should be no updates
+     */
+    SequenceFeature f1 = pos.get(0);
+    SequenceFeature f2 = pos.get(1);
+    assertFalse(store.shiftFeatures(18, 6));
+    pos = store.getPositionalFeatures();
+    assertEquals(pos.size(), 2);
+    SequenceFeatures.sortFeatures(pos, true);
+    assertSame(pos.get(0), f1);
+    assertSame(pos.get(1), f2);
   }
 
   @Test(groups = "Functional")
@@ -1232,4 +1261,18 @@ public class SequenceFeaturesTest
     assertTrue(store.isOntologyTerm("junk", new String[] {}));
     assertTrue(store.isOntologyTerm("junk", (String[]) null));
   }
+
+  @Test(groups = "Functional")
+  public void testDeleteAll()
+  {
+    SequenceFeaturesI store = new SequenceFeatures();
+    assertFalse(store.hasFeatures());
+    store.deleteAll();
+    assertFalse(store.hasFeatures());
+    store.add(new SequenceFeature("Cath", "Desc", 12, 20, 0f, "Group"));
+    store.add(new SequenceFeature("Pfam", "Desc", 6, 12, 2f, "Group2"));
+    assertTrue(store.hasFeatures());
+    store.deleteAll();
+    assertFalse(store.hasFeatures());
+  }
 }
index 28c5cf0..69634a9 100644 (file)
@@ -44,12 +44,12 @@ public class TestHtsContigDb
   @Test(groups = "Functional")
   public final void testGetSequenceProxy() throws Exception
   {
-    String pathname = "test/jalview/ext/htsjdk/pgmb.fasta";
+    String pathname = "test/jalview/ext/htsjdk/pgmB.fasta";
     HtsContigDb db = new HtsContigDb("ADB", new File(pathname));
-    
+
     assertTrue(db.isValid());
     assertTrue(db.isIndexed()); // htsjdk opens the .fai file
-    
+
     SequenceI sq = db.getSequenceProxy("Deminut");
     assertNotNull(sq);
     assertEquals(sq.getLength(), 606);
@@ -60,7 +60,7 @@ public class TestHtsContigDb
     sq = db.getSequenceProxy("PPL_06716");
     assertNotNull(sq);
     assertEquals(sq.getLength(), 602);
-    
+
     // dict = db.getDictionary(f, truncate))
   }
 
@@ -73,7 +73,7 @@ public class TestHtsContigDb
     expectedExceptions = java.lang.IllegalArgumentException.class)
   public final void testGetSequenceProxy_indexed()
   {
-    String pathname = "test/jalview/ext/htsjdk/pgmb.fasta.fai";
+    String pathname = "test/jalview/ext/htsjdk/pgmB.fasta.fai";
     new HtsContigDb("ADB", new File(pathname));
     fail("Expected exception opening .fai file");
   }
@@ -92,17 +92,19 @@ public class TestHtsContigDb
   @Test(groups = "Functional")
   public void testCreateFastaSequenceIndex() throws IOException
   {
-    File fasta = new File("test/jalview/ext/htsjdk/pgmb.fasta");
-    
+    File fasta = new File("test/jalview/ext/htsjdk/pgmB.fasta");
+
     /*
      * create .fai with no overwrite fails if it exists
      */
-    try {
+    try
+    {
       HtsContigDb.createFastaSequenceIndex(fasta.toPath(), false);
       fail("Expected exception");
     } catch (IOException e)
     {
-      // expected
+      // we expect an IO Exception because the pgmB.fasta.fai exists, since it
+      // was checked it in.
     }
 
     /*
index f5e637c..bcad464 100644 (file)
@@ -62,9 +62,9 @@ public class JmolParserTest
    * 1GAQ has been reduced to alpha carbons only
    * 1QCF is the full PDB file including headers, HETATM etc
    */
-  String[] testFile = new String[] { "./examples/1GAQ.txt",
+  String[] testFile = new String[] { "./examples/1gaq.txt",
       "./test/jalview/ext/jmol/1xyz.pdb",
-      "./test/jalview/ext/jmol/1qcf.pdb" };
+      "./test/jalview/ext/jmol/1QCF.pdb" };
 
   //@formatter:off
   // a modified and very cut-down extract of 4UJ4
@@ -130,17 +130,17 @@ public class JmolParserTest
       JmolParser jtest = new JmolParser(pdbStr, DataSourceType.FILE);
       Vector<SequenceI> seqs = jtest.getSeqs(), mcseqs = mctest.getSeqs();
 
-      assertTrue(
-              "No sequences extracted from testfile\n"
-                      + (jtest.hasWarningMessage() ? jtest.getWarningMessage()
-                              : "(No warnings raised)"), seqs != null
-                      && seqs.size() > 0);
+      assertTrue("No sequences extracted from testfile\n"
+              + (jtest.hasWarningMessage() ? jtest.getWarningMessage()
+                      : "(No warnings raised)"),
+              seqs != null && seqs.size() > 0);
       for (SequenceI sq : seqs)
       {
-        assertEquals("JMol didn't process " + pdbStr
-                + " to the same sequence as MCView",
-                sq.getSequenceAsString(), mcseqs.remove(0)
-                        .getSequenceAsString());
+        assertEquals(
+                "JMol didn't process " + pdbStr
+                        + " to the same sequence as MCView",
+                sq.getSequenceAsString(),
+                mcseqs.remove(0).getSequenceAsString());
         AlignmentI al = new Alignment(new SequenceI[] { sq });
         validateSecStrRows(al);
       }
@@ -175,13 +175,15 @@ public class JmolParserTest
 
   private void checkFirstAAIsAssoc(SequenceI sq)
   {
-    assertTrue("No secondary structure assigned for protein sequence for "
-            + sq.getName(),
+    assertTrue(
+            "No secondary structure assigned for protein sequence for "
+                    + sq.getName(),
             sq.getAnnotation() != null && sq.getAnnotation().length >= 1
                     && sq.getAnnotation()[0].hasIcons);
     assertTrue(
             "Secondary structure not associated for sequence "
-                    + sq.getName(), sq.getAnnotation()[0].sequenceRef == sq);
+                    + sq.getName(),
+            sq.getAnnotation()[0].sequenceRef == sq);
   }
 
   /**
@@ -194,7 +196,8 @@ public class JmolParserTest
   {
     PDBfile mctest = new PDBfile(false, false, false,
             pastePDBDataWithChainBreak, DataSourceType.PASTE);
-    JmolParser jtest = new JmolParser(pastePDBDataWithChainBreak, DataSourceType.PASTE);
+    JmolParser jtest = new JmolParser(pastePDBDataWithChainBreak,
+            DataSourceType.PASTE);
     Vector<SequenceI> seqs = jtest.getSeqs();
     Vector<SequenceI> mcseqs = mctest.getSeqs();
 
@@ -216,8 +219,7 @@ public class JmolParserTest
   {
     PDBfile mctest = new PDBfile(false, false, false, pdbWithAltLoc,
             DataSourceType.PASTE);
-    JmolParser jtest = new JmolParser(pdbWithAltLoc,
-            DataSourceType.PASTE);
+    JmolParser jtest = new JmolParser(pdbWithAltLoc, DataSourceType.PASTE);
     Vector<SequenceI> seqs = jtest.getSeqs();
     Vector<SequenceI> mcseqs = mctest.getSeqs();
 
index 4d904cf..99394dc 100644 (file)
@@ -41,7 +41,7 @@ public class ChimeraConnect
     JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION);
   }
 
-  @Test(groups = { "Functional" })
+  @Test(groups = { "External" })
   public void testLaunchAndExit()
   {
     final StructureManager structureManager = new StructureManager(true);
index 5ed0cac..7801250 100644 (file)
@@ -277,7 +277,7 @@ public class AlignViewportTest
    * Test for JAL-1306 - conservation thread should run even when only Quality
    * (and not Conservation) is enabled in Preferences
    */
-  @Test(groups = { "Functional" })
+  @Test(groups = { "Functional" }, timeOut=2000)
   public void testUpdateConservation_qualityOnly()
   {
     Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS",
@@ -292,7 +292,24 @@ public class AlignViewportTest
             Boolean.FALSE.toString());
     AlignFrame af = new FileLoader().LoadFileWaitTillLoaded(
             "examples/uniref50.fa", DataSourceType.FILE);
-    AlignmentAnnotation[] anns = af.viewport.getAlignment()
+
+    /*
+     * wait for Conservation thread to complete
+     */
+    AlignViewport viewport = af.getViewport();
+    synchronized (this)
+    {
+      while (viewport.getAlignmentConservationAnnotation() != null)
+      {
+        try
+        {
+          wait(50);
+        } catch (InterruptedException e)
+        {
+        }
+      }
+    }
+    AlignmentAnnotation[] anns = viewport.getAlignment()
             .getAlignmentAnnotation();
     assertNotNull("No annotations found", anns);
     assertEquals("More than one annotation found", 1, anns.length);
index 2819dbf..e84b87a 100644 (file)
@@ -222,7 +222,7 @@ public class AlignmentPanelTest
 
   /**
    * Test that update layout reverts to original (unwrapped) values for endRes
-   * and endSeq when switching from wrapped to unwrapped mode (JAL-2739)
+   * when switching from wrapped back to unwrapped mode (JAL-2739)
    */
   @Test(groups = "Functional")
   public void TestUpdateLayout_endRes()
@@ -235,14 +235,14 @@ public class AlignmentPanelTest
     af.alignPanel.getAlignViewport().setWrapAlignment(true);
     af.alignPanel.updateLayout();
 
-    // endRes changes
+    // endRes has changed
     assertNotEquals(ranges.getEndRes(), endres);
 
     // unwrap
     af.alignPanel.getAlignViewport().setWrapAlignment(false);
     af.alignPanel.updateLayout();
 
-    // endRes and endSeq back to original values
+    // endRes back to original value
     assertEquals(ranges.getEndRes(), endres);
 
   }
index e93bfac..9b21274 100644 (file)
@@ -1,6 +1,7 @@
 package jalview.gui;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
 
 import jalview.analysis.AlignmentGenerator;
@@ -11,6 +12,7 @@ import jalview.datamodel.SequenceGroup;
 import jalview.io.DataSourceType;
 import jalview.io.FileLoader;
 
+import java.awt.event.MouseEvent;
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
@@ -18,6 +20,8 @@ import java.io.PrintStream;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import junit.extensions.PA;
+
 public class FreeUpMemoryTest
 {
   private static final int ONE_MB = 1000 * 1000;
@@ -30,16 +34,12 @@ public class FreeUpMemoryTest
   {
     Jalview.main(new String[] { "-nonews", "-props",
         "test/jalview/testProps.jvprops" });
-    Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS",
-            Boolean.TRUE.toString());
-    Cache.applicationProperties.setProperty("SHOW_QUALITY",
-            Boolean.TRUE.toString());
-    Cache.applicationProperties.setProperty("SHOW_CONSERVATION",
-            Boolean.TRUE.toString());
-    Cache.applicationProperties.setProperty("SHOW_OCCUPANCY",
-            Boolean.TRUE.toString());
-    Cache.applicationProperties.setProperty("SHOW_IDENTITY",
-            Boolean.TRUE.toString());
+    String True = Boolean.TRUE.toString();
+    Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS", True);
+    Cache.applicationProperties.setProperty("SHOW_QUALITY", True);
+    Cache.applicationProperties.setProperty("SHOW_CONSERVATION", True);
+    Cache.applicationProperties.setProperty("SHOW_OCCUPANCY", True);
+    Cache.applicationProperties.setProperty("SHOW_IDENTITY", True);
   }
 
   /**
@@ -84,18 +84,19 @@ public class FreeUpMemoryTest
   protected void checkUsedMemory(long expectedMax)
   {
     /*
-     * request garbage collection and wait briefly for it to run;
+     * request garbage collection and wait for it to run;
      * NB there is no guarantee when, or whether, it will do so
+     * wait time depends on JRE/processor, generous allowance here  
      */
     System.gc();
-    waitFor(100);
+    waitFor(1500);
 
     /*
      * a second gc() call should not be necessary - but it is!
      * the test passes with it, and fails without it
      */
     System.gc();
-    waitFor(100);
+    waitFor(1500);
 
     /*
      * check used memory is 'reasonably low'
@@ -142,6 +143,20 @@ public class FreeUpMemoryTest
     }
 
     /*
+     * open an Overview window
+     */
+    af.overviewMenuItem_actionPerformed(null);
+    assertNotNull(af.alignPanel.overviewPanel);
+
+    /*
+     * exercise the pop-up menu in the Overview Panel (JAL-2864)
+     */
+    Object[] args = new Object[] {
+        new MouseEvent(af, 0, 0, 0, 0, 0, 1, true) };
+    PA.invokeMethod(af.alignPanel.overviewPanel,
+            "showPopupMenu(java.awt.event.MouseEvent)", args);
+
+    /*
      * set a selection group - potential memory leak if it retains
      * a reference to the alignment
      */
diff --git a/test/jalview/gui/ScalePanelTest.java b/test/jalview/gui/ScalePanelTest.java
new file mode 100644 (file)
index 0000000..91b541c
--- /dev/null
@@ -0,0 +1,58 @@
+package jalview.gui;
+
+import static org.testng.Assert.assertTrue;
+
+import jalview.datamodel.Alignment;
+import jalview.datamodel.AlignmentI;
+import jalview.datamodel.Sequence;
+import jalview.datamodel.SequenceGroup;
+import jalview.datamodel.SequenceI;
+
+import java.awt.event.MouseEvent;
+
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class ScalePanelTest
+{
+  @BeforeClass(alwaysRun = true)
+  public void setUpJvOptionPane()
+  {
+    JvOptionPane.setInteractiveMode(false);
+    JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION);
+  }
+
+  @Test(groups = "Functional")
+  public void testPreventNegativeStartColumn()
+  {
+    SequenceI seq1 = new Sequence("Seq1", "MATRESS");
+    SequenceI seq2 = new Sequence("Seq2", "MADNESS");
+    AlignmentI al = new Alignment(new SequenceI[] { seq1, seq2 });
+    
+    AlignFrame alignFrame = new AlignFrame(al, al.getWidth(),
+            al.getHeight());
+    ScalePanel scalePanel = new ScalePanel(
+            alignFrame.getViewport(), alignFrame.alignPanel
+    );
+    
+    MouseEvent mouse = new MouseEvent(
+            scalePanel, 0, 1, 0, 4, 0, 1, false
+    );
+    scalePanel.mousePressed(mouse);
+    scalePanel.mouseDragged(mouse);
+
+    // simulate dragging selection leftwards beyond the sequences giving
+    // negative X
+    mouse = new MouseEvent(scalePanel, 0, 1, 0, -30, 0, 1, false);
+
+    scalePanel.mouseReleased(mouse);
+
+    SequenceGroup sg = scalePanel.av.getSelectionGroup();
+    int startCol = sg.getStartRes();
+
+    assertTrue(startCol >= 0);
+
+
+  }
+
+}
index dd4f6ba..5be7968 100644 (file)
@@ -94,7 +94,7 @@ public class IdentifyFileTest
         { "examples/plantfdx.fa", FileFormat.Fasta },
         { "examples/dna_interleaved.phy", FileFormat.Phylip },
         { "examples/2GIS.pdb", FileFormat.PDB },
-        { "examples/rf00031_folded.stk", FileFormat.Stockholm },
+        { "examples/RF00031_folded.stk", FileFormat.Stockholm },
         { "examples/testdata/test.rnaml", FileFormat.Rnaml },
         { "examples/testdata/test.aln", FileFormat.Clustal },
         { "examples/testdata/test.pfam", FileFormat.Pfam },
index dfd7973..010a4b2 100644 (file)
@@ -55,10 +55,9 @@ public class JvCacheableInputBoxTest
     cacheBox.updateCache();
     try
     {
-      // This 1ms delay is essential to prevent the
-      // assertion below from executing before
-      // cacheBox.updateCache() finishes updating the cache
-      Thread.sleep(100);
+      // This delay is to let
+      // cacheBox.updateCache() finish updating the cache
+      Thread.sleep(200);
     } catch (InterruptedException e)
     {
       e.printStackTrace();
index 2eb718b..a96caec 100644 (file)
@@ -523,11 +523,60 @@ public class FeatureColourTest
     assertFalse(fc.hasThreshold());
     assertEquals(Color.RED, fc.getMinColour());
     assertEquals(Color.GREEN, fc.getMaxColour());
+    assertEquals(Color.RED, fc.getNoColour());
     assertEquals(10f, fc.getMin());
     assertEquals(20f, fc.getMax());
     assertTrue(fc.isAutoScaled());
 
     /*
+     * the same, with 'no value colour' specified as max
+     */
+    fc = FeatureColour
+            .parseJalviewFeatureColour("red|green|novaluemax|10.0|20.0");
+    assertEquals(Color.RED, fc.getMinColour());
+    assertEquals(Color.GREEN, fc.getMaxColour());
+    assertEquals(Color.GREEN, fc.getNoColour());
+    assertEquals(10f, fc.getMin());
+    assertEquals(20f, fc.getMax());
+
+    /*
+     * the same, with 'no value colour' specified as min
+     */
+    fc = FeatureColour
+            .parseJalviewFeatureColour("red|green|novalueMin|10.0|20.0");
+    assertEquals(Color.RED, fc.getMinColour());
+    assertEquals(Color.GREEN, fc.getMaxColour());
+    assertEquals(Color.RED, fc.getNoColour());
+    assertEquals(10f, fc.getMin());
+    assertEquals(20f, fc.getMax());
+
+    /*
+     * the same, with 'no value colour' specified as none
+     */
+    fc = FeatureColour
+            .parseJalviewFeatureColour("red|green|novaluenone|10.0|20.0");
+    assertEquals(Color.RED, fc.getMinColour());
+    assertEquals(Color.GREEN, fc.getMaxColour());
+    assertNull(fc.getNoColour());
+    assertEquals(10f, fc.getMin());
+    assertEquals(20f, fc.getMax());
+
+    /*
+     * the same, with invalid 'no value colour'
+     */
+    try
+    {
+      fc = FeatureColour
+              .parseJalviewFeatureColour("red|green|blue|10.0|20.0");
+      fail("expected exception");
+    } catch (IllegalArgumentException e)
+    {
+      assertEquals(
+              "Couldn't parse the minimum value for graduated colour ('blue')",
+              e.getMessage());
+    }
+
+    /*
      * graduated colour (explicitly by 'score') (no threshold)
      */
     fc = FeatureColour
index 26c3574..885cb71 100644 (file)
@@ -68,7 +68,7 @@ public class ViewStyleTest
     for (Field field : fields)
     {
       field.setAccessible(true);
-      if (!copyConstructorIgnores(field.getName()))
+      if (!copyConstructorIgnores(field))
       {
         changeValue(vs1, field);
       }
@@ -78,37 +78,49 @@ public class ViewStyleTest
 
     for (Field field1 : fields)
     {
-      final Object value1 = field1.get(vs1);
-      final Object value2 = field1.get(vs2);
-      String msg = "Mismatch in " + field1.getName() + "(" + value1 + "/"
+      if (!copyConstructorIgnores(field1))
+      {
+        final Object value1 = field1.get(vs1);
+        final Object value2 = field1.get(vs2);
+        String msg = "Mismatch in " + field1.getName() + "(" + value1 + "/"
               + value2 + ") - not set in copy constructor?";
-      assertEquals(msg, value1, value2);
+        assertEquals(msg, value1, value2);
+      }
     }
     assertEquals("Hashcode not equals", vs1.hashCode(), vs2.hashCode());
   }
 
   /**
-   * Add any field names in here that we expect to be ignored by the copy
-   * constructor
+   * Add tests here for any fields that we expect to be ignored by 
+   * the copy constructor
    * 
-   * @param name
+   * @param field
    * @return
    */
-  private boolean copyConstructorIgnores(String name)
+  private boolean copyConstructorIgnores(Field field)
   {
     /*
-     * currently none!
+     * ignore instrumentation added by jacoco for test coverage
      */
+    if (field.isSynthetic())
+    {
+      return true;
+    }
+    if (field.getType().toString().contains("com_atlassian_clover"))
+    {
+      return true;
+    }
+
     return false;
   }
-
+  
   /**
-   * Change the value of one field in a ViewStyle object
-   * 
-   * @param vs
-   * @param field
-   * @throws IllegalAccessException
-   */
+  * Change the value of one field in a ViewStyle object
+  * 
+  * @param vs
+  * @param field
+  * @throws IllegalAccessException
+  */
   protected void changeValue(ViewStyle vs, Field field)
           throws IllegalAccessException
   {
@@ -210,19 +222,22 @@ public class ViewStyleTest
     Field[] fields = ViewStyle.class.getDeclaredFields();
     for (Field field : fields)
     {
-      field.setAccessible(true);
-      Object oldValue = field.get(vs2);
-      changeValue(vs2, field);
-      assertFalse("equals method ignores " + field.getName(),
+      if (!copyConstructorIgnores(field))
+      {
+        field.setAccessible(true);
+        Object oldValue = field.get(vs2);
+        changeValue(vs2, field);
+        assertFalse("equals method ignores " + field.getName(),
               vs1.equals(vs2));
 
-      if (vs1.hashCode() == vs2.hashCode())
-      {
-        // uncomment next line to see which fields hashCode ignores
-        // System.out.println("hashCode ignores " + field.getName());
+        if (vs1.hashCode() == vs2.hashCode())
+        {
+          // uncomment next line to see which fields hashCode ignores
+          // System.out.println("hashCode ignores " + field.getName());
+        }
+        // restore original value before testing the next field
+        field.set(vs2, oldValue);
       }
-      // restore original value before testing the next field
-      field.set(vs2, oldValue);
     }
   }
 }
index e38064e..54f4e48 100644 (file)
@@ -1,6 +1,8 @@
 Checkstyle for Jalview
 ----------------------
 
+See
+https://issues.jalview.org/browse/JAL-1854
 http://checkstyle.sourceforge.net/
 GNU LGPL
 
@@ -9,8 +11,16 @@ To get the Eclipse Checkstyle plugin
        - Help | Eclipse Marketplace
        - search for checkstyle
        - install eclipse-cs checkstyle plugin
-The current version is 6.19.1 (August 2016).
 
+Change Log
+----------
+See http://checkstyle.sourceforge.net/releasenotes.html
+Aug 2016       Initial version used is 6.19.1
+Dec 2018       Updated to 8.12.0 (latest on Eclipse Marketplace, 8.15 is latest release)
+                       SuppressionCommentFilter relocated (changed in 8.1)
+                       FileContentsHolder removed (changed in 8.2)
+                       Updates to import-control.xml for code changes (htsjdk, stackoverflowusers)
+                       
 Config
 ------
 
@@ -37,6 +47,7 @@ How to use checkstyle
        Option 2: on demand on selected code
                - right-click on a class or package and Checkstyle | Check code with checkstyle
                - (or Clear Checkstyle violations to remove checkstyle warnings)
+               - recommended to use this as a QA step when changing or reviewing code
 
 Checkstyle rules
 ----------------
@@ -64,11 +75,11 @@ Suppressing findings
 Tips
 ----
        Sometimes checkstyle needs a kick before it will refresh its findings.
-       A whitespace edit in checkstyle.xml usually does this. There may be better ways.
+       Click the 'refresh' icon at top right in Eclipse | Preferences | Checkstyle.
        
        Invalid configuration files may result in checkstyle failing with an error reported
        in the Eclipse log file. 
-       Help | Installation Details | Configuration takes you to a screen with a 
+       Eclipse | About | Installation Details | Configuration takes you to a screen with a 
        'View Error Log' button.
        
        Sometimes checkstyle can fail silently. Try 'touching' (editing) config files, failing
index 122b8d0..29a3047 100644 (file)
@@ -26,6 +26,7 @@
     <suppress checks="[a-zA-Z0-9]*" files="[\\/]ext[\\/]edu*"/>
     <suppress checks="[a-zA-Z0-9]*" files="[\\/]ext[\\/]vamsas*"/>
     <suppress checks="[a-zA-Z0-9]*" files="[\\/]org[\\/]jibble*"/>
+    <suppress checks="[a-zA-Z0-9]*" files="[\\/]org[\\/]stackoverflowusers*"/>
     <suppress checks="[a-zA-Z0-9]*" files="[\\/]uk[\\/]ac*"/>
     
     <!-- 
index 85ac8e6..17946f7 100644 (file)
        </module>
 
        <!-- 
-               Allow suppression of rules by comments, e.g.:
-               // CHECKSTYLE.OFF: ParameterNumber
-               ..method declaration
-               // CHECKSTYLE.ON: ParameterNumber
-       -->
-       <module name="SuppressionCommentFilter">
-               <property name="offCommentFormat" value="CHECKSTYLE.OFF\: ([\w\|]+)"/>
-               <property name="onCommentFormat" value="CHECKSTYLE.ON\: ([\w\|]+)"/>
-               <property name="checkFormat" value="$1"/>
-       </module>
-
-       <!-- 
                Check language bundles have the same keys and no duplicates
                (ensure Checkstyle is configured to scan non-source files)
         -->
                <property name="tabWidth" value="4"/>
 
                <!-- 
-                       Enables parsing of suppressions comments
-                       see http://checkstyle.sourceforge.net/config_filters.html#SuppressionCommentFilter 
+                       Allow suppression of rules by comments, e.g.:
+                       // CHECKSTYLE.OFF: ParameterNumber
+                       ..method declaration
+                       // CHECKSTYLE.ON: ParameterNumber
                -->
-               <module name="FileContentsHolder"/>
+               <module name="SuppressionCommentFilter">
+                       <property name="offCommentFormat" value="CHECKSTYLE.OFF\: ([\w\|]+)"/>
+                       <property name="onCommentFormat" value="CHECKSTYLE.ON\: ([\w\|]+)"/>
+                       <property name="checkFormat" value="$1"/>
+               </module>
 
        <!-- ****************************** -->
        <!--         NAMING STANDARDS       -->
index c47aaec..478966f 100644 (file)
@@ -94,6 +94,7 @@
                <allow pkg="compbio.metadata" class="jalview.gui.WsJobParameters"/>
                <allow pkg="fr.orsay.lri.varna" class="jalview.gui.AppVarna"/>
                <allow pkg="fr.orsay.lri.varna" class="jalview.gui.AppVarnaBinding"/>
+               <allow pkg="org.stackoverflowusers.file" class="jalview.gui.Desktop"/>
                <allow pkg="uk.ac.vamsas" class="jalview.gui.VamsasApplication"/>
            </subpackage>
                
                <allow pkg="uk.ac.vamsas"/>
                <allow pkg="fr.orsay.lri.varna"/>
                <allow pkg="MCview"/>
+                       <subpackage name="vcf">
+                       <allow pkg="htsjdk\.*" regex="true"/>
+                       </subpackage>       
                </subpackage>       
                                
                <subpackage name="javascript">