Always log shell ops

src/pyscaffold/actions.py

@@ -318,7 +318,7 @@ def init_git(struct: Structure, opts: ScaffoldOpts) -> ActionParams:
     """
     path = opts.get("project_path", ".")
     if not opts["update"] and not repo.is_git_repo(path):
-        repo.init_commit_repo(path, struct, log=True, pretend=opts.get("pretend"))
+        repo.init_commit_repo(path, struct, pretend=opts.get("pretend"))
 
     return struct, opts
 

src/pyscaffold/exceptions.py

@@ -77,7 +77,10 @@ class DirectoryDoesNotExist(DirectErrorForUser):
 class GitNotInstalled(DirectErrorForUser):
     """PyScaffold requires git to run."""
 
-    DEFAULT_MESSAGE = "Make sure git is installed and working."
+    DEFAULT_MESSAGE = (
+        "Make sure git is installed and working. "
+        "Use flag --very-verbose for more information."
+    )
 
     def __init__(self, message=DEFAULT_MESSAGE, *args, **kwargs):
         super().__init__(message, *args, **kwargs)

src/pyscaffold/extensions/namespace.py

@@ -132,7 +132,7 @@ def move_old_package(struct: Structure, opts: ScaffoldOpts) -> ActionParams:
             directory structure as dictionary of dictionaries and input options
     """
     project_path = Path(opts.get("project_path", "."))
-    with chdir(project_path, log=True, **opts):
+    with chdir(project_path, **opts):
         old_path = Path("src", opts["package"])
         namespace_path = opts["qual_pkg"].replace(".", os.sep)
         target = Path("src", namespace_path)
@@ -155,6 +155,6 @@ def move_old_package(struct: Structure, opts: ScaffoldOpts) -> ActionParams:
                     namespace_path,
                 )
 
-            move(old_path, target=target, log=True, pretend=opts["pretend"])
+            move(old_path, target=target, pretend=opts["pretend"])
 
     return struct, opts

src/pyscaffold/extensions/pre_commit.py

@@ -117,7 +117,7 @@ def install(struct: Structure, opts: ScaffoldOpts) -> ActionParams:
     if pre_commit:
         try:
             with chdir(opts.get("project_path", "."), **opts):
-                pre_commit("install", log=True, pretend=opts.get("pretend"))
+                pre_commit("install", pretend=opts.get("pretend"))
             logger.warning(SUCCESS_MSG)
             return struct, opts
         except ShellCommandException:

src/pyscaffold/file_system.py

@@ -35,17 +35,6 @@ def tmpfile(**kwargs):
         file.unlink()
 
 
-@contextmanager
-def _chdir_logging_context(path: PathLike, should_log: bool):
-    """Private auxiliar function for logging inside chdir"""
-    if should_log:
-        logger.report("chdir", path)
-        with logger.indent():
-            yield
-    else:
-        yield
-
-
 @contextmanager
 def chdir(path: PathLike, **kwargs):
     """Contextmanager to change into a directory
@@ -54,19 +43,18 @@ def chdir(path: PathLike, **kwargs):
         path : path to change current working directory to
 
     Keyword Args:
-        log (bool): log activity when true. Default: ``False``.
         pretend (bool): skip execution (but log) when pretending.
             Default ``False``.
     """
     should_pretend = kwargs.get("pretend")
-    should_log = kwargs.get("log", should_pretend)
     # ^ When pretending, automatically output logs
     #   (after all, this is the primary purpose of pretending)
 
     curr_dir = os.getcwd()
 
     try:
-        with _chdir_logging_context(path, should_log):
+        logger.report("chdir", path)
+        with logger.indent():
             if not should_pretend:
                 os.chdir(path)
             yield
@@ -84,20 +72,15 @@ def move(*src: PathLike, target: PathLike, **kwargs):
         target (PathLike): if target is a directory, ``src`` will be
             moved inside it. Otherwise, it will be the new path (note that it
             may be overwritten)
