Skip to content
This repository

Allow %magic argument filenames with spaces to be specified with quotes under win32 #734

Merged
merged 2 commits into from over 2 years ago

3 participants

Robert Kern Evan Patterson Jonathan March
Robert Kern
Collaborator

This just modifies get_py_filename() to apply Windows semantics to the filename when quoted. This should fix the majority of %magic commands that take file names. There are probably some others that do not use get_py_filename().

This does not fix completion yet.

Fixes #773, I think. I haven't spun up a Windows VM to test it there on a full system, but the unit tests have been written to test the functionality on all platforms.

Evan Patterson
Collaborator

If this is merged, then we can also get rid of this old hack in IPythonWidget:

https://github.com/ipython/ipython/blob/master/IPython/frontend/qt/console/ipython_widget.py#L252

Jonathan March
Collaborator

Works as desired in Windows XP for %run and %cd
(also still works correctly in OSX and Ubuntu)

Unsurprisingly, in Windows:
%save is still broken (not high priority IMO)
Tab completion is still broken for space-containing paths.

Robert Kern
Collaborator

That should have fixed %save.

Jonathan March
Collaborator

Yes, %save now works in Windows XP.

Jonathan March
Collaborator

Also works on Windows 7.
All relevant (previously passing) tests pass in Windows.

Jonathan March jdmarch merged commit 6ecc466 into from August 29, 2011
Jonathan March jdmarch closed this August 29, 2011
Jonathan March
Collaborator

@epatters - I don't think the hack in IPythonWidget can be removed until problems of handling backslashes in Windows filenames is better resolved.

Brian E. Granger ellisonbg referenced this pull request from a commit January 10, 2012
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Aug 26, 2011
Robert Kern BUG: Allow %magic argument filenames with spaces to be specified with…
… quotes under win32.
de1bf8a
Aug 28, 2011
Robert Kern BUG: break out the filename-unquoting from get_py_filename to be used…
… in other contexts. Fix %save, in this respect.
e3bec84
This page is out of date. Refresh to see the latest.
16  IPython/core/magic.py
@@ -53,7 +53,7 @@
53 53
 from IPython.lib.pylabtools import mpl_runner
54 54
 from IPython.testing.skipdoctest import skip_doctest
55 55
 from IPython.utils.io import file_read, nlprint
56  
-from IPython.utils.path import get_py_filename
  56
+from IPython.utils.path import get_py_filename, unquote_filename
57 57
 from IPython.utils.process import arg_split, abbrev_cwd
58 58
 from IPython.utils.terminal import set_term_title
59 59
 from IPython.utils.text import LSString, SList, format_screen
@@ -1426,10 +1426,12 @@ def magic_prun(self, parameter_s ='',user_mode=1,
1426 1426
         dump_file = opts.D[0]
1427 1427
         text_file = opts.T[0]
1428 1428
         if dump_file:
  1429
+            dump_file = unquote_filename(dump_file)
1429 1430
             prof.dump_stats(dump_file)
1430 1431
             print '\n*** Profile stats marshalled to file',\
1431 1432
                   `dump_file`+'.',sys_exit
1432 1433
         if text_file:
  1434
+            text_file = unquote_filename(text_file)
1433 1435
             pfile = file(text_file,'w')
1434 1436
             pfile.write(output)
1435 1437
             pfile.close()
@@ -2071,7 +2073,7 @@ def magic_save(self,parameter_s = ''):
2071 2073
         it asks for confirmation before overwriting existing files."""
2072 2074
 
2073 2075
         opts,args = self.parse_options(parameter_s,'r',mode='list')
2074  
-        fname, codefrom = args[0], " ".join(args[1:])
  2076
+        fname, codefrom = unquote_filename(args[0]), " ".join(args[1:])
2075 2077
         if not fname.endswith('.py'):
2076 2078
             fname += '.py'
2077 2079
         if os.path.isfile(fname):
@@ -2111,6 +2113,7 @@ def magic_loadpy(self, arg_s):
2111 2113
         %loadpy myscript.py
2112 2114
         %loadpy http://www.example.com/myscript.py
2113 2115
         """
  2116
+        arg_s = unquote_filename(arg_s)
2114 2117
         if not arg_s.endswith('.py'):
2115 2118
             raise ValueError('%%load only works with .py files: %s' % arg_s)
2116 2119
         if arg_s.startswith('http'):
@@ -2127,12 +2130,13 @@ def _find_edit_target(self, args, opts, last_call):
2127 2130
         
2128 2131
         def make_filename(arg):
2129 2132
             "Make a filename from the given args"
  2133
+            arg = unquote_filename(arg)
2130 2134
             try:
2131 2135
                 filename = get_py_filename(arg)
2132 2136
             except IOError:
2133 2137
                 # If it ends with .py but doesn't already exist, assume we want
2134 2138
                 # a new file. 
2135  
-                if args.endswith('.py'):
  2139
+                if arg.endswith('.py'):
2136 2140
                     filename = arg
2137 2141
                 else:
2138 2142
                     filename = None
@@ -2826,8 +2830,7 @@ def magic_cd(self, parameter_s=''):
2826 2830
                               "Use '%%bookmark -l' to see your bookmarks." % ps)
