From 4308336623cfd8da36474581dae6bcabe4e3abe7 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Wed, 9 Mar 2016 14:58:22 -0500
Subject: [PATCH 1/4] make context.exit raise a custom exception

This exception is caught in Command.main where we turn this into either
an exception or a system exit. This allows context.exit to work when
standalone_mode=False.
---
 click/core.py         | 10 ++++++++--
 click/exceptions.py   | 39 ++++++++++++++++++++++++++++++++-------
 tests/test_context.py | 26 +++++++++++++++++++++++++-
 3 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/click/core.py b/click/core.py
index 525b09111..c8e358443 100644
--- a/click/core.py
+++ b/click/core.py
@@ -9,7 +9,7 @@
 from .types import convert_type, IntRange, BOOL
 from .utils import make_str, make_default_short_help, echo, get_os_args
 from .exceptions import ClickException, UsageError, BadParameter, Abort, \
-     MissingParameter
+     MissingParameter, Exit
 from .termui import prompt, confirm, style
 from .formatting import HelpFormatter, join_options
 from .parser import OptionParser, split_opt
@@ -498,7 +498,7 @@ def abort(self):
 
     def exit(self, code=0):
         """Exits the application with a given exit code."""
-        sys.exit(code)
+        raise Exit(code, self)
 
     def get_usage(self):
         """Helper method to get formatted usage string for the current
@@ -718,6 +718,12 @@ def main(self, args=None, prog_name=None, complete_var=None,
             except (EOFError, KeyboardInterrupt):
                 echo(file=sys.stderr)
                 raise Abort()
+            except Exit as e:
+                if not standalone_mode:
+                    if e.exit_code:
+                        raise
+                    return None
+                sys.exit(e.exit_code)
             except ClickException as e:
                 if not standalone_mode:
                     raise
diff --git a/click/exceptions.py b/click/exceptions.py
index b5641343d..b49aa6e3c 100644
--- a/click/exceptions.py
+++ b/click/exceptions.py
@@ -13,6 +13,7 @@ class ClickException(Exception):
 
     #: The exit code for this exception
     exit_code = 1
+    show_prefix = 'Error: '
 
     def __init__(self, message):
         ctor_msg = message
@@ -37,19 +38,17 @@ def __str__(self):
     def show(self, file=None):
         if file is None:
             file = get_text_stderr()
-        echo('Error: %s' % self.format_message(), file=file)
+        echo('%s%s' % (self.show_prefix, self.format_message()), file=file)
 
 
-class UsageError(ClickException):
-    """An internal exception that signals a usage error.  This typically
-    aborts any further handling.
+class ContextAwareException(ClickException):
+    """An exception that knows how to use a :class:`~click.Context` object
+    to properly display itself.
 
     :param message: the error message to display.
     :param ctx: optionally the context that caused this error.  Click will
                 fill in the context automatically in some situations.
     """
-    exit_code = 2
-
     def __init__(self, message, ctx=None):
         ClickException.__init__(self, message)
         self.ctx = ctx
@@ -67,7 +66,33 @@ def show(self, file=None):
         if self.ctx is not None:
             color = self.ctx.color
             echo(self.ctx.get_usage() + '\n%s' % hint, file=file, color=color)
-        echo('Error: %s' % self.format_message(), file=file, color=color)
+        echo('%s%s' % (self.show_prefix, self.format_message()), file=file, color=color)
+
+
+class Exit(ContextAwareException):
+    """An exception that indicates that the application should exit with some
+    status code.
+
+    :param code: the status code to exit with.
+    :param ctx: optionally the context that caused this error.  Click will
+                fill in the context automatically in some situations.
+    """
+    show_prefix = 'Exit: '
+
+    def __init__(self, code, ctx=None):
+        ContextAwareException.__init__(self, str(code), ctx=None)
+        self.exit_code = code
+
+
+class UsageError(ContextAwareException):
+    """An internal exception that signals a usage error.  This typically
+    aborts any further handling.
+
+    :param message: the error message to display.
+    :param ctx: optionally the context that caused this error.  Click will
+                fill in the context automatically in some situations.
+    """
+    exit_code = 2
 
 
 class BadParameter(UsageError):
diff --git a/tests/test_context.py b/tests/test_context.py
index 3b07cc80b..f0dfedb19 100644
--- a/tests/test_context.py
+++ b/tests/test_context.py
@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+import pytest
+
 import click
 
 
@@ -204,7 +206,6 @@ def foo():
     assert result.output == 'aha!\n'
     assert called == [True]
 
