Merge branch 'path_expand' of https://github.com/VHarisop/xonsh into VHarisop-path_expand

news/path_expand.rst

@@ -0,0 +1,25 @@
+**Added:**
+
+* A new class, ``xonsh.tools.EnvPath`` has been added. This class implements a
+  ``MutableSequence`` object and overrides the ``__getitem__`` method so that
+  when its entries are requested (either explicitly or implicitly), variable
+  and user expansion is performed, and relative paths are resolved.
+  ``EnvPath`` accepts objects (or lists of objects) of ``str``, ``bytes`` or
+  ``pathlib.Path`` types.
+
+**Changed:**
+
+* All ``PATH``-like environment variables are now stored in an ``EnvPath``
+  object, so that non-absolute paths or paths containing environment variables
+  can be resolved properly.
+
+**Deprecated:** None
+
+**Removed:** None
+
+**Fixed:** 
+
+* Issue where ``xonsh`` did not expand user and environment variables in
+  ``$PATH``, forcing the user to add absolute paths.
+
+**Security:** None

tests/test_environ.py

@@ -22,23 +22,42 @@ def test_env_normal():
     assert_equal('wakka', env['VAR'])
 
 def test_env_path_list():
+    env = Env(MYPATH=['/home/wakka'])
+    assert_equal(['/home/wakka'], env['MYPATH'].paths)
     env = Env(MYPATH=['wakka'])
-    assert_equal(['wakka'], env['MYPATH'])
+    assert_equal([os.path.abspath('wakka')], env['MYPATH'].paths)
 
 def test_env_path_str():
+    env = Env(MYPATH='/home/wakka' + os.pathsep + '/home/jawaka')
+    assert_equal(['/home/wakka', '/home/jawaka'], env['MYPATH'].paths)
     env = Env(MYPATH='wakka' + os.pathsep + 'jawaka')
-    assert_equal(['wakka', 'jawaka'], env['MYPATH'])
+    assert_equal([os.path.abspath('wakka'), os.path.abspath('jawaka')],
+                 env['MYPATH'].paths)
 
 def test_env_detype():
     env = Env(MYPATH=['wakka', 'jawaka'])
-    assert_equal('wakka' + os.pathsep + 'jawaka', env.detype()['MYPATH'])
+    assert_equal(os.path.abspath('wakka') + os.pathsep + \
+                 os.path.abspath('jawaka'),
+                 env.detype()['MYPATH'])
 
 def test_env_detype_mutable_access_clear():
+    env = Env(MYPATH=['/home/wakka', '/home/jawaka'])
+    assert_equal('/home/wakka' + os.pathsep + '/home/jawaka',
+                 env.detype()['MYPATH'])
+    env['MYPATH'][0] = '/home/woah'
+    assert_equal(None, env._detyped)
+    assert_equal('/home/woah' + os.pathsep + '/home/jawaka',
+                 env.detype()['MYPATH'])
+
     env = Env(MYPATH=['wakka', 'jawaka'])
-    assert_equal('wakka' + os.pathsep + 'jawaka', env.detype()['MYPATH'])
+    assert_equal(os.path.abspath('wakka') + os.pathsep + \
+                 os.path.abspath('jawaka'),
+                 env.detype()['MYPATH'])
     env['MYPATH'][0] = 'woah'
     assert_equal(None, env._detyped)
-    assert_equal('woah' + os.pathsep + 'jawaka', env.detype()['MYPATH'])
+    assert_equal(os.path.abspath('woah') + os.pathsep + \
+                 os.path.abspath('jawaka'),
+                 env.detype()['MYPATH'])
 
 def test_env_detype_no_dict():
     env = Env(YO={'hey': 42})

tests/test_tools.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 """Tests the xonsh lexer."""
 import os
+import pathlib
 from tempfile import TemporaryDirectory
 import stat
 
