Merge branch 'improve-runner-args-visibility'

docs/api/util.rst

@@ -0,0 +1,5 @@
+========
+``util``
+========
+
+.. automodule:: patchwork.util

docs/changelog.rst

@@ -2,6 +2,16 @@
 Changelog
 =========
 
+- :support:`-` Publicly document the `.util` module and its `~.util.set_runner`
+  decorator, which decorates the functions in the `.files` module (and any
+  future applicable modules) allowing users to specify extra arguments like
+  ``sudo=True`` or ``runner_method="local"``.
+
+  As part of publicizing this, we added some docstring mutation to the
+  decorator so returned objects expose Sphinx autodoc hints and parameter
+  lists. This should replace the ``function(*args, **kwargs)`` signatures that
+  used to end up in the rendered documentation.
+- :support:`-` Add parameter lists to the members of the `.files` module.
 - :release:`1.0.1 <2018-06-20>`
 - :bug:`23` Fix some outstanding Python 2-isms (use of ``iteritems``) in
   `.info.distro_name` and `.info.distro_family`, as well as modules which

patchwork/files.py

@@ -13,6 +13,17 @@
 def directory(c, runner, path, user=None, group=None, mode=None):
     """
     Ensure a directory exists and has given user and/or mode
+
+    :param c:
+        `~invoke.context.Context` within to execute commands.
+    :param str path:
+        File path to directory.
+    :param str user:
+        Username which should own the directory.
+    :param str group:
+        Group which should own the directory; defaults to ``user``.
+    :param str mode:
+        ``chmod`` compatible mode string to apply to the directory.
     """
     runner("mkdir -p {}".format(path))
     if user is not None:
@@ -26,6 +37,11 @@ def directory(c, runner, path, user=None, group=None, mode=None):
 def exists(c, runner, path):
     """
     Return True if given path exists on the current remote host.
+
+    :param c:
+        `~invoke.context.Context` within to execute commands.
+    :param str path:
+        Path to check for existence.
     """
     cmd = 'test -e "$(echo {})"'.format(path)
     return runner(cmd, hide=True, warn=True).ok
@@ -41,13 +57,23 @@ def contains(c, runner, filename, text, exact=False, escape=True):
     change this behavior so that only a line containing exactly ``text``
     results in a True return value.
 
-    This function leverages ``egrep`` on the remote end (so it may not follow
-    Python regular expression syntax perfectly), and skips the usual outer
-    ``env.shell`` wrapper that most commands execute with.
+    This function leverages ``egrep`` on the remote end, so it may not follow
+    Python regular expression syntax perfectly.
 
     If ``escape`` is False, no extra regular expression related escaping is
     performed (this includes overriding ``exact`` so that no ``^``/``$`` is
     added.)
+
+    :param c:
+        `~invoke.context.Context` within to execute commands.
+    :param str filename:
+        File path within which to check for ``text``.
+    :param str text:
+        Text to search for.
+    :param bool exact:
+        Whether to expect an exact match.
+    :param bool escape:
+        Whether to perform regex-oriented escaping on ``text``.
     """
     if escape:
         text = _escape_for_regex(text)
@@ -76,6 +102,18 @@ def append(c, runner, filename, text, partial=False, escape=True):
 
     Because ``text`` is single-quoted, single quotes will be transparently
     backslash-escaped. This can be disabled with ``escape=False``.
+
+    :param c:
+        `~invoke.context.Context` within to execute commands.
+    :param str filename:
+        File path to append onto.
+    :param str text:
+        Text to append.
+    :param bool partial:
+        Whether to test partial line matches when determining if appending is
+        necessary.
+    :param bool escape:
+        Whether to perform regex-oriented escaping on ``text``.
     """
     # Normalize non-list input to be a list
     if isinstance(text, six.string_types):

patchwork/util.py

@@ -1,4 +1,11 @@
+"""
+Helpers and decorators, primarily for internal or advanced use.
+"""
+
+import textwrap
+
 from functools import wraps
+from inspect import getargspec, formatargspec
 
 
 # TODO: calling all functions as eg directory(c, '/foo/bar/') (with initial c)
@@ -25,56 +32,42 @@
 # or 3.0 release to see it move elsewhere once patterns are established.
 
 