-        log (bool): log activity when true. Default: ``False``.
         pretend (bool): skip execution (but log) when pretending.
             Default ``False``.
     """
     should_pretend = kwargs.get("pretend")
-    should_log = kwargs.get("log", should_pretend)
-    # ^ When pretending, automatically output logs
-    #   (after all, this is the primary purpose of pretending)
 
     for path in src:
         if not should_pretend:
             shutil.move(str(path), str(target))
-        if should_log:
-            logger.report("move", path, target=target)
+        logger.report("move", path, target=target)
 
 
 def create_file(path: PathLike, content: str, pretend=False, encoding="utf-8"):

src/pyscaffold/shell.py

@@ -68,15 +68,9 @@ def __init__(self, command: str, shell: bool = True, cwd: Optional[str] = None):
     def run(self, *args, **kwargs) -> subprocess.CompletedProcess:
         """Execute command with the given arguments via :obj:`subprocess.run`."""
         command = f"{self._command} {join(args)}".strip()
+        logger.report("run", command, context=self._cwd)
 
         should_pretend = kwargs.pop("pretend", False)
-        should_log = kwargs.pop("log", should_pretend)
-        # ^ When pretending, automatically output logs
-        #   (after all, this is the primary purpose of pretending)
-
-        if should_log:
-            logger.report("run", command, context=self._cwd)
-
         if should_pretend:
             return subprocess.CompletedProcess(command, 0, None, None)
 
@@ -100,7 +94,11 @@ def __call__(self, *args, **kwargs) -> Iterator[str]:
         try:
             completed.check_returncode()
         except subprocess.CalledProcessError as ex:
-            msg = "\n".join(e or "" for e in (completed.stdout, completed.stderr))
+            stdout, stderr = (e or "" for e in (completed.stdout, completed.stderr))
+            stdout, stderr = (e.strip() for e in (stdout, stderr))
+            sep = " :: " if stdout and stderr else ""
+            msg = sep.join([stdout, stderr])
+            logger.debug(f"last command failed with {msg}")
             raise ShellCommandException(msg) from ex
 
         return (line for line in (completed.stdout or "").splitlines())

tests/conftest.py

@@ -248,8 +248,7 @@ def git_mock(monkeypatch, logger):
     def _git(*args, **kwargs):
         cmd = " ".join(["git"] + list(args))
 
-        if kwargs.get("log", False):
-            logger.report("run", cmd, context=os.getcwd())
+        logger.report("run", cmd, context=os.getcwd())
 
         def _response():
             yield "git@mock"

tests/test_file_system.py

@@ -22,7 +22,7 @@ def test_chdir(caplog, tmpdir, isolated_logger):
     curr_dir = os.getcwd()
     dname = uniqstr()  # Use a unique name to get easily identifiable logs
     temp_dir = str(tmpdir.mkdir(dname))
-    with fs.chdir(temp_dir, log=True):
+    with fs.chdir(temp_dir):
         new_dir = os.getcwd()
     assert new_dir == os.path.realpath(temp_dir)
     assert curr_dir == os.getcwd()
@@ -176,15 +176,8 @@ def test_move_log(tmpfolder, caplog):
     # Given a file or directory exists,
     tmpfolder.join(fname1).write("text")
     tmpfolder.join(fname2).write("text")
-    # When it is moved without log kwarg,
-    tmpfolder.join(dname).ensure_dir()
-    fs.move(fname1, target=dname)
-    # Then no log should be created.
-    logs = caplog.text
-    assert not re.search(f"move.+{fname1}.+to.+{dname}", logs)
-    # When it is moved with log kwarg,
-    fs.move(fname2, target=dname, log=True)
-    # Then log should be created.
+    fs.move(fname2, target=dname)
+    # log should be created.
     logs = caplog.text
     assert re.search(f"move.+{fname2}.+to.+{dname}", logs)