From: kiramt Date: Wed, 7 Jun 2017 17:04:43 +0000 (+0100) Subject: JAL-2591 Further refactoring (still incomplete) X-Git-Tag: Release_2_10_3b1~183^2~9 X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=fd8dc2879b13709703b57dd7ed4b3754b253e12f JAL-2591 Further refactoring (still incomplete) --- diff --git a/src/jalview/appletgui/AlignFrame.java b/src/jalview/appletgui/AlignFrame.java index a75adbb..e77ae54 100644 --- a/src/jalview/appletgui/AlignFrame.java +++ b/src/jalview/appletgui/AlignFrame.java @@ -1900,7 +1900,7 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener, static StringBuffer copiedSequences; - static Vector copiedHiddenColumns; + static Vector copiedHiddenColumns; protected void copy_actionPerformed() { @@ -1924,12 +1924,13 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener, if (viewport.hasHiddenColumns() && viewport.getSelectionGroup() != null) { - copiedHiddenColumns = new Vector(); + copiedHiddenColumns = viewport.getAlignment().getHiddenColumns() + .getHiddenColumnsCopy(); int hiddenOffset = viewport.getSelectionGroup().getStartRes(); - for (int[] region : viewport.getAlignment().getHiddenColumns()) + for (int[] region : copiedHiddenColumns) { - copiedHiddenColumns.addElement(new int[] { - region[0] - hiddenOffset, region[1] - hiddenOffset }); + region[0] = region[0] - hiddenOffset; + region[1] = region[1] - hiddenOffset; } } else @@ -2044,7 +2045,7 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener, { for (int i = 0; i < copiedHiddenColumns.size(); i++) { - int[] region = (int[]) copiedHiddenColumns.elementAt(i); + int[] region = copiedHiddenColumns.elementAt(i); af.viewport.hideColumns(region[0], region[1]); } } diff --git a/src/jalview/appletgui/AnnotationColumnChooser.java b/src/jalview/appletgui/AnnotationColumnChooser.java index c49a5f3..bb67efc 100644 --- a/src/jalview/appletgui/AnnotationColumnChooser.java +++ b/src/jalview/appletgui/AnnotationColumnChooser.java @@ -46,7 +46,7 @@ import java.awt.event.MouseEvent; import java.awt.event.MouseListener; import java.awt.event.TextEvent; import java.awt.event.TextListener; -import java.util.Iterator; +import java.util.ArrayList; import java.util.Vector; //import javax.swing.JPanel; @@ -300,12 +300,14 @@ public class AnnotationColumnChooser extends AnnotationRowFilter implements .getOldHiddenColumns(); if (oldHidden != null) { - for (Iterator itr = oldHidden.iterator(); itr.hasNext();) + ArrayList regions = oldHidden.getHiddenColumnsCopyAsList(); + for (int[] positions : regions) { - int positions[] = itr.next(); av.hideColumns(positions[0], positions[1]); } } + // TODO not clear why we need to hide all the columns (above) if we are + // going to copy the hidden columns over wholesale anyway av.getAlignment().setHiddenColumns(oldHidden); } ap.paintAlignment(true); diff --git a/src/jalview/appletgui/ScalePanel.java b/src/jalview/appletgui/ScalePanel.java index 5bb0f75..2abd65a 100755 --- a/src/jalview/appletgui/ScalePanel.java +++ b/src/jalview/appletgui/ScalePanel.java @@ -334,17 +334,8 @@ public class ScalePanel extends Panel implements MouseMotionListener, int res = (evt.getX() / av.getCharWidth()) + av.getRanges().getStartRes(); - res = av.getAlignment().getHiddenColumns().adjustForHiddenColumns(res); - - reveal = null; - for (int[] region : av.getAlignment().getHiddenColumns()) - { - if (res + 1 == region[0] || res - 1 == region[1]) - { - reveal = region; - break; - } - } + reveal = av.getAlignment().getHiddenColumns() + .getRegionWithEdgeAtRes(res); repaint(); } diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index be10721..5966312 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -67,13 +67,16 @@ public class HiddenColumns { LOCK.readLock().lock(); StringBuilder regionBuilder = new StringBuilder(); - for (int[] range : hiddenColumns) + if (hiddenColumns != null) { - regionBuilder.append(delimiter).append(range[0]).append(between) - .append(range[1]); - } + for (int[] range : hiddenColumns) + { + regionBuilder.append(delimiter).append(range[0]).append(between) + .append(range[1]); + } - regionBuilder.deleteCharAt(0); + regionBuilder.deleteCharAt(0); + } return regionBuilder.toString(); } finally { @@ -331,29 +334,37 @@ public class HiddenColumns try { LOCK.readLock().lock(); - List positions = new ArrayList<>( - hiddenColumns.size()); + List positions = null; - positions.add(hiddenColumns.elementAt(0)[0]); - for (int i = 1; i < hiddenColumns.size(); ++i) + if (hiddenColumns != null) { + positions = new ArrayList<>(hiddenColumns.size()); - int result = 0; - if (hiddenColumns != null) + positions.add(hiddenColumns.elementAt(0)[0]); + for (int i = 1; i < hiddenColumns.size(); ++i) { - int index = 0; - int gaps = 0; - do + + int result = 0; + if (hiddenColumns != null) { - int[] region = hiddenColumns.elementAt(index); - gaps += region[1] + 1 - region[0]; - result = region[1] + 1; - index++; - } while (index <= i); + int index = 0; + int gaps = 0; + do + { + int[] region = hiddenColumns.elementAt(index); + gaps += region[1] + 1 - region[0]; + result = region[1] + 1; + index++; + } while (index <= i); - result -= gaps; + result -= gaps; + } + positions.add(result); } - positions.add(result); + } + else + { + positions = new ArrayList<>(); } return positions; @@ -665,6 +676,13 @@ public class HiddenColumns return copy; } + /** + * Returns a copy of the vector of hidden regions, as a vector. Before using + * this method please consider if you really need access to the hidden regions + * - a new (or existing!) method on HiddenColumns might be more appropriate. + * + * @return hidden regions as vector of [start,end] pairs + */ public Vector getHiddenColumnsCopy() { try @@ -677,6 +695,14 @@ public class HiddenColumns } } + /** + * Returns a copy of the vector of hidden regions, as an ArrayList. Before + * using this method please consider if you really need access to the hidden + * regions - a new (or existing!) method on HiddenColumns might be more + * appropriate. + * + * @return hidden regions as an ArrayList of [start,end] pairs + */ public ArrayList getHiddenColumnsCopyAsList() { try @@ -1711,4 +1737,37 @@ public class HiddenColumns } + /** + * Finds the hidden region (if any) which starts or ends at res + * + * @param res + * visible residue position, unadjusted for hidden columns + * @return region as [start,end] or null if no matching region is found + */ + public int[] getRegionWithEdgeAtRes(int res) + { + try + { + LOCK.readLock().lock(); + int adjres = adjustForHiddenColumns(res); + + int[] reveal = null; + if (hiddenColumns != null) + { + for (int[] region : hiddenColumns) + { + if (adjres + 1 == region[0] || adjres - 1 == region[1]) + { + reveal = region; + break; + } + } + } + return reveal; + } finally + { + LOCK.readLock().unlock(); + } + } + } diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java index ae2e44a..2c7837f 100644 --- a/src/jalview/gui/AlignFrame.java +++ b/src/jalview/gui/AlignFrame.java @@ -1885,7 +1885,9 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, hiddenColumns = new ArrayList<>(); int hiddenOffset = viewport.getSelectionGroup().getStartRes(); int hiddenCutoff = viewport.getSelectionGroup().getEndRes(); - for (int[] region : viewport.getAlignment().getHiddenColumns()) + ArrayList hiddenRegions = viewport.getAlignment() + .getHiddenColumns().getHiddenColumnsCopyAsList(); + for (int[] region : hiddenRegions) { if (region[0] >= hiddenOffset && region[1] <= hiddenCutoff) { diff --git a/src/jalview/gui/AnnotationColumnChooser.java b/src/jalview/gui/AnnotationColumnChooser.java index 405b43b..311b9d5 100644 --- a/src/jalview/gui/AnnotationColumnChooser.java +++ b/src/jalview/gui/AnnotationColumnChooser.java @@ -36,7 +36,7 @@ import java.awt.event.ActionListener; import java.awt.event.ItemEvent; import java.awt.event.ItemListener; import java.awt.event.KeyEvent; -import java.util.Iterator; +import java.util.ArrayList; import javax.swing.ButtonGroup; import javax.swing.JCheckBox; @@ -246,12 +246,14 @@ public class AnnotationColumnChooser extends AnnotationRowFilter implements .getOldHiddenColumns(); if (oldHidden != null) { - for (Iterator itr = oldHidden.iterator(); itr.hasNext();) + ArrayList regions = oldHidden.getHiddenColumnsCopyAsList(); + for (int[] positions : regions) { - int positions[] = itr.next(); av.hideColumns(positions[0], positions[1]); } } + // TODO not clear why we need to hide all the columns (above) if we are + // going to copy the hidden columns over wholesale anyway av.getAlignment().setHiddenColumns(oldHidden); } ap.paintAlignment(true); diff --git a/src/jalview/gui/ScalePanel.java b/src/jalview/gui/ScalePanel.java index dcaafcd..3cdba8d 100755 --- a/src/jalview/gui/ScalePanel.java +++ b/src/jalview/gui/ScalePanel.java @@ -401,20 +401,15 @@ public class ScalePanel extends JPanel implements MouseMotionListener, int res = (evt.getX() / av.getCharWidth()) + av.getRanges().getStartRes(); + reveal = av.getAlignment().getHiddenColumns() + .getRegionWithEdgeAtRes(res); + res = av.getAlignment().getHiddenColumns().adjustForHiddenColumns(res); - for (int[] region : av.getAlignment().getHiddenColumns()) - { - if (res + 1 == region[0] || res - 1 == region[1]) - { - reveal = region; - ToolTipManager.sharedInstance().registerComponent(this); - this.setToolTipText( - MessageManager.getString("label.reveal_hidden_columns")); - repaint(); - return; - } - } + ToolTipManager.sharedInstance().registerComponent(this); + this.setToolTipText( + MessageManager.getString("label.reveal_hidden_columns")); + repaint(); } /** diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index 5b92677..62c2d22 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -20,6 +20,7 @@ */ package jalview.datamodel; +import static org.testng.Assert.assertNull; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertFalse; import static org.testng.AssertJUnit.assertSame; @@ -590,11 +591,15 @@ public class HiddenColumnsTest public void testFindHiddenRegionPositions() { HiddenColumns hc = new HiddenColumns(); + + List positions = hc.findHiddenRegionPositions(); + assertTrue(positions.isEmpty()); + hc.hideColumns(3, 7); hc.hideColumns(10, 10); hc.hideColumns(14, 15); - List positions = hc.findHiddenRegionPositions(); + positions = hc.findHiddenRegionPositions(); assertEquals(3, positions.size()); assertEquals(3, positions.get(0).intValue()); assertEquals(5, positions.get(1).intValue()); @@ -605,11 +610,15 @@ public class HiddenColumnsTest public void testRegionsToString() { HiddenColumns hc = new HiddenColumns(); + + String result = hc.regionsToString(",", "--"); + assertEquals("", result); + hc.hideColumns(3, 7); hc.hideColumns(10, 10); hc.hideColumns(14, 15); - String result = hc.regionsToString(",", "--"); + result = hc.regionsToString(",", "--"); assertEquals("3--7,10--10,14--15", result); } @@ -642,4 +651,28 @@ public class HiddenColumnsTest assertEquals(23, startEnd[1]); } + @Test(groups = "Functional") + public void testGetRegionWithEdgeAtRes() + { + HiddenColumns hc = new HiddenColumns(); + + int[] result = hc.getRegionWithEdgeAtRes(5); + assertNull(result); + + hc.hideColumns(3, 7); + hc.hideColumns(10, 10); + hc.hideColumns(14, 15); + + result = hc.getRegionWithEdgeAtRes(3); + assertEquals(3, result[0]); + assertEquals(7, result[1]); + + result = hc.getRegionWithEdgeAtRes(5); + assertEquals(10, result[0]); + assertEquals(10, result[1]); + + result = hc.getRegionWithEdgeAtRes(6); + assertNull(result); + } + }