@@ -18,7 +19,7 @@ from xonsh.tools import (
     is_string_or_callable, check_for_partial_string, CommandsCache,
     is_dynamic_cwd_width, to_dynamic_cwd_tuple, dynamic_cwd_tuple_to_str,
     is_logfile_opt, to_logfile_opt, logfile_opt_to_str, argvquote,
-    executables_in, find_next_break, expand_case_matching)
+    executables_in, find_next_break, expand_case_matching, EnvPath)
 
 LEXER = Lexer()
 LEXER.build()
@@ -369,7 +370,10 @@ def test_ensure_string():
 def test_is_env_path():
     cases = [
         ('/home/wakka', False),
-        (['/home/jawaka'], True),
+        (['/home/jawaka'], False),
+        (EnvPath(['/home/jawaka']), True),
+        (EnvPath(['jawaka']), True),
+        (EnvPath(b'jawaka:wakka'), True),
         ]
     for inp, exp in cases:
         obs = is_env_path(inp)
@@ -381,10 +385,11 @@ def test_str_to_env_path():
         ('/home/wakka', ['/home/wakka']),
         ('/home/wakka' + os.pathsep + '/home/jawaka',
          ['/home/wakka', '/home/jawaka']),
+        (b'/home/wakka', ['/home/wakka']),
         ]
     for inp, exp in cases:
         obs = str_to_env_path(inp)
-        yield assert_equal, exp, obs
+        yield assert_equal, exp, obs.paths
 
 
 def test_env_path_to_str():
@@ -398,6 +403,54 @@ def test_env_path_to_str():
         yield assert_equal, exp, obs
 
 
+def test_env_path():
+    """
+    Test the EnvPath class.
+    """
+    # lambda to expand the expected paths
+    expand = lambda path: \
+            os.path.realpath(os.path.expanduser(os.path.expandvars(path)))
+    getitem_cases = [
+        ('xonsh_dir', 'xonsh_dir'),
+        ('.', '.'),
+        ('../', '../'),
+        ('~/', '~/'),
+        (b'~/../', '~/../'),
+    ]
+    for inp, exp in getitem_cases:
+        obs = EnvPath(inp)[0] # call to __getitem__
+        yield assert_equal, expand(exp), obs
+
+    # cases that involve path-separated strings
+    multipath_cases = [
+        (os.pathsep.join(['xonsh_dir', '../', './', '~/']),
+         ['xonsh_dir', '../', '.', '~/']),
+        ('/home/wakka' + os.pathsep + '/home/jakka' + os.pathsep + '~/',
+         ['/home/wakka', '/home/jakka', '~/'])
+    ]
+    for inp, exp in multipath_cases:
+        obs = [i for i in EnvPath(inp)]
+        yield assert_equal, [expand(i) for i in exp], obs
+
+    # cases that involve pathlib.Path objects
+    pathlib_cases = [
+        (pathlib.Path('/home/wakka'), ['/home/wakka']),
+        (pathlib.Path('~/'), ['~/']),
+        (pathlib.Path('.'), ['.']),
+        (['/home/wakka', pathlib.Path('/home/jakka'), '~/'],
+         ['/home/wakka', '/home/jakka', '~/']),
+        (['/home/wakka', pathlib.Path('../'), '../'],
+         ['/home/wakka', '../', '../']),
+        (['/home/wakka', pathlib.Path('~/'), '~/'],
+         ['/home/wakka', '~/', '~/']),
+    ]
+
+    for inp, exp in pathlib_cases:
+        # iterate over EnvPath to acquire all expanded paths
+        obs = [i for i in EnvPath(inp)]
+        yield assert_equal, [expand(i) for i in exp], obs
+
+
 def test_is_bool():
     yield assert_equal, True, is_bool(True)
     yield assert_equal, True, is_bool(False)

xonsh/tools.py

@@ -25,6 +25,7 @@ import glob
 import string
 import ctypes
 import builtins
+import pathlib
 import warnings
 import functools
 import threading
@@ -100,6 +101,91 @@ class XonshCalledProcessError(XonshError, CalledProcessError):
         self.completed_command = completed_command
 
 