-
 def test_make_pass_decorator_args(runner):
     """
     Test to check that make_pass_decorator doesn't consume arguments based on
@@ -239,3 +240,26 @@ def test2(ctx, foo):
     result = runner.invoke(cli, ['test2'])
     assert not result.exception
     assert result.output == 'foocmd\n'
+
+def test_exit_not_standalone_failure():
+    @click.command()
+    @click.pass_context
+    def cli(ctx):
+        ctx.exit(1)
+
+    with pytest.raises(click.exceptions.Exit) as e:
+        cli.main([], 'test_exit_not_standalone', standalone_mode=False)
+    assert e.value.exit_code == 1
+
+
+def test_exit_not_standalone_success():
+    @click.command()
+    @click.pass_context
+    def cli(ctx):
+        ctx.exit(0)
+
+    assert cli.main(
+        [],
+        'test_exit_not_standalone',
+        standalone_mode=False,
+    ) is None

From 225736b9eaa5cea30356d3a48e5c2bbe788e6298 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Wed, 9 Mar 2016 14:59:44 -0500
Subject: [PATCH 2/4] add tests for the runner when the app uses context.exit

---
 tests/test_context.py |  2 ++
 tests/test_testing.py | 30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/tests/test_context.py b/tests/test_context.py
index f0dfedb19..cad2ae325 100644
--- a/tests/test_context.py
+++ b/tests/test_context.py
@@ -206,6 +206,7 @@ def foo():
     assert result.output == 'aha!\n'
     assert called == [True]
 
+
 def test_make_pass_decorator_args(runner):
     """
     Test to check that make_pass_decorator doesn't consume arguments based on
@@ -241,6 +242,7 @@ def test2(ctx, foo):
     assert not result.exception
     assert result.output == 'foocmd\n'
 
+
 def test_exit_not_standalone_failure():
     @click.command()
     @click.pass_context
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 12f57de39..b8565adbc 100644
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -153,16 +153,34 @@ def cli_string():
         click.echo('hello world')
         sys.exit('error')
 
+    @click.command()
+    @click.pass_context
+    def cli_string_ctx_exit(ctx):
+        click.echo('hello world')
+        ctx.exit('error')
+
     @click.command()
     def cli_int():
         click.echo('hello world')
         sys.exit(1)
 
+    @click.command()
+    @click.pass_context
+    def cli_int_ctx_exit(ctx):
+        click.echo('hello world')
+        ctx.exit(1)
+
     @click.command()
     def cli_float():
         click.echo('hello world')
         sys.exit(1.0)
 
+    @click.command()
+    @click.pass_context
+    def cli_float_ctx_exit(ctx):
+        click.echo('hello world')
+        ctx.exit(1.0)
+
     @click.command()
     def cli_no_error():
         click.echo('hello world')
@@ -173,14 +191,26 @@ def cli_no_error():
     assert result.exit_code == 1
     assert result.output == 'hello world\nerror\n'
 
+    result = runner.invoke(cli_string_ctx_exit)
+    assert result.exit_code == 1
+    assert result.output == 'hello world\nerror\n'
+
     result = runner.invoke(cli_int)
     assert result.exit_code == 1
     assert result.output == 'hello world\n'
 
+    result = runner.invoke(cli_int_ctx_exit)
+    assert result.exit_code == 1
+    assert result.output == 'hello world\n'
+
     result = runner.invoke(cli_float)
     assert result.exit_code == 1
     assert result.output == 'hello world\n1.0\n'
 
+    result = runner.invoke(cli_float_ctx_exit)
+    assert result.exit_code == 1
+    assert result.output == 'hello world\n1.0\n'
+
     result = runner.invoke(cli_no_error)
     assert result.exit_code == 0
     assert result.output == 'hello world\n'

From 1602436cd1bc65841ac0ac3b2ebe9ff94e7d1941 Mon Sep 17 00:00:00 2001
From: Stephen Rosen <sirosen@uchicago.edu>
Date: Sun, 26 Aug 2018 17:41:23 -0400
Subject: [PATCH 3/4] Simplify Exit Exception

= Refactor BaseCommand to simplify Exit handling

BaseCommand.main() will get the return value from `self.invoke` and
pass it to `ctx.exit()`, so that commands which `return 1` will
`ctx.exit(1)`
This is not a behavioral change -- `ctx.exit()` already does this.

Do not reraise Exit exceptions when invoked non-standalone. Instead,
return their exit codes as the result of `BaseCommand.main`. So, a
command which explicitly invokes `ctx.exit(0)` in its execution will
effectively `return 0` instead of raising a specialized exception.

= Make Exit its own RuntimeError subtype, don't pollute stdio on exit

Exit exceptions should not be a subtype of ClickException with the
output pretty-printing (`show()`) which happens when they are raised in
non-standalone mode. That would make `ctx.exit(...)` needlessly pollute
stderr. This would have downstream impact on everyone using context exit
calls, and generally be a Bad Thing (tm).