-# TODO: is it possible to both expose the wrapped function's signature to
-# Sphinx autodoc AND remain a signature-altering decorator? =/ Using @decorator
-# is nice but it is only for signature-PRESERVING decorators. We really want a)
-# prevent users from having to phrase runner as 'runner=None', and b) the
-# convenience of sudo=True...but that means signature-altering.
-# TODO: this may be another spot where we want to reinstate fabric 1's docs
-# unwrap function.
 def set_runner(f):
     """
     Set 2nd posarg of decorated function to some callable ``runner``.
 
     The final value of ``runner`` depends on other args given to the decorated
     function (**note:** *not* the decorator itself!) as follows:
 
-    - By default, you can simply ignore the decorated function's ``runner``
-      argument entirely, in which case it will end up being set to the ``run``
-      method on the first positional arg (expected to be a
-      `~invoke.context.Context` and thus, the default value is `Context.run
-      <invoke.context.Context.run>`).
-    - You can override which method on that object is selected, by handing an
+    - By default, ``runner`` is set to the ``run`` method of the first
+      positional arg, which is expected to be a `~invoke.context.Context` (or
+      subclass). Thus the default runner is `Context.run
+      <invoke.context.Context.run>` (or, if the function was given a Fabric
+      `~fabric.connection.Connection`, `Connection.run
+      <fabric.connection.Connection.run>`).
+    - You can override which method on the context is selected, by handing an
       attribute name string to ``runner_method``.
-    - Since the common case for this functionality is to trigger use of
-      `~invoke.context.Context.sudo`, there is a convenient shorthand, setting
+    - Since the common case for overriding the runner is to trigger use of
+      `~invoke.context.Context.sudo`, there is a convenient shorthand: giving
       ``sudo=True``.
     - Finally, you may give a callable object to ``runner`` directly, in which
       case nothing special really happens (it's largely as if you called the
-      function undecorated). This is useful for cases where you're calling one
+      function undecorated, albeit with a kwarg instead of a positional
+      argument). This is mostly useful for cases where you're calling one
       decorated function from within another.
 
-    .. note::
-        The ``runner_method`` and ``sudo`` kwargs exist only at the decorator
-        level, and are not passed into the decorated function.
-
-    .. note::
-        If more than one of the above kwargs is given at the same time, only
-        one will win, in the following order: ``runner``, then
-        ``runner_method``, then ``sudo``.
-
-    As an example, given this example ``function``::
+    Given this ``function``::
 
         @set_runner
         def function(c, runner, arg1, arg2=None):
             runner("some command based on arg1 and arg2")
 
-    One may call it without any runner-related arguments, in which case
+    one may call it without any runner-related arguments, in which case
     ``runner`` ends up being a reference to ``c.run``::
 
         function(c, "my-arg1", arg2="my-arg2")
 
-    Or one may specify ``sudo`` to trigger use of ``c.sudo``::
+    or one may specify ``sudo`` to trigger use of ``c.sudo``::
 
         function(c, "my-arg1", arg2="my-arg2", sudo=True)
 
@@ -83,17 +76,29 @@ def function(c, runner, arg1, arg2=None):
 
         class AdminContext(Context):
             def run_admin(self, *args, **kwargs):
-                kwargs['user'] = 'admin'
+                kwargs["user"] = "admin"
                 return self.sudo(*args, **kwargs)
 
-        function(AdminContext(), "my-arg1", runner_method='run_admin')
+        function(AdminContext(), "my-arg1", runner_method="run_admin")
 
-    Finally, to reiterate, you can always give ``runner`` directly to avoid any
-    special processing (though be careful not to get mixed up - if this runner
-    isn't actually a method on the ``c`` context object, debugging could be
-    frustrating!)::
+    As noted above, you can always give ``runner`` (as a kwarg) directly to
+    avoid most special processing::
 
         function(c, "my-arg1", runner=some_existing_runner_object)
+
+    .. note::
+        If more than one of the ``runner_method``, ``sudo`` or ``runner``
+        kwargs are given simultaneously, only one will win, in the following
+        order: ``runner``, then ``runner_method``, then ``sudo``.
+
+    .. note::
+        As part of the signature modification, `set_runner` also modifies the
+        resulting value's docstring as follows:
+
+        - Prepends a Sphinx autodoc compatible signature string, which is
+          stripped out automatically on doc builds; see the Sphinx
+          ``autodoc_docstring_signature`` setting.
+        - Adds trailing ``:param:`` annotations for the extra args as well.
     """
 
     @wraps(f)