+def expandpath(path):
+    """
+    Performs environment variable / user expansion on a given path
+    if the relevant flag has been set.
+    """
+    env = getattr(builtins, '__xonsh_env__', os.environ)
+    if env.get('EXPAND_ENV_VARS', False):
+        # expand variables and use os.path.abspath to handle cases
+        # with relative paths like ../ or ./
+        path = os.path.expanduser(expandvars(path))
+    return os.path.abspath(path)
+
+
+def decode_bytes(path):
+    """
+    Tries to decode a path in bytes using XONSH_ENCODING if available,
+    otherwise using sys.getdefaultencoding().
+    """
+    env = getattr(builtins, '__xonsh_env__', os.environ)
+    enc = env.get('XONSH_ENCODING', DEFAULT_ENCODING)
+    return path.decode(encoding=enc,
+                       errors=env.get('XONSH_ENCODING_ERRORS'))
+
+
+class EnvPath(collections.MutableSequence):
+    """
+    A class that implements an environment path, which is a list of
+    strings. Provides a custom method that expands all paths if the
+    relevant env variable has been set.
+    """
+    def __init__(self, args=None):
+        if not args:
+            self._l = []
+        else:
+            if isinstance(args, str):
+                self._l = args.split(os.pathsep)
+            elif isinstance(args, pathlib.Path):
+                self._l = [args]
+            elif isinstance(args, bytes):
+                # decode bytes to a string and then split based on
+                # the default path separator
+                self._l = decode_bytes(args).split(os.pathsep)
+            elif isinstance(args, collections.Iterable):
+                # put everything in a list -before- performing the type check
+                # in order to be able to retrieve it later, for cases such as
+                # when a generator expression was passed as an argument
+                args = list(args)
+                if not all(isinstance(i, (str, bytes, pathlib.Path)) \
+                                      for i in args):
+                    # make TypeError's message as informative as possible
+                    # when given an invalid initialization sequence
+                    raise TypeError(
+                            "EnvPath's initialization sequence should only "
+                            "contain str, bytes and pathlib.Path entries")
+                self._l = args
+            else:
+                raise TypeError('EnvPath cannot be initialized with items '
+                                'of type %s' % type(args))
+
+    def __getitem__(self, item):
+        return expandpath(self._l[item])
+
+    def __setitem__(self, index, item):
+        self._l.__setitem__(index, item)
+
+    def __len__(self):
+        return len(self._l)
+
+    def __delitem__(self, key):
+        self._l.__delitem__(key)
+
+    def insert(self, index, value):
+        self._l.insert(index, value)
+
+    @property
+    def paths(self):
+        """
+        Returns the list of directories that this EnvPath contains.
+        """
+        return list(self)
+
+    def __repr__(self):
+        return repr(self._l)
+
+
 class DefaultNotGivenType(object):
     """Singleton for representing when no default value is given."""
 
@@ -695,18 +781,15 @@ def ensure_string(x):
 
 def is_env_path(x):
     """This tests if something is an environment path, ie a list of strings."""
-    if isinstance(x, str):
-        return False
-    else:
-        return (isinstance(x, abc.Sequence) and
-                all(isinstance(a, str) for a in x))
+    return isinstance(x, EnvPath)
 
 
 def str_to_env_path(x):
     """Converts a string to an environment path, ie a list of strings,
     splitting on the OS separator.
     """
-    return x.split(os.pathsep)
+    # splitting will be done implicitly in EnvPath's __init__
+    return EnvPath(x)
 
 
 def env_path_to_str(x):
@@ -1247,6 +1330,9 @@ def expandvars(path):
     if isinstance(path, bytes):
         path = path.decode(encoding=ENV.get('XONSH_ENCODING'),
                            errors=ENV.get('XONSH_ENCODING_ERRORS'))
+    elif isinstance(path, pathlib.Path):
+        # get the path's string representation
+        path = str(path)
     if '$' not in path and (not ON_WINDOWS or '%' not in path):
         return path
     varchars = string.ascii_letters + string.digits + '_-'