Instead, like Abort, Exit is a subclass of RuntimeError with very little
"meat on its bones". It's an exception containing a single integer (exit
code)  which is then interpreted in that way in BaseCommand.main

closes #533
closes #667
---
 CHANGES.rst           |  2 ++
 click/core.py         | 17 ++++++++-------
 click/exceptions.py   | 49 +++++++++++++++----------------------------
 tests/test_context.py | 17 ++++-----------
 4 files changed, 32 insertions(+), 53 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index a41bfc618..da2c576f1 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -8,6 +8,8 @@ Version 7.0
 
 (upcoming release with new features, release date to be decided)
 
+- Non-standalone calls to Context.exit return the exit code, rather than
+  calling ``sys.exit`` (`#667`_)(`#533`_)
 - Updated test env matrix. (`#1027`_)
 - Fixes a ZeroDivisionError in ProgressBar.make_step,
   when the arg passed to the first call of ProgressBar.update is 0. (`#1012`_)(`#447`_)
diff --git a/click/core.py b/click/core.py
index c8e358443..2fa6e18e9 100644
--- a/click/core.py
+++ b/click/core.py
@@ -498,7 +498,7 @@ def abort(self):
 
     def exit(self, code=0):
         """Exits the application with a given exit code."""
-        raise Exit(code, self)
+        raise Exit(code)
 
     def get_usage(self):
         """Helper method to get formatted usage string for the current
@@ -714,16 +714,12 @@ def main(self, args=None, prog_name=None, complete_var=None,
                     rv = self.invoke(ctx)
                     if not standalone_mode:
                         return rv
-                    ctx.exit()
+                    if not rv:
+                        rv = 0
+                    ctx.exit(rv)
             except (EOFError, KeyboardInterrupt):
                 echo(file=sys.stderr)
                 raise Abort()
-            except Exit as e:
-                if not standalone_mode:
-                    if e.exit_code:
-                        raise
-                    return None
-                sys.exit(e.exit_code)
             except ClickException as e:
                 if not standalone_mode:
                     raise
@@ -734,6 +730,11 @@ def main(self, args=None, prog_name=None, complete_var=None,
                     sys.exit(1)
                 else:
                     raise
+        except Exit as e:
+            if standalone_mode:
+                sys.exit(e.exit_code)
+            else:
+                return e.exit_code
         except Abort:
             if not standalone_mode:
                 raise
diff --git a/click/exceptions.py b/click/exceptions.py
index b49aa6e3c..d8a9275cc 100644
--- a/click/exceptions.py
+++ b/click/exceptions.py
@@ -13,7 +13,6 @@ class ClickException(Exception):
 
     #: The exit code for this exception
     exit_code = 1
-    show_prefix = 'Error: '
 
     def __init__(self, message):
         ctor_msg = message
@@ -38,17 +37,19 @@ def __str__(self):
     def show(self, file=None):
         if file is None:
             file = get_text_stderr()
-        echo('%s%s' % (self.show_prefix, self.format_message()), file=file)
+        echo('Error: %s' % self.format_message(), file=file)
 
 
-class ContextAwareException(ClickException):
-    """An exception that knows how to use a :class:`~click.Context` object
-    to properly display itself.
+class UsageError(ClickException):
+    """An internal exception that signals a usage error.  This typically
+    aborts any further handling.
 
     :param message: the error message to display.
     :param ctx: optionally the context that caused this error.  Click will
                 fill in the context automatically in some situations.
     """
+    exit_code = 2
+
     def __init__(self, message, ctx=None):
         ClickException.__init__(self, message)
         self.ctx = ctx
@@ -66,33 +67,7 @@ def show(self, file=None):
         if self.ctx is not None:
             color = self.ctx.color
             echo(self.ctx.get_usage() + '\n%s' % hint, file=file, color=color)
-        echo('%s%s' % (self.show_prefix, self.format_message()), file=file, color=color)
-
-
-class Exit(ContextAwareException):
-    """An exception that indicates that the application should exit with some
-    status code.
-
-    :param code: the status code to exit with.
-    :param ctx: optionally the context that caused this error.  Click will
-                fill in the context automatically in some situations.
-    """
-    show_prefix = 'Exit: '
-
-    def __init__(self, code, ctx=None):
-        ContextAwareException.__init__(self, str(code), ctx=None)
-        self.exit_code = code
-
-
-class UsageError(ContextAwareException):
-    """An internal exception that signals a usage error.  This typically
-    aborts any further handling.
-
-    :param message: the error message to display.
-    :param ctx: optionally the context that caused this error.  Click will
-                fill in the context automatically in some situations.
-    """
-    exit_code = 2
+        echo('Error: %s' % self.format_message(), file=file, color=color)
 
 
 class BadParameter(UsageError):
