refactor special case in userEnterCommand for already being in command

author: Bruce Smith <bruce@nanorex.com> 2008-07-30 21:34:54 +0000
committer: Bruce Smith <bruce@nanorex.com> 2008-07-30 21:34:54 +0000
commit: e13b3b6a088ab598498d0c0537ba5976fb6a3d58 (patch)
tree: 7fd953867d95ea8d2594284ca3f19a1808fa3e12
parent: 0d060dea74f0792554898e69ce815319c35db7cb (diff)
download: nanoengineer-e13b3b6a088ab598498d0c0537ba5976fb6a3d58.tar.gz
nanoengineer-e13b3b6a088ab598498d0c0537ba5976fb6a3d58.zip
9 files changed, 72 insertions, 58 deletions
diff --git a/cad/src/analysis/ESP/ESPImage.py b/cad/src/analysis/ESP/ESPImage.py
index 83b10f413..c903cace6 100755
--- a/cad/src/analysis/ESP/ESPImage.py
+++ b/cad/src/analysis/ESP/ESPImage.py
@@ -306,8 +306,7 @@ class ESPImage(RectGadget):
         #bruce 060403 changes: force Build, not Select Atoms as before; only do this if current mode is not Build.
         # (But why do we need to force it into any particular mode? I don't know. [bruce 070608])
         commandSequencer = self.assy.w.commandSequencer #bruce 071008
-        if commandSequencer.currentCommand.commandName != 'DEPOSIT':
-            commandSequencer.userEnterCommand('DEPOSIT')
+        commandSequencer.userEnterCommand('DEPOSIT')
         Jig.edit(self)
 
     def make_selobj_cmenu_items(self, menu_spec):
diff --git a/cad/src/cnt/model/NanotubeGroup.py b/cad/src/cnt/model/NanotubeGroup.py
index 87cddb5d4..fe8f6f5f9 100644
--- a/cad/src/cnt/model/NanotubeGroup.py
+++ b/cad/src/cnt/model/NanotubeGroup.py
@@ -131,7 +131,7 @@ class NanotubeGroup(Group):
         @see: Group.edit()
         """
         commandSequencer = self.assy.w.commandSequencer
-        commandSequencer.userEnterCommand('BUILD_NANOTUBE')
+        commandSequencer.userEnterCommand('BUILD_NANOTUBE', always_update = True)
         currentCommand = commandSequencer.currentCommand
         assert currentCommand.commandName == 'BUILD_NANOTUBE'
         currentCommand.editStructure(self)
diff --git a/cad/src/commandSequencer/CommandSequencer.py b/cad/src/commandSequencer/CommandSequencer.py
index 6981bda5f..fb74d4445 100644
--- a/cad/src/commandSequencer/CommandSequencer.py
+++ b/cad/src/commandSequencer/CommandSequencer.py
@@ -394,7 +394,8 @@ class modeMixin(object):
 
     # user requests a specific new command.
 
-    def userEnterCommand(self, commandName, **options): # needs revision or replacement for USE_COMMAND_STACK
+    def userEnterCommand(self, commandName, always_update = False, **options):
+        # TODO: needs revision or replacement for USE_COMMAND_STACK
         """
         Public method, called from the UI when the user asks to enter
         a specific command (named by commandName), e.g. using a toolbutton
@@ -406,6 +407,14 @@ class modeMixin(object):
         a command instance object. (Details of commandName, and all options,
         are documented in Command._f_userEnterCommand.)
 