2827 2831
 
2828 2832
         # strip extra quotes on Windows, because os.chdir doesn't like them
2829  
-        if sys.platform == 'win32':
2830  
-            ps = ps.strip('\'"')
  2833
+        ps = unquote_filename(ps)
2831 2834
         # at this point ps should point to the target dir
2832 2835
         if ps:
2833 2836
             try:
@@ -2870,7 +2873,7 @@ def magic_pushd(self, parameter_s=''):
2870 2873
         """
2871 2874
         
2872 2875
         dir_s = self.shell.dir_stack
2873  
-        tgt = os.path.expanduser(parameter_s)
  2876
+        tgt = os.path.expanduser(unquote_filename(parameter_s))
2874 2877
         cwd = os.getcwdu().replace(self.home_dir,'~')
2875 2878
         if tgt:
2876 2879
             self.magic_cd(parameter_s)
@@ -3531,6 +3534,7 @@ def magic_notebook(self, s):
3531 3534
         args = magic_arguments.parse_argstring(self.magic_notebook, s)
3532 3535
 
3533 3536
         from IPython.nbformat import current
  3537
+        args.filename = unquote_filename(args.filename)
3534 3538
         if args.export:
3535 3539
             fname, name, format = current.parse_filename(args.filename)
3536 3540
             cells = []
6  IPython/testing/globalipapp.py
@@ -63,14 +63,14 @@ class py_file_finder(object):
63 63
     def __init__(self,test_filename):
64 64
         self.test_filename = test_filename
65 65
         
66  
-    def __call__(self,name):
  66
+    def __call__(self,name,win32=False):
67 67
         from IPython.utils.path import get_py_filename
68 68
         try:
69  
-            return get_py_filename(name)
  69
+            return get_py_filename(name,win32=win32)
70 70
         except IOError:
71 71
             test_dir = os.path.dirname(self.test_filename)
72 72
             new_path = os.path.join(test_dir,name)
73  
-            return get_py_filename(new_path)
  73
+            return get_py_filename(new_path,win32=win32)
74 74
     
75 75
 
76 76
 def _run_ns_sync(self,arg_s,runner=None):
14  IPython/testing/tools.py
@@ -330,4 +330,16 @@ def mute_warn():
330 330
     try:
331 331
         yield
332 332
     finally:
333  
-        warn.warn = save_warn
  333
+        warn.warn = save_warn
  334
+
  335
+@contextmanager
  336
+def make_tempfile(name):
  337
+    """ Create an empty, named, temporary file for the duration of the context.
  338
+    """
  339
+    f = open(name, 'w')
  340
+    f.close()
  341
+    try:
  342
+        yield
  343
+    finally:
  344
+        os.unlink(name)
  345
+
23  IPython/utils/path.py
@@ -82,13 +82,32 @@ def get_long_path_name(path):
82 82
     return _get_long_path_name(path)
83 83
 
84 84
 
85  
-def get_py_filename(name):
  85
+def unquote_filename(name, win32=(sys.platform=='win32')):
  86
+    """ On Windows, remove leading and trailing quotes from filenames.
  87
+    """
  88
+    if win32:
  89
+        if name.startswith(("'", '"')) and name.endswith(("'", '"')):
  90
+            name = name[1:-1]
  91
+    return name
  92
+
  93
+
  94
+def get_py_filename(name, force_win32=None):
86 95
     """Return a valid python filename in the current directory.
87 96
 
88 97
     If the given name is not a file, it adds '.py' and searches again.
89  
-    Raises IOError with an informative message if the file isn't found."""
  98
+    Raises IOError with an informative message if the file isn't found.
  99
+    
  100
+    On Windows, apply Windows semantics to the filename. In particular, remove
  101
+    any quoting that has been applied to it. This option can be forced for
  102
+    testing purposes.
  103
