Refactor code used by %edit for better sharing.

Merge branch 'i426' of https://github.com/takluyver/ipython into takluyver-i426

IPython/core/magic.py

@@ -89,6 +89,9 b' def needs_local_scope(func):'
     """Decorator to mark magic functions which need to local scope to run."""
     func.needs_local_scope = True
     return func
+    
+# Used for exception handling in magic_edit
+class MacroToEdit(ValueError): pass
 
 #***************************************************************************
 # Main class implementing Magic functionality
@@ -2020,7 +2023,6 b' Currently the magic system has the following functions:\\n"""'
           'print macro_name'.
           
         """
-
         opts,args = self.parse_options(parameter_s,'r',mode='list')
         if not args:   # List existing macros
             return sorted(k for k,v in self.shell.user_ns.iteritems() if\
@@ -2111,6 +2113,126 b' Currently the magic system has the following functions:\\n"""'
         else:
             content = open(arg_s).read()
         self.set_next_input(content)
+        
+    def _find_edit_target(self, args, opts, last_call):
+        """Utility method used by magic_edit to find what to edit."""
+        
+        def make_filename(arg):
+            "Make a filename from the given args"
+            try:
+                filename = get_py_filename(arg)
+            except IOError:
+                # If it ends with .py but doesn't already exist, assume we want
+                # a new file. 
+                if args.endswith('.py'):
+                    filename = arg
+                else:
+                    filename = None
+            return filename
+        
+        # Set a few locals from the options for convenience:
+        opts_prev = 'p' in opts
+        opts_raw = 'r' in opts
+
+        # custom exceptions
+        class DataIsObject(Exception): pass
+        
+        # Default line number value
+        lineno = opts.get('n',None)
+
+        if opts_prev:
+            args = '_%s' % last_call[0]
+            if not self.shell.user_ns.has_key(args):
+                args = last_call[1]
+            
+        # use last_call to remember the state of the previous call, but don't
+        # let it be clobbered by successive '-p' calls.
+        try:
+            last_call[0] = self.shell.displayhook.prompt_count
+            if not opts_prev:
+                last_call[1] = parameter_s
+        except:
+            pass
+
+        # by default this is done with temp files, except when the given
+        # arg is a filename
+        use_temp = True
+
+        data = ''
+        
+        # First, see if the arguments should be a filename.
+        filename = make_filename(args)
+        if filename:
+            use_temp = False
+        elif args:
+            # Mode where user specifies ranges of lines, like in %macro.
+            data = self.extract_input_lines(args, opts_raw)
+            if not data:
+                try:
+                    # Load the parameter given as a variable. If not a string,
+                    # process it as an object instead (below)
+
+                    #print '*** args',args,'type',type(args)  # dbg
+                    data = eval(args, self.shell.user_ns)
+                    if not isinstance(data, basestring):
+                        raise DataIsObject
+
+                except (NameError,SyntaxError):
+                    # given argument is not a variable, try as a filename
+                    filename = make_filename(args)
+                    if filename is None:
+                        warn("Argument given (%s) can't be found as a variable "
+                             "or as a filename." % args)
+                        return
+                    use_temp = False
+                
+                except DataIsObject:
+                    # macros have a special edit function
+                    if isinstance(data, Macro):
+                        raise MacroToEdit(data)
+                                    
+                    # For objects, try to edit the file where they are defined
+                    try:
+                        filename = inspect.getabsfile(data)
+                        if 'fakemodule' in filename.lower() and inspect.isclass(data):                     
+                            # class created by %edit? Try to find source
+                            # by looking for method definitions instead, the
+                            # __module__ in those classes is FakeModule.
+                            attrs = [getattr(data, aname) for aname in dir(data)]
+                            for attr in attrs:
+                                if not inspect.ismethod(attr):
+                                    continue
+                                filename = inspect.getabsfile(attr)
+                                if filename and 'fakemodule' not in filename.lower():
+                                    # change the attribute to be the edit target instead
+                                    data = attr 
+                                    break
+                        
+                        datafile = 1
+                    except TypeError:
+                        filename = make_filename(args)
+                        datafile = 1
+                        warn('Could not find file where `%s` is defined.\n'
+                             'Opening a file named `%s`' % (args,filename))
+                    # Now, make sure we can actually read the source (if it was in
+                    # a temp file it's gone by now).
+                    if datafile:
+                        try:
+                            if lineno is None:
+                                lineno = inspect.getsourcelines(data)[1]
+                        except IOError:
+                            filename = make_filename(args)
+                            if filename is None:
+                                warn('The file `%s` where `%s` was defined cannot '
+                                     'be read.' % (filename,data))
+                                return
+                    use_temp = False
+
+        if use_temp:
+            filename = self.shell.mktempfile(data)
+            print 'IPython will make a temporary file named:',filename
+        
+        return filename, lineno, use_temp
 
     def _edit_macro(self,mname,macro):
         """open an editor with the macro data in a file"""
@@ -2126,7 +2248,7 b' Currently the magic system has the following functions:\\n"""'
     def magic_ed(self,parameter_s=''):
         """Alias to %edit."""
         return self.magic_edit(parameter_s)
