From a1042daec18f3b0b337503d68c5aad734da74d7b Mon Sep 17 00:00:00 2001
From: Paul McCarthy <pauld.mccarthy@gmail.com>
Date: Thu, 19 Nov 2015 16:55:47 +0000
Subject: [PATCH] Built-in perspective names are now reserved. User is prompted
 to choose a different name if they use a built-in name, and is prompted to
 confirm overwrite if they use the name of an existing (user-saved)
 perspective.

---
 fsl/data/strings.py                    | 10 ++++-
 fsl/fsleyes/actions/saveperspective.py | 55 ++++++++++++++++++--------
 fsl/fsleyes/frame.py                   |  4 +-
 fsl/fsleyes/perspectives.py            | 35 +++++++++++-----
 4 files changed, 76 insertions(+), 28 deletions(-)

diff --git a/fsl/data/strings.py b/fsl/data/strings.py
index 52722919c..d8065744c 100644
--- a/fsl/data/strings.py
+++ b/fsl/data/strings.py
@@ -52,9 +52,17 @@ messages = TypeDict({
     'ProcessingDialog.error' : 'An error has occurred: {}'
                                '\n\nDetails: {}',
 
-    'perspectives.savePerspective'     : 'Enter a name for the perspective',
     'perspectives.applyingPerspective' : 'Applying {} perspective ...',
 
+    'SavePerspectiveAction.enterName'        : 'Enter a name for the '
+                                               'perspective',
+    'SavePerspectiveAction.nameIsBuiltIn'    : '"{}" is a reserved '
+                                               'perspective name - '
+                                               'enter a different name.', 
+    'SavePerspectiveAction.confirmOverwrite' : 'A perspective with the name '
+                                               '"{}" already exists - do '
+                                               'you want to replace it?', 
+
     'ClearPerspectiveAction.confirmClear' : 'All saved perspectives will be '
                                             'cleared! Are you sure you want '
                                             'to continue?',
diff --git a/fsl/fsleyes/actions/saveperspective.py b/fsl/fsleyes/actions/saveperspective.py
index f121ccaea..c99a4eb79 100644
--- a/fsl/fsleyes/actions/saveperspective.py
+++ b/fsl/fsleyes/actions/saveperspective.py
@@ -38,22 +38,45 @@ class SavePerspectiveAction(action.Action):
         is saved via the :func:`.perspectives.savePerspective` function.
         """
 
-        dlg = wx.TextEntryDialog(
-            self.__frame,
-            message=strings.messages['perspectives.savePerspective'])
-
-        if dlg.ShowModal() != wx.ID_OK:
-            return
-
-        name = dlg.GetValue()
-
-        if name.strip() == '':
-            return
-
-        # TODO Prevent using built-in perspective names
-
-        # TODO Name collision - confirm overwrite
-
+        builtIns = perspectives.BUILT_IN_PERSPECTIVES.keys()
+        saved    = perspectives.getAllPerspectives()
+        
+        while True:
+            dlg = wx.TextEntryDialog(
+                self.__frame,
+                message=strings.messages[self, 'enterName'])
+
+            if dlg.ShowModal() != wx.ID_OK:
+                return
+
+            name = dlg.GetValue()
+
+            if name.strip() == '':
+                return
+
+            # Not allowed to use built-in perspective names
+            if name in builtIns:
+                dlg = wx.MessageDialog(
+                    self.__frame,
+                    message=strings.messages[
+                        self, 'nameIsBuiltIn'].format(name),
+                    style=(wx.ICON_EXCLAMATION | wx.OK))
+                dlg.ShowModal()
+                continue
+
+            # Name collision - confirm overwrite
+            if name in saved:
+                dlg = wx.MessageDialog(
+                    self.__frame,
+                    message=strings.messages[
+                        self, 'confirmOverwrite'].format(name),
+                    style=(wx.ICON_QUESTION | wx.YES_NO | wx.NO_DEFAULT))
+
+                if dlg.ShowModal() == wx.ID_NO:
+                    continue
+                
+            break
+                    
         perspectives.savePerspective(self.__frame, name)
 
         self.__frame.refreshPerspectiveMenu()
diff --git a/fsl/fsleyes/frame.py b/fsl/fsleyes/frame.py
index a10174cf5..32e8b9683 100644
--- a/fsl/fsleyes/frame.py
+++ b/fsl/fsleyes/frame.py
@@ -376,7 +376,9 @@ class FSLEyesFrame(wx.Frame):
         elif panel is not None:
             paneInfo = self.__auiManager.GetPane(panel)
 
-        if panel is None:
+        # This method may get called when
+        # the user closes a control panel
+        if panel is None or panel not in self.__viewPanels:
             return
 
         self       .__viewPanels    .remove(panel)
diff --git a/fsl/fsleyes/perspectives.py b/fsl/fsleyes/perspectives.py
index f8ca8d6de..9e5a4e069 100644
--- a/fsl/fsleyes/perspectives.py
+++ b/fsl/fsleyes/perspectives.py
@@ -53,24 +53,34 @@ def getAllPerspectives():
 def loadPerspective(frame, name):
     """
     """
-    log.debug('Loading perspective {}'.format(name))
 
-    persp = fslsettings.read('fsleyes.perspectives.{}'.format(name), None)
+    if name in BUILT_IN_PERSPECTIVES.keys():
+        
+        log.debug('Loading built-in perspective {}'.format(name))
+        persp = BUILT_IN_PERSPECTIVES[name]
+        
+    else:
+        log.debug('Loading saved perspective {}'.format(name))
+        persp = fslsettings.read('fsleyes.perspectives.{}'.format(name), None)
+
+    if persp is None:
+        raise ValueError('No perspective named "{}" exists'.format(name))
+
     log.debug('Serialised perspective:\n{}'.format(persp))
               
     persp = deserialisePerspective(persp)
-
     frameChildren, frameLayout, vpChildrens, vpLayouts = persp
 
+    # Show a message while re-configuring the frame
     dlg = fsldlg.SimpleMessageDialog(
         frame,
-        strings.messages['perspectives.applyingPerspective'].format(name))
+        strings.messages['perspectives.applyingPerspective'].format(
+            strings.perspectives.get(name, name)))
     dlg.Show()
 
     # Clear all existing view
     # panels from the frame
     for vp in frame.getViewPanels():
-        print 'Removing view panel {}'.format(type(vp).__name__)
         frame.removeViewPanel(vp)
 
     # Add all of the view panels
@@ -87,9 +97,9 @@ def loadPerspective(frame, name):
     for vp, vpChildren, vpLayout in zip(viewPanels, vpChildrens, vpLayouts):
         
         for child in vpChildren:
-            
             _addControlPanel(vp, child)
-            vp.getAuiManager().LoadPerspective(vpLayout)
+            
+        vp.getAuiManager().LoadPerspective(vpLayout)
 
     dlg.Close()
     dlg.Destroy()
@@ -98,8 +108,13 @@ def loadPerspective(frame, name):
 def savePerspective(frame, name):
     """
     """
+
+    if name in BUILT_IN_PERSPECTIVES.keys():
+        raise ValueError('A built-in perspective named "{}" '
+                         'already exists'.format(name))
     
     log.debug('Saving current perspective with name {}'.format(name))
+    
     persp = serialisePerspective(frame)
     fslsettings.write('fsleyes.perspectives.{}'.format(name), persp)
 
@@ -390,8 +405,8 @@ BUILT_IN_PERSPECTIVES = collections.OrderedDict((
      textwrap.dedent("""
                      OrthoPanel,TimeSeriesPanel,
                      layout2|name=OrthoPanel;caption=Ortho View 1;state=67377088;dir=5;layer=0;row=0;pos=0;prop=100000;bestw=853;besth=-1;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|name=TimeSeriesPanel;caption=Time series 2;state=67377148;dir=3;layer=0;row=0;pos=0;prop=100000;bestw=-1;besth=472;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|dock_size(5,0,0)=22|dock_size(3,0,0)=261|
-                     LocationPanel,ClusterPanel,OverlayListPanel,AtlasPanel,
-                     layout2|name=Panel;caption=;state=768;dir=5;layer=0;row=0;pos=0;prop=100000;bestw=20;besth=20;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|name=LocationPanel;caption=Location;state=67373052;dir=2;layer=1;row=0;pos=1;prop=98544;bestw=440;besth=109;minw=440;minh=109;maxw=-1;maxh=-1;floatx=1051;floaty=349;floatw=440;floath=125;notebookid=-1;transparent=255|name=ClusterPanel;caption=Cluster browser;state=67373052;dir=3;layer=0;row=0;pos=0;prop=100000;bestw=390;besth=96;minw=390;minh=96;maxw=-1;maxh=-1;floatx=276;floaty=578;floatw=390;floath=112;notebookid=-1;transparent=255|name=OverlayListPanel;caption=Overlay list;state=67373052;dir=2;layer=1;row=0;pos=2;prop=87792;bestw=197;besth=80;minw=197;minh=80;maxw=-1;maxh=-1;floatx=1047;floaty=492;floatw=197;floath=96;notebookid=-1;transparent=255|name=AtlasPanel;caption=Atlases;state=67373052;dir=2;layer=1;row=0;pos=0;prop=113664;bestw=318;besth=84;minw=318;minh=84;maxw=-1;maxh=-1;floatx=1091;floaty=143;floatw=318;floath=100;notebookid=-1;transparent=255|dock_size(5,0,0)=10|dock_size(3,0,0)=130|dock_size(2,1,0)=566|
+                     OverlayDisplayToolBar,LocationPanel,AtlasPanel,OverlayListPanel,OrthoToolBar,ClusterPanel,
+                     layout2|name=Panel;caption=;state=768;dir=5;layer=0;row=0;pos=0;prop=100000;bestw=20;besth=20;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|name=OverlayDisplayToolBar;caption=Display toolbar;state=67382012;dir=1;layer=10;row=0;pos=0;prop=100000;bestw=860;besth=49;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|name=LocationPanel;caption=Location;state=67373052;dir=3;layer=2;row=0;pos=1;prop=98544;bestw=440;besth=109;minw=440;minh=109;maxw=-1;maxh=-1;floatx=2730;floaty=1104;floatw=440;floath=125;notebookid=-1;transparent=255|name=AtlasPanel;caption=Atlases;state=67373052;dir=2;layer=1;row=0;pos=0;prop=98904;bestw=318;besth=84;minw=318;minh=84;maxw=-1;maxh=-1;floatx=1091;floaty=143;floatw=318;floath=100;notebookid=-1;transparent=255|name=OverlayListPanel;caption=Overlay list;state=67373052;dir=3;layer=2;row=0;pos=0;prop=87792;bestw=197;besth=80;minw=197;minh=80;maxw=-1;maxh=-1;floatx=2608;floaty=1116;floatw=197;floath=96;notebookid=-1;transparent=255|name=OrthoToolBar;caption=Ortho view toolbar;state=67382012;dir=1;layer=10;row=1;pos=0;prop=100000;bestw=815;besth=34;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=2072;floaty=80;floatw=824;floath=50;notebookid=-1;transparent=255|name=ClusterPanel;caption=Cluster browser;state=67373052;dir=2;layer=1;row=0;pos=1;prop=114760;bestw=390;besth=96;minw=390;minh=96;maxw=-1;maxh=-1;floatx=3516;floaty=636;floatw=390;floath=112;notebookid=-1;transparent=255|dock_size(5,0,0)=10|dock_size(2,1,0)=566|dock_size(1,10,0)=51|dock_size(1,10,1)=36|dock_size(3,2,0)=130|
                      ,
-                     layout2|name=FigureCanvasWxAgg;caption=;state=768;dir=5;layer=0;row=0;pos=0;prop=100000;bestw=640;besth=480;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|dock_size(5,0,0)=10| 
+                     layout2|name=FigureCanvasWxAgg;caption=;state=768;dir=5;layer=0;row=0;pos=0;prop=100000;bestw=640;besth=480;minw=-1;minh=-1;maxw=-1;maxh=-1;floatx=-1;floaty=-1;floatw=-1;floath=-1;notebookid=-1;transparent=255|dock_size(5,0,0)=642|
                      """))))
-- 
GitLab