@@ -248,3 +223,13 @@ def format_message(self):
 
 class Abort(RuntimeError):
     """An internal signalling exception that signals Click to abort."""
+
+
+class Exit(RuntimeError):
+    """An exception that indicates that the application should exit with some
+    status code.
+
+    :param code: the status code to exit with.
+    """
+    def __init__(self, code):
+        self.exit_code = code
diff --git a/tests/test_context.py b/tests/test_context.py
index cad2ae325..35933beba 100644
--- a/tests/test_context.py
+++ b/tests/test_context.py
@@ -1,6 +1,4 @@
 # -*- coding: utf-8 -*-
-import pytest
-
 import click
 
 
@@ -195,6 +193,7 @@ def test_close_before_pop(runner):
     @click.pass_context
     def cli(ctx):
         ctx.obj = 'test'
+
         @ctx.call_on_close
         def foo():
             assert click.get_current_context().obj == 'test'
@@ -243,25 +242,17 @@ def test2(ctx, foo):
     assert result.output == 'foocmd\n'
 
 
-def test_exit_not_standalone_failure():
+def test_exit_not_standalone():
     @click.command()
     @click.pass_context
     def cli(ctx):
         ctx.exit(1)
 
-    with pytest.raises(click.exceptions.Exit) as e:
-        cli.main([], 'test_exit_not_standalone', standalone_mode=False)
-    assert e.value.exit_code == 1
-
+    assert cli.main([], 'test_exit_not_standalone', standalone_mode=False) == 1
 
-def test_exit_not_standalone_success():
     @click.command()
     @click.pass_context
     def cli(ctx):
         ctx.exit(0)
 
-    assert cli.main(
-        [],
-        'test_exit_not_standalone',
-        standalone_mode=False,
-    ) is None
+    assert cli.main([], 'test_exit_not_standalone', standalone_mode=False) == 0

From a94c0be3b53997b55ce5a0808da4ca3b0a0f44dc Mon Sep 17 00:00:00 2001
From: Stephen Rosen <sirosen@uchicago.edu>
Date: Sun, 26 Aug 2018 19:10:31 -0400
Subject: [PATCH 4/4] Bugfix for Exit handling

`rv = BaseCommand.invoke(ctx); ...; ctx.exit(rv)` seems correct at first
blush, but there are actually important subtleties to consider here. For
example, chained commands will produce a list of outputs, and that can
have the consequence that `rv` is truthy -- but full of falsey values,
possibly meant to indicate success.

Stick with the existing simple `ctx.exit()` call (no args) so that we
don't run into these behaviors. Add comments to `BaseCommand.main`
explaining what's going on and why the logic is a little more nuanced
than you might first guess.
---
 click/core.py       | 19 ++++++++++++++++---
 click/exceptions.py |  2 +-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/click/core.py b/click/core.py
index 2fa6e18e9..f56204c16 100644
--- a/click/core.py
+++ b/click/core.py
@@ -714,9 +714,14 @@ def main(self, args=None, prog_name=None, complete_var=None,
                     rv = self.invoke(ctx)
                     if not standalone_mode:
                         return rv
-                    if not rv:
-                        rv = 0
-                    ctx.exit(rv)
+                    # it's not safe to `ctx.exit(rv)` here!
+                    # note that `rv` may actually contain data like "1" which
+                    # has obvious effects
+                    # more subtle case: `rv=[None, None]` can come out of
+                    # chained commands which all returned `None` -- so it's not
+                    # even always obvious that `rv` indicates success/failure
+                    # by its truthiness/falsiness
+                    ctx.exit()
             except (EOFError, KeyboardInterrupt):
                 echo(file=sys.stderr)
                 raise Abort()
@@ -734,6 +739,14 @@ def main(self, args=None, prog_name=None, complete_var=None,
             if standalone_mode:
                 sys.exit(e.exit_code)
             else:
+                # in non-standalone mode, return the exit code
+                # note that this is only reached if `self.invoke` above raises
+                # an Exit explicitly -- thus bypassing the check there which
+                # would return its result
+                # the results of non-standalone execution may therefore be
+                # somewhat ambiguous: if there are codepaths which lead to
+                # `ctx.exit(1)` and to `return 1`, the caller won't be able to
+                # tell the difference between the two
                 return e.exit_code
         except Abort:
             if not standalone_mode:
diff --git a/click/exceptions.py b/click/exceptions.py
index d8a9275cc..6fa17658c 100644
--- a/click/exceptions.py
+++ b/click/exceptions.py
@@ -231,5 +231,5 @@ class Exit(RuntimeError):
 
     :param code: the status code to exit with.
     """
-    def __init__(self, code):
+    def __init__(self, code=0):
         self.exit_code = code