+        If commandName is a command name string and we are already in that
+        command, then do nothing unless always_update is true [new feature,
+        080730; prior code did nothing except self.update_after_new_mode(),
+        equivalent to passing always_update = True to new code].
+        Note: all calls which pass always_update as of 080730 do so only
+        to preserve old code behavior; passing it is probably not needed
+        for most of them. [###REVIEW those calls]
+
         The current command has to exit (or be suspended) before the new one
         can be entered, but it's allowed to refuse to exit, and if it does
         exit it needs a chance to clean up first. So we let the current
@@ -427,22 +436,34 @@ class modeMixin(object):
         self.update_after_new_mode().
         [Note, that's now in GLPane but should probably move into this class.]
         
-        See also: userEnterTemporaryCommand
+        @see: userEnterTemporaryCommand (which is the only caller that passes
+              us any options, as of before 080730)
+
+        @see: MWsemantics.ensureInCommand
         """
-        # Note: we don't have a special case for already being in the same
-        # command; individual commands can implement that if they wish.
+        # Note: _f_userEnterCommand has a special case for already being in
+        # the same-named command, provided that is a basicCommand subclass
+        # (i.e. not nullCommand). A lot of callers have a test for this
+        # before the call, but they don't need it, except that it
+        # avoids the call herein of self.update_after_new_mode().
+        # CHANGING THIS NOW: avoid that update here too, and simplify the callers.
+        # [bruce 080730]
         try:
-            self.currentCommand._f_userEnterCommand(commandName, **options)
-
-            # REVIEW: the following update_after_new_mode looks redundant with
-            # the one at the end of start_using_mode, if that one has always
-            # run at this point (which I think, but didn't prove).
-            # [bruce 070813 comment]
+            already_in_command = (self.currentCommand.commandName == commandName)
             
-            # TODO, maybe: let current command decide whether/how to do
-            # this update:
-            self.update_after_new_mode()
-                # might be unnecessary if command didn't change -- that's ok
+            if not already_in_command:
+                self.currentCommand._f_userEnterCommand(commandName, **options)
+
+            if always_update or not already_in_command:
+                # REVIEW: the following update_after_new_mode looks redundant with
+                # the one at the end of start_using_mode, if that one has always
+                # run at this point (which I think, but didn't prove).
+                # [bruce 070813 comment]
+                
+                # TODO, maybe: let current command decide whether/how to do
+                # this update:
+                self.update_after_new_mode()
+            pass
         except:
             # This should never happen unless there's a bug in some command --
             # so don't bother trying to get into the user's requested
@@ -482,9 +503,9 @@ class modeMixin(object):
         (This means a series of temporary commands can be run,
         after which the prior non-temporary one will be resumed.)
 
-        Note: semantics/API is likely to be revised; see code comments.
+        @note: semantics/API is likely to be revised; see code comments.
 
-        See also: userEnterCommand
+        @see: userEnterCommand
         """
         # REVIEW: do we need to generalize command.command_can_be_suspended
         # to a relation between two commands
@@ -550,7 +571,7 @@ class modeMixin(object):
         # Set self.prevMode (our depth-1 suspended command stack)
         self.prevMode = prior_command
             # bruce 070813 save command object, not commandName
-        self.userEnterCommand(commandName, suspend_old_mode = True)
+        self.userEnterCommand(commandName, suspend_old_mode = True, always_update = True) ## todo: remove always_update if ok.
             # TODO: if this can become the only use of suspend_old_mode,
             # make it a private option _suspend_old_mode.
             # Indeed, it's now the only use except for internal and
@@ -798,7 +819,7 @@ class modeMixin(object):
         self._commandTable[commandName] = modeobj
             # also put it in under this name, if different
             ### [will this cause bugs?]
-        self.userEnterCommand(commandName)
+        self.userEnterCommand(commandName, always_update = True)
             # note: self is acting as the command sequencer here
         return
 
diff --git a/cad/src/commands/PlayMovie/movieMode.py b/cad/src/commands/PlayMovie/movieMode.py
index d9c7fce53..ce223fec5 100755
--- a/cad/src/commands/PlayMovie/movieMode.py
+++ b/cad/src/commands/PlayMovie/movieMode.py
@@ -404,7 +404,7 @@ def simMoviePlayer(assy):
         return
 
     if assy.current_movie and assy.current_movie.might_be_playable():
-        win.commandSequencer.userEnterCommand('MOVIE')
+        win.commandSequencer.userEnterCommand('MOVIE', always_update = True)
         return
 
     # no valid current movie, look for saved one with same name as assy
@@ -431,14 +431,14 @@ def simMoviePlayer(assy):
             # for any loaded Part. So let's not... tho we might presume (from filename choice we used)
             # it was valid for Main Part. Maybe print warning for clip item, and for not valid? #e
             env.history.message("Movie Player: %s previously saved movie for this part." % ("playing" or "loading"))
-            win.commandSequencer.userEnterCommand('MOVIE')
+            win.commandSequencer.userEnterCommand('MOVIE', always_update = True)
             return
     # else if no assy.filename or no movie found from that:
     # bruce 050327 comment -- do what the old code did, except for the moviePlay
     # which seems wrong and tracebacks now.
     assy.current_movie = Movie(assy)
         # temporary kluge until bugs in movieMode for no assy.current_movie are fixed
-    win.commandSequencer.userEnterCommand('MOVIE')
+    win.commandSequencer.userEnterCommand('MOVIE', always_update = True)
     return
 
 # end
diff --git a/cad/src/dna/model/DnaGroup.py b/cad/src/dna/model/DnaGroup.py
index 7c434b19f..81c2c65b4 100644
--- a/cad/src/dna/model/DnaGroup.py
+++ b/cad/src/dna/model/DnaGroup.py
@@ -153,7 +153,7 @@ class DnaGroup(Group):
         @see: Group.edit()
         """
         commandSequencer = self.assy.w.commandSequencer
-        commandSequencer.userEnterCommand('BUILD_DNA')
+        commandSequencer.userEnterCommand('BUILD_DNA', always_update = True)
         currentCommand = commandSequencer.currentCommand
         assert currentCommand.commandName == 'BUILD_DNA'
         currentCommand.editStructure(self)
diff --git a/cad/src/foundation/Group.py b/cad/src/foundation/Group.py
index 766031edb..bbc09bdb8 100755
--- a/cad/src/foundation/Group.py
+++ b/cad/src/foundation/Group.py
@@ -1163,7 +1163,10 @@ class Group(NodeWithAtomContents):
         """
         if self.editCommand:
             commandSequencer = self.assy.w.commandSequencer
-            commandSequencer.userEnterCommand('DNA_DUPLEX')
+            commandSequencer.userEnterCommand('DNA_DUPLEX', always_update = True)
+                ### REVIEW: is this special treatment of DNA_DUPLEX justified
+                # in this class? Same question for the getProps and setProps
+                # methods below. [bruce 080730 question]
             currentCommand = commandSequencer.currentCommand
             assert currentCommand.commandName == 'DNA_DUPLEX'
             currentCommand.editStructure(self)
diff --git a/cad/src/ne1_ui/MWsemantics.py b/cad/src/ne1_ui/MWsemantics.py
index 0667f7379..7b5746c0f 100755
--- a/cad/src/ne1_ui/MWsemantics.py
+++ b/cad/src/ne1_ui/MWsemantics.py
@@ -924,9 +924,7 @@ class MWsemantics(QMainWindow,
             #toolbar for dna is not visible . This whole thing will get revised 
             #after the command stack cleanup (to be coded soon)
             # -- Ninad 2008-07-29
-            if currentCommand.commandName != "PASTE":
-                commandSequencer.userEnterCommand('PASTE') 
-                return
+            commandSequencer.userEnterCommand('PASTE')
         else:
             msg = orangemsg("Clipboard is empty. Paste Command cancelled.")
             env.history.message(msg)
@@ -1376,11 +1374,11 @@ class MWsemantics(QMainWindow,
 
     # get into Select Atoms mode
     def toolsSelectAtoms(self): # note: this can NO LONGER be called from update_select_mode [as of bruce 060403]
-        self.commandSequencer.userEnterCommand('SELECTATOMS')
+        self.commandSequencer.userEnterCommand('SELECTATOMS', always_update = True)
 
     # get into Select Chunks mode
     def toolsSelectMolecules(self):# note: this can also be called from update_select_mode [bruce 060403 comment]
-        self.commandSequencer.userEnterCommand('SELECTMOLS')
+        self.commandSequencer.userEnterCommand('SELECTMOLS', always_update = True)
 
     # get into Move Chunks (or Translate Components) command
     def toolsMoveMolecule(self):
@@ -1397,19 +1395,19 @@ class MWsemantics(QMainWindow,
     # get into Build mode
     def toolsBuildAtoms(self): # note: this can now be called from update_select_mode [as of bruce 060403]
         self.depositState = 'Atoms'
-        self.commandSequencer.userEnterCommand('DEPOSIT')
+        self.commandSequencer.userEnterCommand('DEPOSIT', always_update = True)
 
     # get into cookiecutter mode
     def toolsCookieCut(self):
-        self.commandSequencer.userEnterCommand('COOKIE')
+        self.commandSequencer.userEnterCommand('COOKIE', always_update = True)
 
     # get into Extrude mode
     def toolsExtrude(self):
-        self.commandSequencer.userEnterCommand('EXTRUDE')
+        self.commandSequencer.userEnterCommand('EXTRUDE', always_update = True)
 
     # get into Fuse Chunks mode
     def toolsFuseChunks(self):
-        self.commandSequencer.userEnterCommand('FUSECHUNKS')
+        self.commandSequencer.userEnterCommand('FUSECHUNKS', always_update = True)
 
     ###################################
     # Simulator Toolbar Slots
@@ -1547,11 +1545,10 @@ class MWsemantics(QMainWindow,
         If the current command's .commandName differs from the one given, change
         to that command.
 
-        @note: it's likely that this method is not needed since
-        userEnterCommand has the same special case of doing nothing
-        if we're already in the named command. If so, the special case
-        could be removed with no effect, and this method could be
-        inlined to just userEnterCommand.
+        @note: As of 080730, userEnterCommand has the same special case
+               of doing nothing if we're already in the named command.
+               So we just call it. (Even before, it had almost that
+               special case; see its docstring for details.)
 
         @note: all uses of this method are causes for suspicion, about
         whether some sort of refactoring or generalization is called for,
@@ -1562,8 +1559,7 @@ class MWsemantics(QMainWindow,
          but maybe not using this method in particular.)
         """
         commandSequencer = self.commandSequencer
-        if commandSequencer.currentCommand.commandName != commandName:
-            commandSequencer.userEnterCommand(commandName)
+        commandSequencer.userEnterCommand(commandName)
             # note: this changes the value of .currentCommand
         return
 
@@ -1573,13 +1569,10 @@ class MWsemantics(QMainWindow,
 
     def insertGraphene(self):   
         """
-        Invokes the graphene command ('BUILD_GRAPHNE')
+        Invokes the graphene command ('BUILD_GRAPHENE')
         """
         commandSequencer = self.commandSequencer
-        currentCommand = commandSequencer.currentCommand
-        if currentCommand.commandName != "BUILD_GRAPHENE":
-            commandSequencer.userEnterCommand(
-                'BUILD_GRAPHENE')
+        commandSequencer.userEnterCommand('BUILD_GRAPHENE')
 
         self.commandSequencer.currentCommand.runCommand()
 
@@ -1606,8 +1599,7 @@ class MWsemantics(QMainWindow,
             selNanotubeGroup.edit()
         else:
             commandSequencer = self.commandSequencer
-            if commandSequencer.currentCommand.commandName != 'BUILD_NANOTUBE':
-                commandSequencer.userEnterCommand('BUILD_NANOTUBE')
+            commandSequencer.userEnterCommand('BUILD_NANOTUBE')
 
             assert self.commandSequencer.currentCommand.commandName == 'BUILD_NANOTUBE'
             self.commandSequencer.currentCommand.runCommand()
@@ -1725,8 +1717,7 @@ class MWsemantics(QMainWindow,
         @see:B{self.insertDna}
         """
         commandSequencer = self.commandSequencer
-        if commandSequencer.currentCommand.commandName != 'DNA_DUPLEX':
-            commandSequencer.userEnterCommand('DNA_DUPLEX')
+        commandSequencer.userEnterCommand('DNA_DUPLEX')
 
         assert self.commandSequencer.currentCommand.commandName == 'DNA_DUPLEX'
 
@@ -1749,8 +1740,7 @@ class MWsemantics(QMainWindow,
             selDnaGroup.edit()
         else:
             commandSequencer = self.commandSequencer
-            if commandSequencer.currentCommand.commandName != 'BUILD_DNA':
-                commandSequencer.userEnterCommand('BUILD_DNA')
+            commandSequencer.userEnterCommand('BUILD_DNA')
 
             assert self.commandSequencer.currentCommand.commandName == 'BUILD_DNA'
             self.commandSequencer.currentCommand.runCommand()
@@ -1838,8 +1828,7 @@ class MWsemantics(QMainWindow,
             self.insertPeptide()
         else:
             commandSequencer = self.commandSequencer
-            if commandSequencer.currentCommand.commandName != 'BUILD_PROTEIN':
-                commandSequencer.userEnterCommand('BUILD_PROTEIN')
+            commandSequencer.userEnterCommand('BUILD_PROTEIN')
                 
             assert self.commandSequencer.currentCommand.commandName == 'BUILD_PROTEIN'
             self.commandSequencer.currentCommand.runCommand()
diff --git a/cad/src/protein/model/PeptideGroup.py b/cad/src/protein/model/PeptideGroup.py
index e48a933c9..8cd9b305b 100644
--- a/cad/src/protein/model/PeptideGroup.py
+++ b/cad/src/protein/model/PeptideGroup.py
@@ -131,7 +131,7 @@ class PeptideGroup(Group):
         @see: Group.edit()
         """
         commandSequencer = self.assy.w.commandSequencer
-        commandSequencer.userEnterCommand('BUILD_Peptide')
+        commandSequencer.userEnterCommand('BUILD_Peptide', always_update = True)
         currentCommand = commandSequencer.currentCommand
         assert currentCommand.commandName == 'BUILD_Peptide'
         currentCommand.editStructure(self)
diff --git a/cad/src/prototype/test_commands_init.py b/cad/src/prototype/test_commands_init.py
index 5ca589ad6..8459af4c6 100755
--- a/cad/src/prototype/test_commands_init.py
+++ b/cad/src/prototype/test_commands_init.py
@@ -95,8 +95,10 @@ def enter_example_command(widget, example_command_classname):
 def enter_example_command_doit(glpane, example_command_classname):
     example_command_class = globals()[example_command_classname]
     example_command_class.commandName += 'x'
-        # kluge to defeat _f_userEnterCommand comparison of commandName -- not sure if it works; pretty sure it's needed for now
-        # TODO: replace it with a new option to pass to that method
+        # kluge to defeat _f_userEnterCommand (and now userEnterCommand)
+        # comparison of commandName -- not sure if it works; pretty sure
+        # it's needed for now
+        # TODO: replace it with a new option to pass to that method.
     cmdrun = construct_cmdrun(example_command_class, glpane)
     start_cmdrun(cmdrun)
     return
author	Bruce Smith <bruce@nanorex.com>	2008-07-30 21:34:54 +0000
committer	Bruce Smith <bruce@nanorex.com>	2008-07-30 21:34:54 +0000
commit	e13b3b6a088ab598498d0c0537ba5976fb6a3d58 (patch)
tree	7fd953867d95ea8d2594284ca3f19a1808fa3e12
parent	0d060dea74f0792554898e69ce815319c35db7cb (diff)
download	nanoengineer-e13b3b6a088ab598498d0c0537ba5976fb6a3d58.tar.gz nanoengineer-e13b3b6a088ab598498d0c0537ba5976fb6a3d58.zip