From d85a2741994c169e1b81db8f9166f5214ff1f561 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Fri, 15 May 2020 08:43:26 +0100 Subject: [PATCH] JAL-3551 pull up Close Viewer dialog to base class --- resources/lang/Messages.properties | 4 +- resources/lang/Messages_es.properties | 4 +- src/jalview/appletgui/AppletJmol.java | 40 ++++++++--------- src/jalview/ext/jmol/JalviewJmolBinding.java | 10 ----- .../ext/rbvi/chimera/JalviewChimeraBinding.java | 39 ++-------------- src/jalview/gui/AppJmol.java | 17 ------- src/jalview/gui/ChimeraViewFrame.java | 47 +------------------- src/jalview/gui/PymolBindingModel.java | 7 +-- src/jalview/gui/PymolViewer.java | 41 +---------------- src/jalview/gui/StructureViewerBase.java | 46 +++++++++++++++++++ .../structures/models/AAStructureBindingModel.java | 28 ++++++++++++ .../ext/rbvi/chimera/JalviewChimeraView.java | 26 +++++------ 12 files changed, 123 insertions(+), 186 deletions(-) diff --git a/resources/lang/Messages.properties b/resources/lang/Messages.properties index f973ec4..8747588 100644 --- a/resources/lang/Messages.properties +++ b/resources/lang/Messages.properties @@ -272,7 +272,7 @@ label.viewer_path = Path to {0} program label.viewer_path_tip = Jalview will first try any path entered here, else standard installation locations.
Double-click to browse for file. label.invalid_viewer_path = Path not found or not executable label.viewer_missing = Structure viewer not found.
Please enter the path to the executable (if installed),
or download and install the program. -label.chimera_failed = Error opening Chimera - is it installed?\nCheck path in Preferences, Structure +label.open_viewer_failed = Error opening {0} - is it installed?\nCheck path in Preferences, Structure label.min_colour = Minimum Colour label.max_colour = Maximum Colour label.no_colour = No Colour @@ -504,7 +504,7 @@ label.sequence_details = Sequence Details label.jmol_help = Jmol Help label.chimera_help = Chimera Help label.close_viewer = Close Viewer -label.confirm_close_chimera = This will close Jalview''s connection to {0}.
Do you want to close the Chimera window as well? +label.confirm_close_viewer = This will close Jalview''s connection to {0}.
Do you want to close the {1} window as well? label.all = All label.sort_by = Sort alignment by label.sort_by_score = Sort by Score diff --git a/resources/lang/Messages_es.properties b/resources/lang/Messages_es.properties index 2208756..58f0223 100644 --- a/resources/lang/Messages_es.properties +++ b/resources/lang/Messages_es.properties @@ -1145,7 +1145,7 @@ label.invalid_search=Texto de b action.export_annotations=Exportar Anotaciones action.set_as_reference=Marcar como Referencia action.unmark_as_reference=Desmarcar como Referencia -label.chimera_failed=Error al abrir Chimera - está instalado?\nCompruebe ruta en Preferencias, Estructura +label.open_viewer_failed=Error al abrir {0} - está instalado?\nCompruebe ruta en Preferencias, Estructura label.find=Buscar label.select_pdb_file=Seleccionar Fichero PDB label.structures_filter=Filtro de Estructuras @@ -1192,7 +1192,7 @@ label.protein=Prote warn.oneseq_msainput_selection=La selección actual sólo contiene una única secuencia. ¿Quieres enviar todas las secuencias para la alineación en su lugar? label.use_rnaview=Usar RNAView para estructura secondaria label.search_all=Introducir uno o más valores de búsqueda separados por punto y coma ";" (Nota: buscará en toda la base de datos PDB) -label.confirm_close_chimera=Cerrará la conexión de Jalview a {0}.
¿Quieres cerrar la ventana Chimera también? +label.confirm_close_viewer=Cerrará la conexión de Jalview a {0}.
¿Quieres cerrar la ventana {1} también? tooltip.rnalifold_calculations=Se calcularán predicciones de estructura secondaria de RNA para el alineaminento, y se actualizarán si se efectuan cambios tooltip.rnalifold_settings=Modificar la configuración de la predicción RNAAlifold. Úselo para ocultar o mostrar resultados del cálculo de RNA, o cambiar parámetros de el plegado de RNA. label.show_selected_annotations=Mostrar anotaciones seleccionadas diff --git a/src/jalview/appletgui/AppletJmol.java b/src/jalview/appletgui/AppletJmol.java index 89a912f..6e95633 100644 --- a/src/jalview/appletgui/AppletJmol.java +++ b/src/jalview/appletgui/AppletJmol.java @@ -20,25 +20,6 @@ */ package jalview.appletgui; -import jalview.bin.JalviewLite; -import jalview.datamodel.AlignmentI; -import jalview.datamodel.PDBEntry; -import jalview.datamodel.SequenceI; -import jalview.io.DataSourceType; -import jalview.io.FileParse; -import jalview.io.StructureFile; -import jalview.schemes.BuriedColourScheme; -import jalview.schemes.HelixColourScheme; -import jalview.schemes.HydrophobicColourScheme; -import jalview.schemes.PurinePyrimidineColourScheme; -import jalview.schemes.StrandColourScheme; -import jalview.schemes.TaylorColourScheme; -import jalview.schemes.TurnColourScheme; -import jalview.schemes.UserColourScheme; -import jalview.schemes.ZappoColourScheme; -import jalview.structure.StructureSelectionManager; -import jalview.util.MessageManager; - import java.awt.BorderLayout; import java.awt.CheckboxMenuItem; import java.awt.Color; @@ -64,6 +45,25 @@ import java.util.ArrayList; import java.util.List; import java.util.Vector; +import jalview.bin.JalviewLite; +import jalview.datamodel.AlignmentI; +import jalview.datamodel.PDBEntry; +import jalview.datamodel.SequenceI; +import jalview.io.DataSourceType; +import jalview.io.FileParse; +import jalview.io.StructureFile; +import jalview.schemes.BuriedColourScheme; +import jalview.schemes.HelixColourScheme; +import jalview.schemes.HydrophobicColourScheme; +import jalview.schemes.PurinePyrimidineColourScheme; +import jalview.schemes.StrandColourScheme; +import jalview.schemes.TaylorColourScheme; +import jalview.schemes.TurnColourScheme; +import jalview.schemes.UserColourScheme; +import jalview.schemes.ZappoColourScheme; +import jalview.structure.StructureSelectionManager; +import jalview.util.MessageManager; + public class AppletJmol extends EmbmenuFrame implements // StructureListener, KeyListener, ActionListener, ItemListener @@ -411,7 +411,7 @@ public class AppletJmol extends EmbmenuFrame implements void closeViewer() { - jmb.closeViewer(); + jmb.closeViewer(true); jmb = null; this.setVisible(false); } diff --git a/src/jalview/ext/jmol/JalviewJmolBinding.java b/src/jalview/ext/jmol/JalviewJmolBinding.java index 08ea052..c3a22fb 100644 --- a/src/jalview/ext/jmol/JalviewJmolBinding.java +++ b/src/jalview/ext/jmol/JalviewJmolBinding.java @@ -111,16 +111,6 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel return getViewerTitle("Jmol", true); } - public void closeViewer() - { - // remove listeners for all structures in viewer - getSsm().removeStructureViewerListener(this, this.getStructureFiles()); - jmolViewer.dispose(); - lastCommand = null; - jmolViewer = null; - releaseUIResources(); - } - @Override public List executeCommand(StructureCommandI command, boolean getReply) diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java index 476a933..98cc1ff 100644 --- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java +++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java @@ -38,7 +38,6 @@ import ext.edu.ucsf.rbvi.strucviz2.StructureManager; import ext.edu.ucsf.rbvi.strucviz2.StructureManager.ModelType; import jalview.api.AlignmentViewPanel; import jalview.api.structures.JalviewStructureDisplayI; -import jalview.bin.Cache; import jalview.datamodel.AlignmentI; import jalview.datamodel.PDBEntry; import jalview.datamodel.SearchResultMatchI; @@ -245,9 +244,10 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel * Close down the Jalview viewer and listener, and (optionally) the associated * Chimera window. */ + @Override public void closeViewer(boolean closeChimera) { - getSsm().removeStructureViewerListener(this, this.getStructureFiles()); + super.closeViewer(closeChimera); if (closeChimera) { chimeraManager.exitChimera(); @@ -263,7 +263,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel { chimeraMonitor.interrupt(); } - releaseUIResources(); } /** @@ -339,7 +338,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel * * @return */ - public boolean isChimeraRunning() + @Override + public boolean isViewerRunning() { return chimeraManager.isChimeraLaunched(); } @@ -545,37 +545,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel } /** - * Ask Chimera to save its session to the given file. Returns true if - * successful, else false. - * - * @param filepath - * @return - */ - public boolean saveSession(String filepath) - { - if (isChimeraRunning()) - { - /* - * Chimera: https://www.cgl.ucsf.edu/chimera/current/docs/UsersGuide/midas/save.html - * ChimeraX: https://www.cgl.ucsf.edu/chimerax/docs/user/commands/save.html - */ - String command = getCommandGenerator().saveSession(filepath) - .getCommand(); - List reply = chimeraManager.sendChimeraCommand(command, true); - if (reply.contains("Session written")) - { - return true; - } - else - { - Cache.log - .error("Error saving Chimera session: " + reply.toString()); - } - } - return false; - } - - /** * Ask Chimera to open a session file. Returns true if successful, else false. * The filename must have a .py (Chimera) or .cxs (ChimeraX) extension for * this command to work. diff --git a/src/jalview/gui/AppJmol.java b/src/jalview/gui/AppJmol.java index 0dde255..0610239 100644 --- a/src/jalview/gui/AppJmol.java +++ b/src/jalview/gui/AppJmol.java @@ -255,23 +255,6 @@ public class AppJmol extends StructureViewerBase } @Override - public void closeViewer(boolean closeExternalViewer) - { - // Jmol does not use an external viewer - if (jmb != null) - { - jmb.closeViewer(); - } - setAlignmentPanel(null); - _aps.clear(); - _alignwith.clear(); - _colourwith.clear(); - // TODO: check for memory leaks where instance isn't finalised because jmb - // holds a reference to the window - jmb = null; - } - - @Override public void run() { _started = true; diff --git a/src/jalview/gui/ChimeraViewFrame.java b/src/jalview/gui/ChimeraViewFrame.java index 24ac87a..8fcea0c 100644 --- a/src/jalview/gui/ChimeraViewFrame.java +++ b/src/jalview/gui/ChimeraViewFrame.java @@ -314,7 +314,8 @@ public class ChimeraViewFrame extends StructureViewerBase if (!jmb.launchChimera()) { JvOptionPane.showMessageDialog(Desktop.desktop, - MessageManager.getString("label.chimera_failed"), + MessageManager.formatMessage("label.open_viewer_failed", + getViewerName()), MessageManager.getString("label.error_loading_file"), JvOptionPane.ERROR_MESSAGE); this.dispose(); @@ -335,50 +336,6 @@ public class ChimeraViewFrame extends StructureViewerBase } /** - * Close down this instance of Jalview's Chimera viewer, giving the user the - * option to close the associated Chimera window (process). They may wish to - * keep it open until they have had an opportunity to save any work. - * - * @param closeChimera - * if true, close any linked Chimera process; if false, prompt first - */ - @Override - public void closeViewer(boolean closeChimera) - { - if (jmb != null && jmb.isChimeraRunning()) - { - if (!closeChimera) - { - String prompt = MessageManager - .formatMessage("label.confirm_close_chimera", new Object[] - { jmb.getViewerTitle(getViewerName(), false) }); - prompt = JvSwingUtils.wrapTooltip(true, prompt); - int confirm = JvOptionPane.showConfirmDialog(this, prompt, - MessageManager.getString("label.close_viewer"), - JvOptionPane.YES_NO_CANCEL_OPTION); - /* - * abort closure if user hits escape or Cancel - */ - if (confirm == JvOptionPane.CANCEL_OPTION - || confirm == JvOptionPane.CLOSED_OPTION) - { - return; - } - closeChimera = confirm == JvOptionPane.YES_OPTION; - } - jmb.closeViewer(closeChimera); - } - setAlignmentPanel(null); - _aps.clear(); - _alignwith.clear(); - _colourwith.clear(); - // TODO: check for memory leaks where instance isn't finalised because jmb - // holds a reference to the window - jmb = null; - dispose(); - } - - /** * Open any newly added PDB structures in Chimera, having first fetched data * from PDB (if not already saved). */ diff --git a/src/jalview/gui/PymolBindingModel.java b/src/jalview/gui/PymolBindingModel.java index fcf586a..fc957bb 100644 --- a/src/jalview/gui/PymolBindingModel.java +++ b/src/jalview/gui/PymolBindingModel.java @@ -129,14 +129,16 @@ public class PymolBindingModel extends AAStructureBindingModel return ViewerType.PYMOL; } - public boolean isPymolRunning() + @Override + public boolean isViewerRunning() { return pymolManager.isPymolLaunched(); } + @Override public void closeViewer(boolean closePymol) { - getSsm().removeStructureViewerListener(this, this.getStructureFiles()); + super.closeViewer(closePymol); if (closePymol) { pymolManager.exitPymol(); @@ -147,7 +149,6 @@ public class PymolBindingModel extends AAStructureBindingModel { pymolMonitor.interrupt(); } - releaseUIResources(); } public boolean openSession(String pymolSessionFile) diff --git a/src/jalview/gui/PymolViewer.java b/src/jalview/gui/PymolViewer.java index d0c9ea2..98582b2 100644 --- a/src/jalview/gui/PymolViewer.java +++ b/src/jalview/gui/PymolViewer.java @@ -290,7 +290,8 @@ public class PymolViewer extends StructureViewerBase if (!binding.launchPymol()) { JvOptionPane.showMessageDialog(Desktop.desktop, - MessageManager.getString("label.pymol_failed"), + MessageManager.formatMessage("label.open_viewer_failed", + getViewerName()), MessageManager.getString("label.error_loading_file"), JvOptionPane.ERROR_MESSAGE); this.dispose(); @@ -316,44 +317,6 @@ public class PymolViewer extends StructureViewerBase } @Override - public void closeViewer(boolean closePymol) - { - if (binding != null && binding.isPymolRunning()) - { - if (!closePymol) - { - // TODO i18n (and pull up) - String prompt = MessageManager - .formatMessage("label.confirm_close_pymol", new Object[] - { binding.getViewerTitle(getViewerName(), false) }); - prompt = JvSwingUtils.wrapTooltip(true, prompt); - int confirm = JvOptionPane.showConfirmDialog(this, prompt, - MessageManager.getString("label.close_viewer"), - JvOptionPane.YES_NO_CANCEL_OPTION); - /* - * abort closure if user hits escape or Cancel - */ - if (confirm == JvOptionPane.CANCEL_OPTION - || confirm == JvOptionPane.CLOSED_OPTION) - { - return; - } - closePymol = confirm == JvOptionPane.YES_OPTION; - } - binding.closeViewer(closePymol); - } - setAlignmentPanel(null); - _aps.clear(); - _alignwith.clear(); - _colourwith.clear(); - // TODO: check for memory leaks where instance isn't finalised because - // binding - // holds a reference to the window - binding = null; - dispose(); - } - - @Override public ViewerType getViewerType() { return ViewerType.PYMOL; diff --git a/src/jalview/gui/StructureViewerBase.java b/src/jalview/gui/StructureViewerBase.java index 1bbc397..9b6fd61 100644 --- a/src/jalview/gui/StructureViewerBase.java +++ b/src/jalview/gui/StructureViewerBase.java @@ -1162,4 +1162,50 @@ public abstract class StructureViewerBase extends GStructureViewer return getBinding() == null ? null : getBinding().saveSession(); } + /** + * Close down this instance of Jalview's Chimera viewer, giving the user the + * option to close the associated Chimera window (process). They may wish to + * keep it open until they have had an opportunity to save any work. + * + * @param forceClose + * if true, close any linked Chimera process; if false, prompt first + */ + @Override + public void closeViewer(boolean forceClose) + { + AAStructureBindingModel binding = getBinding(); + if (binding != null && binding.isViewerRunning()) + { + if (!forceClose) + { + String viewerName = getViewerName(); + String prompt = MessageManager + .formatMessage("label.confirm_close_viewer", new Object[] + { binding.getViewerTitle(viewerName, false), viewerName }); + prompt = JvSwingUtils.wrapTooltip(true, prompt); + int confirm = JvOptionPane.showConfirmDialog(this, prompt, + MessageManager.getString("label.close_viewer"), + JvOptionPane.YES_NO_CANCEL_OPTION); + /* + * abort closure if user hits escape or Cancel + */ + if (confirm == JvOptionPane.CANCEL_OPTION + || confirm == JvOptionPane.CLOSED_OPTION) + { + return; + } + forceClose = confirm == JvOptionPane.YES_OPTION; + } + binding.closeViewer(forceClose); + } + setAlignmentPanel(null); + _aps.clear(); + _alignwith.clear(); + _colourwith.clear(); + // TODO: check for memory leaks where instance isn't finalised because jmb + // holds a reference to the window + // jmb = null; + dispose(); + } + } diff --git a/src/jalview/structures/models/AAStructureBindingModel.java b/src/jalview/structures/models/AAStructureBindingModel.java index 88c181c..6e926df 100644 --- a/src/jalview/structures/models/AAStructureBindingModel.java +++ b/src/jalview/structures/models/AAStructureBindingModel.java @@ -1566,4 +1566,32 @@ public abstract class AAStructureBindingModel executeCommand(cmd, false); } } + + /** + * Returns true if the viewer is an external structure viewer for which the + * process is still alive, else false (for Jmol, or an external viewer which + * the user has independently closed) + * + * @return + */ + public boolean isViewerRunning() + { + return false; + } + + /** + * Closes Jalview's structure viewer panel and releases associated resources. + * If it is managing an external viewer program, and {@code forceClose} is + * true, also shuts down that program. + * + * @param forceClose + */ + public void closeViewer(boolean forceClose) + { + getSsm().removeStructureViewerListener(this, this.getStructureFiles()); + releaseUIResources(); + + // add external viewer shutdown in overrides + // todo - or can maybe pull up to here + } } diff --git a/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java b/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java index 30c23af..d397a6b 100644 --- a/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java +++ b/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java @@ -25,6 +25,16 @@ import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; +import java.io.File; +import java.io.IOException; +import java.util.List; +import java.util.Vector; + +import org.testng.annotations.AfterClass; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + import jalview.api.FeatureRenderer; import jalview.api.structures.JalviewStructureDisplayI; import jalview.bin.Cache; @@ -48,16 +58,6 @@ import jalview.ws.sifts.SiftsClient; import jalview.ws.sifts.SiftsException; import jalview.ws.sifts.SiftsSettings; -import java.io.File; -import java.io.IOException; -import java.util.List; -import java.util.Vector; - -import org.testng.annotations.AfterClass; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.Test; - @Test(singleThreaded = true) public class JalviewChimeraView { @@ -149,7 +149,7 @@ public class JalviewChimeraView } } - assertTrue(binding.isChimeraRunning(), "Failed to start Chimera"); + assertTrue(binding.isViewerRunning(), "Failed to start Chimera"); assertEquals(chimeraViewer.getBinding().getPdbCount(), 1); chimeraViewer.closeViewer(true); @@ -209,7 +209,7 @@ public class JalviewChimeraView } } while (!binding.isFinishedInit()); - assertTrue(binding.isChimeraRunning(), "Failed to launch Chimera"); + assertTrue(binding.isViewerRunning(), "Failed to launch Chimera"); assertEquals(binding.getPdbCount(), 1); @@ -402,7 +402,7 @@ public class JalviewChimeraView } } while (!binding.isFinishedInit()); - assertTrue(binding.isChimeraRunning(), "Failed to launch Chimera"); + assertTrue(binding.isViewerRunning(), "Failed to launch Chimera"); assertEquals(binding.getPdbCount(), 1); -- 1.7.10.2