JAL-2541 it works!! it works!!!

diff --git a/src/jalview/commands/EditCommand.java b/src/jalview/commands/EditCommand.java
index 6484224..9be8d3f 100644 (file)
--- a/src/jalview/commands/EditCommand.java
+++ b/src/jalview/commands/EditCommand.java
@@ -753,8 +753,7 @@ public class EditCommand implements CommandI
       tmp.append(oldstring.substring(end));
       command.seqs[i].setSequence(tmp.toString());
       command.string[i] = oldstring.substring(start, end).toCharArray();
-      String nogapold = jalview.analysis.AlignSeq.extractGaps(
-              jalview.util.Comparison.GapChars,
+      String nogapold = AlignSeq.extractGaps(Comparison.GapChars,
               new String(command.string[i]));
       if (!nogaprep.toLowerCase().equals(nogapold.toLowerCase()))
       {
@@ -903,8 +902,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)
                   {
@@ -1461,6 +1459,10 @@ public class EditCommand implements CommandI
   protected static void cutFeatures(Edit command, SequenceI seq,
           int fromPosition, int toPosition, boolean cutIsInternal)
   {
+    if (!cutIsInternal)
+    {
+      return;
+    }
     List<SequenceFeature> added = new ArrayList<>();
     List<SequenceFeature> removed = new ArrayList<>();
   

diff --git a/test/jalview/commands/EditCommandTest.java b/test/jalview/commands/EditCommandTest.java
index a415338..111adba 100644 (file)
--- a/test/jalview/commands/EditCommandTest.java
+++ b/test/jalview/commands/EditCommandTest.java
@@ -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()
   {
@@ -762,48 +774,88 @@ public class EditCommandTest
     {
       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);
 
-        sfs = seq0.getSequenceFeatures();
+        verifyCut(seq0, from, to, msg);
 
         /*
-         * confirm the number of features has reduced by the
-         * number of features within the cut region i.e. by
-         * func(length of cut)
+         * undo and verify
          */
-        String msg = String.format("Cut %d-%d ", from + 1, to + 1);
-        if (to - from == 4)
-        {
-          // all columns were cut
-          assertTrue(sfs.isEmpty());
-        }
-        else
-        {
-          assertEquals(msg + "wrong number of features left", func(5)
-                  - func(to - from + 1), sfs.size());
-        }
+        AlignmentI[] views = new AlignmentI[] { alignment };
+        ec.undoCommand(views);
+        sfs = seq0.getSequenceFeatures();
+        assertEquals("After undo of " + msg, func(5), sfs.size());
+        verifyUndo(from, to, sfs);
 
         /*
-         * inspect individual features
+         * redo and verify
          */
-        for (SequenceFeature sf : sfs)
-        {
-          checkFeatureRelocation(sf, from + 1, to + 1, from > 0);
-        }
+        ec.doCommand(views);
+        verifyCut(seq0, from, to, msg);
 
         /*
          * undo ready for next cut
          */
-        testee.undoCommand(new AlignmentI[] { alignment });
-        sfs = seq0.getSequenceFeatures();
-        assertEquals("After undo of " + msg, func(5), sfs.size());
-        verifyUndo(from, to, sfs);
+        ec.undoCommand(views);
       }
     }
   }
 
   /**
+   * 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
+   */
+  protected void verifyCut(SequenceI seq0, int from, int to,
+          final String msg)
+  {
+    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);
+    }
+  }
+
+  /**
    * Check that after Undo, every feature has start/end that match its original
    * "start" and "end" properties
    * 
@@ -835,7 +887,7 @@ public class EditCommandTest
    *          end of cut (last residue cut)
    * @param newDataset
    */
-  private void checkFeatureRelocation(SequenceFeature sf, int from, int to,
+  private void verifyFeatureRelocation(SequenceFeature sf, int from, int to,
           boolean newDataset)
   {
     // TODO handle the gapped sequence case as well
@@ -846,7 +898,13 @@ public class EditCommandTest
     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 (oldTo < from)
     {
       // before cut region so unchanged
       assertEquals("1: " + msg, oldFrom, sf.getBegin());
@@ -916,7 +974,5 @@ public class EditCommandTest
     SequenceFeature sf = sfs.get(0);
     assertEquals(10, sf.getBegin());
     assertEquals(11, sf.getEnd());
-
-    // TODO add further cases including Undo - see JAL-2541
   }
 }