@@ -113,4 +118,33 @@ def inner(*args, **kwargs):
         args.insert(1, runner)
         return f(*args, **kwargs)
 
+    inner.__doc__ = munge_docstring(f, inner)
     return inner
+
+
+def munge_docstring(f, inner):
+    # Terrible, awful hacks to ensure Sphinx autodoc sees the intended
+    # (modified) signature; leverages the fact that autodoc_docstring_signature
+    # is True by default.
+    args, varargs, keywords, defaults = getargspec(f)
+    # Nix positional version of runner arg, which is always 2nd
+    del args[1]
+    # Add new args to end in desired order
+    args.extend(["sudo", "runner_method", "runner"])
+    # Add default values (remembering that this tuple matches the _end_ of the
+    # signature...)
+    defaults = tuple(list(defaults or []) + [False, "run", None])
+    # Get signature first line for Sphinx autodoc_docstring_signature
+    sigtext = "{}{}".format(
+        f.__name__, formatargspec(args, varargs, keywords, defaults)
+    )
+    docstring = textwrap.dedent(inner.__doc__ or "").strip()
+    # Construct :param: list
+    params = """:param bool sudo:
+    Whether to run shell commands via ``sudo``.
+:param str runner_method:
+    Name of context method to use when running shell commands.
+:param runner:
+    Callable runner function or method. Should ideally be a bound method on the given context object!
+"""  # noqa
+    return "{}\n{}\n\n{}".format(sigtext, docstring, params)

tests/util.py

@@ -1,3 +1,5 @@
+import re
+
 from invoke import Context
 
 from patchwork.util import set_runner
@@ -99,3 +101,85 @@ def myfunc(c, runner):
                 assert runner == c.run
 
             myfunc(Context(), runner_method="run", sudo=True)
+
+        class modified_signature_prepended_to_docstring:
+
+            # NOTE: This is the main canary test with full, exact expectations.
+            # The rest use shortcuts for developer sanity.
+            def multi_line(self):
+
+                @set_runner
+                def myfunc(c, runner, foo, bar="biz"):
+                    """
+                    whatever
+
+                    seriously, whatever
+                    """
+                    pass
+
+                expected = """myfunc(c, foo, bar='biz', sudo=False, runner_method='run', runner=None)
+whatever
+
+seriously, whatever
+
+:param bool sudo:
+    Whether to run shell commands via ``sudo``.
+:param str runner_method:
+    Name of context method to use when running shell commands.
+:param runner:
+    Callable runner function or method. Should ideally be a bound method on the given context object!
+"""  # noqa
+                assert myfunc.__doc__ == expected
+
+            def none(self):
+
+                @set_runner
+                def myfunc(c, runner, foo, bar="biz"):
+                    pass
+
+                assert re.match(
+                    r"myfunc\(.*, sudo=False.*\)\n.*:param str runner_method:.*\n",  # noqa
+                    myfunc.__doc__,
+                    re.DOTALL,
+                )
+
+            def empty(self):
+
+                @set_runner
+                def myfunc(c, runner, foo, bar="biz"):
+                    ""
+                    pass
+
+                assert re.match(
+                    r"myfunc\(.*, sudo=False.*\)\n.*:param str runner_method:.*\n",  # noqa
+                    myfunc.__doc__,
+                    re.DOTALL,
+                )
+
+            def single_line_no_newlines(self):
+
+                @set_runner
+                def myfunc(c, runner, foo, bar="biz"):
+                    "whatever"
+                    pass
+
+                assert re.match(
+                    r"myfunc\(.*, sudo=False.*\)\nwhatever\n\n.*:param str runner_method:.*\n",  # noqa
+                    myfunc.__doc__,
+                    re.DOTALL,
+                )
+
+            def single_line_with_newlines(self):
+
+                @set_runner
+                def myfunc(c, runner, foo, bar="biz"):
+                    """
+                    whatever
+                    """
+                    pass
+
+                assert re.match(
+                    r"myfunc\(.*, sudo=False.*\)\nwhatever\n\n.*:param str runner_method:.*\n",  # noqa
+                    myfunc.__doc__,
+                    re.DOTALL,
+                )