+    """
90 104
 
91 105
     name = os.path.expanduser(name)
  106
+    if force_win32 is None:
  107
+        win32 = (sys.platform == 'win32')
  108
+    else:
  109
+        win32 = force_win32
  110
+    name = unquote_filename(name, win32=win32)
92 111
     if not os.path.isfile(name) and not name.endswith('.py'):
93 112
         name += '.py'
94 113
     if os.path.isfile(name):
45  IPython/utils/tests/test_path.py
@@ -12,6 +12,8 @@
12 12
 # Imports
13 13
 #-----------------------------------------------------------------------------
14 14
 
  15
+from __future__ import with_statement
  16
+
15 17
 import os
16 18
 import shutil
17 19
 import sys
@@ -27,6 +29,7 @@
27 29
 import IPython
28 30
 from IPython.testing import decorators as dec
29 31
 from IPython.testing.decorators import skip_if_not_win32, skip_win32
  32
+from IPython.testing.tools import make_tempfile
30 33
 from IPython.utils import path, io
31 34
 
32 35
 # Platform-dependent imports
@@ -83,7 +86,7 @@ def setup_environment():
83 86
     each testfunction needs a pristine environment.
84 87
     """
85 88
     global oldstuff, platformstuff
86  
-    oldstuff = (env.copy(), os.name, path.get_home_dir, IPython.__file__)
  89
+    oldstuff = (env.copy(), os.name, path.get_home_dir, IPython.__file__, os.getcwd())
87 90
 
88 91
     if os.name == 'nt':
89 92
         platformstuff = (wreg.OpenKey, wreg.QueryValueEx,)
@@ -92,7 +95,8 @@ def setup_environment():
92 95
 def teardown_environment():
93 96
     """Restore things that were remebered by the setup_environment function
94 97
     """
95  
-    (oldenv, os.name, path.get_home_dir, IPython.__file__,) = oldstuff
  98
+    (oldenv, os.name, path.get_home_dir, IPython.__file__, old_wd) = oldstuff
  99
+    os.chdir(old_wd)
96 100
     reload(path)
97 101
     
98 102
     for key in env.keys():
@@ -401,4 +405,39 @@ def test_not_writable_ipdir():
401 405
     io.stderr = stderr
402 406
     nt.assert_true('WARNING' in pipe.getvalue())
403 407
     env.pop('IPYTHON_DIR', None)
404  
-    
  408
+
  409
+def test_unquote_filename():
  410
+    for win32 in (True, False):
  411
+        nt.assert_equals(path.unquote_filename('foo.py', win32=win32), 'foo.py')
  412
+        nt.assert_equals(path.unquote_filename('foo bar.py', win32=win32), 'foo bar.py')
  413
+    nt.assert_equals(path.unquote_filename('"foo.py"', win32=True), 'foo.py')
  414
+    nt.assert_equals(path.unquote_filename('"foo bar.py"', win32=True), 'foo bar.py')
  415
+    nt.assert_equals(path.unquote_filename("'foo.py'", win32=True), 'foo.py')
  416
+    nt.assert_equals(path.unquote_filename("'foo bar.py'", win32=True), 'foo bar.py')
  417
+    nt.assert_equals(path.unquote_filename('"foo.py"', win32=False), '"foo.py"')
  418
+    nt.assert_equals(path.unquote_filename('"foo bar.py"', win32=False), '"foo bar.py"')
  419
+    nt.assert_equals(path.unquote_filename("'foo.py'", win32=False), "'foo.py'")
  420
+    nt.assert_equals(path.unquote_filename("'foo bar.py'", win32=False), "'foo bar.py'")
  421
+
  422
+@with_environment
  423
+def test_get_py_filename():
  424
+    os.chdir(TMP_TEST_DIR)
  425
+    for win32 in (True, False):
  426
+        with make_tempfile('foo.py'):
  427
+            nt.assert_equals(path.get_py_filename('foo.py', force_win32=win32), 'foo.py')
  428
+            nt.assert_equals(path.get_py_filename('foo', force_win32=win32), 'foo.py')
  429
+        with make_tempfile('foo'):
  430
+            nt.assert_equals(path.get_py_filename('foo', force_win32=win32), 'foo')
  431
+            nt.assert_raises(IOError, path.get_py_filename, 'foo.py', force_win32=win32)
  432
+        nt.assert_raises(IOError, path.get_py_filename, 'foo', force_win32=win32)
  433
+        nt.assert_raises(IOError, path.get_py_filename, 'foo.py', force_win32=win32)
  434
+        true_fn = 'foo with spaces.py'
  435
+        with make_tempfile(true_fn):
  436
+            nt.assert_equals(path.get_py_filename('foo with spaces', force_win32=win32), true_fn)
  437
+            nt.assert_equals(path.get_py_filename('foo with spaces.py', force_win32=win32), true_fn)
  438
+            if win32:
  439
+                nt.assert_equals(path.get_py_filename('"foo with spaces.py"', force_win32=True), true_fn)
  440
+                nt.assert_equals(path.get_py_filename("'foo with spaces.py'", force_win32=True), true_fn)
  441
+            else:
  442
+                nt.assert_raises(IOError, path.get_py_filename, '"foo with spaces.py"', force_win32=False)
  443
+                nt.assert_raises(IOError, path.get_py_filename, "'foo with spaces.py'", force_win32=False)
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.