-
+    
     @skip_doctest
     def magic_edit(self,parameter_s='',last_call=['','']):
         """Bring up an editor and execute the resulting code.
@@ -2269,122 +2391,13 b' Currently the magic system has the following functions:\\n"""'
         starting example for further modifications.  That file also has
         general instructions on how to set a new hook for use once you've
         defined it."""
-        
-        # FIXME: This function has become a convoluted mess.  It needs a
-        # ground-up rewrite with clean, simple logic.
-
-        def make_filename(arg):
-            "Make a filename from the given args"
-            try:
-                filename = get_py_filename(arg)
-            except IOError:
-                if args.endswith('.py'):
-                    filename = arg
-                else:
-                    filename = None
-            return filename
-
-        # custom exceptions
-        class DataIsObject(Exception): pass
-
         opts,args = self.parse_options(parameter_s,'prxn:')
-        # Set a few locals from the options for convenience:
-        opts_prev = 'p' in opts
-        opts_raw = 'r' in opts
         
-        # Default line number value
-        lineno = opts.get('n',None)
-
-        if opts_prev:
-            args = '_%s' % last_call[0]
-            if not self.shell.user_ns.has_key(args):
-                args = last_call[1]
-            
-        # use last_call to remember the state of the previous call, but don't
-        # let it be clobbered by successive '-p' calls.
         try:
-            last_call[0] = self.shell.displayhook.prompt_count
-            if not opts_prev:
-                last_call[1] = parameter_s
-        except:
-            pass
-
-        # by default this is done with temp files, except when the given
-        # arg is a filename
-        use_temp = True
-
-        data = ''
-        if args.endswith('.py'):
-            filename = make_filename(args)
-            use_temp = False
-        elif args:
-            # Mode where user specifies ranges of lines, like in %macro.
-            data = self.extract_input_lines(args, opts_raw)
-            if not data:
-                try:
-                    # Load the parameter given as a variable. If not a string,
-                    # process it as an object instead (below)
-
-                    #print '*** args',args,'type',type(args)  # dbg
-                    data = eval(args, self.shell.user_ns)
-                    if not isinstance(data, basestring):
-                        raise DataIsObject
-
-                except (NameError,SyntaxError):
-                    # given argument is not a variable, try as a filename
-                    filename = make_filename(args)
-                    if filename is None:
-                        warn("Argument given (%s) can't be found as a variable "
-                             "or as a filename." % args)
-                        return
-                    use_temp = False
-                
-                except DataIsObject:
-                    # macros have a special edit function
-                    if isinstance(data, Macro):
-                        self._edit_macro(args,data)
-                        return
-                                    
-                    # For objects, try to edit the file where they are defined
-                    try:
-                        filename = inspect.getabsfile(data)
-                        if 'fakemodule' in filename.lower() and inspect.isclass(data):                     
-                            # class created by %edit? Try to find source
-                            # by looking for method definitions instead, the
-                            # __module__ in those classes is FakeModule.
-                            attrs = [getattr(data, aname) for aname in dir(data)]
-                            for attr in attrs:
-                                if not inspect.ismethod(attr):
-                                    continue
-                                filename = inspect.getabsfile(attr)
-                                if filename and 'fakemodule' not in filename.lower():
-                                    # change the attribute to be the edit target instead
-                                    data = attr 
-                                    break
-                        
-                        datafile = 1
-                    except TypeError:
-                        filename = make_filename(args)
-                        datafile = 1
-                        warn('Could not find file where `%s` is defined.\n'
-                             'Opening a file named `%s`' % (args,filename))
-                    # Now, make sure we can actually read the source (if it was in
-                    # a temp file it's gone by now).
-                    if datafile:
-                        try:
-                            if lineno is None:
-                                lineno = inspect.getsourcelines(data)[1]
-                        except IOError:
-                            filename = make_filename(args)
-                            if filename is None:
-                                warn('The file `%s` where `%s` was defined cannot '
-                                     'be read.' % (filename,data))
-                                return
-                    use_temp = False
-
-        if use_temp:
-            filename = self.shell.mktempfile(data)
-            print 'IPython will make a temporary file named:',filename
+            filename, lineno, is_temp = self._find_edit_target(args, opts, last_call)
+        except MacroToEdit as e:
+            self._edit_macro(args, e.args[0])
+            return
 
         # do actual editing here
         print 'Editing...',
@@ -2392,7 +2405,7 b' Currently the magic system has the following functions:\\n"""'
         try:
             # Quote filenames that may have spaces in them
             if ' ' in filename:
-                filename = "%s" % filename
+                filename = "'%s'" % filename
             self.shell.hooks.editor(filename,lineno)
         except TryNext:
             warn('Could not open editor')
@@ -2407,15 +2420,14 b' Currently the magic system has the following functions:\\n"""'
             print
         else:
             print 'done. Executing edited code...'
-            if opts_raw:
+            if 'r' in opts:    # Untranslated IPython code
                 self.shell.run_cell(file_read(filename),
                                                     store_history=False)
             else:
                 self.shell.safe_execfile(filename,self.shell.user_ns,
                                          self.shell.user_ns)
