Repository: amoffat/sh (python)

Issue:
sh.which doesn't return an object and doesn't throw exceptions
Hi,
I experience this unexpected behavior:
Is this a bug, or am I doing something wrong?

Python Version:
```bash 
$ python --version
Python 3.9.5
```
sh Version: 1.14.2

Code snippet:

```python
 try:
      # i expect behavior as described here:
      # https://amoffat.github.io/sh/sections/exit_codes.html
      output = sh.which('xxxxx')
      # output returns a string or None , not an object
      # print(output.exit_code) fails because neither 'str' nor 'None'  has the attribute 'exit_code' 
 except ErrorReturnCode:
      # this exception is not thrown 
      logger.error("program not found")
```

Pull request:
Make sh.which() an internal function
Rename sh.which so normal 'from sh import which' hits the system binary.

Fixes #579 

Patch:
diff --git a/sh.py b/sh.py
index 5fc9c2c..9fd8902 100644
--- a/sh.py
+++ b/sh.py
@@ -534,7 +534,7 @@ def canonicalize(path):
     return os.path.abspath(os.path.expanduser(path))
 
 
-def which(program, paths=None):
+def _which(program, paths=None):
     """ takes a program name or full path, plus an optional collection of search
     paths, and returns the full path of the requested executable.  if paths is
     specified, it is the entire list of search paths, and the PATH env is not
@@ -576,14 +576,14 @@ def which(program, paths=None):
 
 
 def resolve_command_path(program):
-    path = which(program)
+    path = _which(program)
     if not path:
         # our actual command might have a dash in it, but we can't call
         # that from python (we have to use underscores), so we'll check
         # if a dash version of our underscore command exists and use that
         # if it does
         if "_" in program:
-            path = which(program.replace("_", "-"))
+            path = _which(program.replace("_", "-"))
         if not path:
             return None
     return path
@@ -1290,7 +1290,7 @@ class Command(object):
     )
 
     def __init__(self, path, search_paths=None):
-        found = which(path, search_paths)
+        found = _which(path, search_paths)
 
         self._path = encode_to_py3bytes_or_py2str("")
 
@@ -3292,16 +3292,22 @@ class Environment(dict):
         if k.startswith("__") and k.endswith("__"):
             raise AttributeError
 
-        # is it a custom builtin?
-        builtin = getattr(self, "b_" + k, None)
-        if builtin:
-            return builtin
+        if k == 'cd':
+            # Don't resolve the system binary. It's useful in scripts to be
+            # able to switch directories in the current process. Can also be
+            # used as a context manager.
+            return Cd
 
         # is it a command?
         cmd = resolve_command(k, self.baked_args)
         if cmd:
             return cmd
 
+        # is it a custom builtin?
+        builtin = getattr(self, "b_" + k, None)
+        if builtin:
+            return builtin
+
         # how about an environment variable?
         # this check must come after testing if its a command, because on some
         # systems, there are an environment variables that can conflict with
@@ -3315,20 +3321,25 @@ class Environment(dict):
         # nothing found, raise an exception
         raise CommandNotFound(k)
 
-    # methods that begin with "b_" are custom builtins and will override any
-    # program that exists in our path.  this is useful for things like
-    # common shell builtins that people are used to, but which aren't actually
-    # full-fledged system binaries
-    @staticmethod
-    def b_cd(path=None):
-        if path:
-            os.chdir(path)
-        else:
-            os.chdir(os.path.expanduser('~'))
-
+    # Methods that begin with "b_" are implementations of shell built-ins that
+    # people are used to, but which may not have an executable equivalent.
     @staticmethod
     def b_which(program, paths=None):
-        return which(program, paths)
+        return _which(program, paths)
+
+
+class Cd(object):
+    def __new__(cls, path=None):
+        res = super(Cd, cls).__new__(cls)
+        res.old_path = os.getcwd()
+        os.chdir(path or os.path.expanduser('~'))
+        return res
+
+    def __enter__(self):
+        pass
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        os.chdir(self.old_path)
 
 
 class Contrib(ModuleType):  # pragma: no cover


Test patch:
diff --git a/test.py b/test.py
index 41de34d..181290b 100644
--- a/test.py
+++ b/test.py
@@ -243,7 +243,7 @@ class FunctionalTests(BaseTests):
 
     def test_print_command(self):
         from sh import ls, which
-        actual_location = which("ls")
+        actual_location = str(which("ls")).strip()
         out = str(ls)
         self.assertEqual(out, actual_location)
 
@@ -578,12 +578,15 @@ print(dict(HERP=sh.HERP))
         self.assertEqual(out, "{'HERP': 'DERP'}")
 
     def test_which(self):
-        from sh import which, ls
+        # Test 'which' as built-in function
+        from sh import ls
+        which = sh._SelfWrapper__env.b_which
         self.assertEqual(which("fjoawjefojawe"), None)
         self.assertEqual(which("ls"), str(ls))
 
     def test_which_paths(self):
-        from sh import which
+        # Test 'which' as built-in function
+        which = sh._SelfWrapper__env.b_which
         py = create_tmp_test("""
 print("hi")
 """)
@@ -754,7 +757,7 @@ exit(2)
     def test_command_wrapper_equivalence(self):
         from sh import Command, ls, which
 
-        self.assertEqual(Command(which("ls")), ls)
+        self.assertEqual(Command(str(which("ls")).strip()), ls)
 
     def test_doesnt_execute_directories(self):
         save_path = os.environ['PATH']
@@ -959,8 +962,8 @@ print(sys.argv[1])
     def test_command_wrapper(self):
         from sh import Command, which
 
-        ls = Command(which("ls"))
-        wc = Command(which("wc"))
+        ls = Command(str(which("ls")).strip())
+        wc = Command(str(which("wc")).strip())
 
         c1 = int(wc(ls("-A1"), l=True))  # noqa: E741
         c2 = len(os.listdir("."))
@@ -2254,8 +2257,6 @@ p.wait()
 
     def test_pushd_cd(self):
         """ test that pushd works like pushd/popd with built-in cd correctly """
-        import sh
-
         child = realpath(tempfile.mkdtemp())
         try:
             old_wd = os.getcwd()
@@ -2276,6 +2277,12 @@ p.wait()
         self.assertNotEqual(orig, os.getcwd())
         self.assertEqual(my_dir, os.getcwd())
 
+    def test_cd_context_manager(self):
+        orig = os.getcwd()
+        with sh.cd(tempdir):
+            self.assertEqual(tempdir, os.getcwd())
+        self.assertEqual(orig, os.getcwd())
+
     def test_non_existant_cwd(self):
         from sh import ls