-        
                                                      
-        if use_temp:
+        if is_temp:
             try:
                 return open(filename).read()
             except IOError,msg:

IPython/zmq/zmqshell.py

@@ -29,6 +29,7 b' from IPython.core.autocall import ZMQExitAutocall'
 from IPython.core.displayhook import DisplayHook
 from IPython.core.displaypub import DisplayPublisher
 from IPython.core.macro import Macro
+from IPython.core.magic import MacroToEdit
 from IPython.core.payloadpage import install_payload_page
 from IPython.utils import io
 from IPython.utils.path import get_py_filename
@@ -399,130 +400,14 b' class ZMQInteractiveShell(InteractiveShell):'
         general instructions on how to set a new hook for use once you've
         defined it."""
         
-        # FIXME: This function has become a convoluted mess.  It needs a
-        # ground-up rewrite with clean, simple logic.
-
-        def make_filename(arg):
-            "Make a filename from the given args"
-            try:
-                filename = get_py_filename(arg)
-            except IOError:
-                if args.endswith('.py'):
-                    filename = arg
-                else:
-                    filename = None
-            return filename
-
-        # custom exceptions
-        class DataIsObject(Exception): pass
-
         opts,args = self.parse_options(parameter_s,'prn:')
-        # Set a few locals from the options for convenience:
-        opts_p = opts.has_key('p')
-        opts_r = opts.has_key('r')
         
-        # Default line number value
-        lineno = opts.get('n',None)
-        if lineno is not None:
-            try:
-                lineno = int(lineno)
-            except:
-                warn("The -n argument must be an integer.")
-                return
-
-        if opts_p:
-            args = '_%s' % last_call[0]
-            if not self.shell.user_ns.has_key(args):
-                args = last_call[1]
-            
-        # use last_call to remember the state of the previous call, but don't
-        # let it be clobbered by successive '-p' calls.
         try:
-            last_call[0] = self.shell.displayhook.prompt_count
-            if not opts_p:
-                last_call[1] = parameter_s
-        except:
-            pass
-
-        # by default this is done with temp files, except when the given
-        # arg is a filename
-        use_temp = True
-
-        data = ''
-        if args[0].isdigit():
-            # Mode where user specifies ranges of lines, like in %macro.
-            # This means that you can't edit files whose names begin with
-            # numbers this way. Tough.
-            ranges = args.split()
-            data = ''.join(self.extract_input_slices(ranges,opts_r))
-        elif args.endswith('.py'):
-            filename = make_filename(args)
-            use_temp = False
-        elif args:
-            try:
-                # Load the parameter given as a variable. If not a string,
-                # process it as an object instead (below)
-
-                #print '*** args',args,'type',type(args)  # dbg
-                data = eval(args, self.shell.user_ns)
-                if not isinstance(data, basestring):
-                    raise DataIsObject
-
-            except (NameError,SyntaxError):
-                # given argument is not a variable, try as a filename
-                filename = make_filename(args)
-                if filename is None:
-                    warn("Argument given (%s) can't be found as a variable "
-                         "or as a filename." % args)
-                    return
-                use_temp = False
-                
-            except DataIsObject:
-                # macros have a special edit function
-                if isinstance(data, Macro):
-                    self._edit_macro(args,data)
-                    return
-                                
-                # For objects, try to edit the file where they are defined
-                try:
-                    filename = inspect.getabsfile(data)
-                    if 'fakemodule' in filename.lower() and inspect.isclass(data):                     
-                        # class created by %edit? Try to find source
-                        # by looking for method definitions instead, the
-                        # __module__ in those classes is FakeModule.
-                        attrs = [getattr(data, aname) for aname in dir(data)]
-                        for attr in attrs:
-                            if not inspect.ismethod(attr):
-                                continue
-                            filename = inspect.getabsfile(attr)
-                            if filename and 'fakemodule' not in filename.lower():
-                                # change the attribute to be the edit target instead
-                                data = attr 
-                                break
-                    
-                    datafile = 1
-                except TypeError:
-                    filename = make_filename(args)
-                    datafile = 1
-                    warn('Could not find file where `%s` is defined.\n'
-                         'Opening a file named `%s`' % (args,filename))
-                # Now, make sure we can actually read the source (if it was in
-                # a temp file it's gone by now).
-                if datafile:
-                    try:
-                        if lineno is None:
-                            lineno = inspect.getsourcelines(data)[1]
-                    except IOError:
-                        filename = make_filename(args)
-                        if filename is None:
-                            warn('The file `%s` where `%s` was defined cannot '
-                                 'be read.' % (filename,data))
-                            return
-                use_temp = False
-
-        if use_temp:
-            filename = self.shell.mktempfile(data)
-            print('IPython will make a temporary file named:', filename)
+            filename, lineno, _ = self._find_edit_target(args, opts, last_call)
+        except MacroToEdit as e:
+            # TODO: Implement macro editing over 2 processes.
+            print("Macro editing not yet implemented in 2-process model.")
+            return
 
         # Make sure we send to the client an absolute path, in case the working
         # directory of client and kernel don't match