Use Rich for check output (#874)

* Use print instead of output_stream

* Use logging for errors and warnings

* Add color to status

* Update tests

* Update changelog

changelog/874.feature.rst

@@ -0,0 +1 @@
+Use Rich to add color to ``check`` output.

tests/test_check.py

@@ -11,7 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-import io
+import logging
 
 import pretend
 import pytest
@@ -45,24 +45,27 @@ def test_str_representation(self):
         assert str(self.stream) == "result"
 
 
-def test_check_no_distributions(monkeypatch):
-    stream = io.StringIO()
-
+def test_check_no_distributions(monkeypatch, caplog):
     monkeypatch.setattr(commands, "_find_dists", lambda a: [])
 
-    assert not check.check(["dist/*"], output_stream=stream)
-    assert stream.getvalue() == "No files to check.\n"
+    assert not check.check(["dist/*"])
+    assert caplog.record_tuples == [
+        (
+            "twine.commands.check",
+            logging.ERROR,
+            "No files to check.",
+        ),
+    ]
 
 
-def test_check_passing_distribution(monkeypatch):
+def test_check_passing_distribution(monkeypatch, capsys):
     renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: "valid"))
     package = pretend.stub(
         metadata_dictionary=lambda: {
             "description": "blah",
             "description_content_type": "text/markdown",
         }
     )
-    output_stream = io.StringIO()
     warning_stream = ""
 
     monkeypatch.setattr(check, "_RENDERERS", {None: renderer})
@@ -74,13 +77,17 @@ def test_check_passing_distribution(monkeypatch):
     )
     monkeypatch.setattr(check, "_WarningStream", lambda: warning_stream)
 
-    assert not check.check(["dist/*"], output_stream=output_stream)
-    assert output_stream.getvalue() == "Checking dist/dist.tar.gz: PASSED\n"
+    assert not check.check(["dist/*"])
+    assert capsys.readouterr().out == "Checking dist/dist.tar.gz: PASSED\n"
     assert renderer.render.calls == [pretend.call("blah", stream=warning_stream)]
 
 
 @pytest.mark.parametrize("content_type", ["text/plain", "text/markdown"])
-def test_check_passing_distribution_with_none_renderer(content_type, monkeypatch):
+def test_check_passing_distribution_with_none_renderer(
+    content_type,
+    monkeypatch,
+    capsys,
+):
     """Pass when rendering a content type can't fail."""
     package = pretend.stub(
         metadata_dictionary=lambda: {
@@ -96,12 +103,11 @@ def test_check_passing_distribution_with_none_renderer(content_type, monkeypatch
         pretend.stub(from_filename=lambda *a, **kw: package),
     )
 
-    output_stream = io.StringIO()
-    assert not check.check(["dist/*"], output_stream=output_stream)
-    assert output_stream.getvalue() == "Checking dist/dist.tar.gz: PASSED\n"
+    assert not check.check(["dist/*"])
+    assert capsys.readouterr().out == "Checking dist/dist.tar.gz: PASSED\n"
 
 
-def test_check_no_description(monkeypatch, capsys):
+def test_check_no_description(monkeypatch, capsys, caplog):
     package = pretend.stub(
         metadata_dictionary=lambda: {
             "description": None,
@@ -116,18 +122,26 @@ def test_check_no_description(monkeypatch, capsys):
         pretend.stub(from_filename=lambda *a, **kw: package),
     )
 
-    # used to crash with `AttributeError`
-    output_stream = io.StringIO()
-    assert not check.check(["dist/*"], output_stream=output_stream)
-    assert output_stream.getvalue() == (
-        "Checking dist/dist.tar.gz: PASSED, with warnings\n"
-        "  warning: `long_description_content_type` missing. "
-        "defaulting to `text/x-rst`.\n"
-        "  warning: `long_description` missing.\n"
-    )
-
+    assert not check.check(["dist/*"])
 
-def test_strict_fails_on_warnings(monkeypatch, capsys):
+    assert capsys.readouterr().out == (
+        "Checking dist/dist.tar.gz: PASSED with warnings\n"
+    )
+    assert caplog.record_tuples == [
+        (
+            "twine.commands.check",
+            logging.WARNING,
+            "`long_description_content_type` missing. defaulting to `text/x-rst`.",
+        ),
+        (
+            "twine.commands.check",
+            logging.WARNING,
+            "`long_description` missing.",
+        ),
+    ]
+
+
+def test_strict_fails_on_warnings(monkeypatch, capsys, caplog):
     package = pretend.stub(
         metadata_dictionary=lambda: {
             "description": None,
@@ -142,27 +156,34 @@ def test_strict_fails_on_warnings(monkeypatch, capsys):
         pretend.stub(from_filename=lambda *a, **kw: package),
     )
 
-    # used to crash with `AttributeError`
-    output_stream = io.StringIO()
-    assert check.check(["dist/*"], output_stream=output_stream, strict=True)
-    assert output_stream.getvalue() == (
-        "Checking dist/dist.tar.gz: FAILED, due to warnings\n"
-        "  warning: `long_description_content_type` missing. "
-        "defaulting to `text/x-rst`.\n"
-        "  warning: `long_description` missing.\n"
-    )
-
+    assert check.check(["dist/*"], strict=True)
 
-def test_check_failing_distribution(monkeypatch):
+    assert capsys.readouterr().out == (
+        "Checking dist/dist.tar.gz: FAILED due to warnings\n"
+    )
+    assert caplog.record_tuples == [
+        (
+            "twine.commands.check",
+            logging.WARNING,
+            "`long_description_content_type` missing. defaulting to `text/x-rst`.",
+        ),
+        (
+            "twine.commands.check",
+            logging.WARNING,
+            "`long_description` missing.",
+        ),
+    ]
+
+
+def test_check_failing_distribution(monkeypatch, capsys, caplog):
     renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: None))
     package = pretend.stub(
         metadata_dictionary=lambda: {
             "description": "blah",
             "description_content_type": "text/markdown",
         }
     )
-    output_stream = io.StringIO()
-    warning_stream = "WARNING"
+    warning_stream = "Syntax error"
 
     monkeypatch.setattr(check, "_RENDERERS", {None: renderer})
     monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"])
@@ -173,13 +194,17 @@ def test_check_failing_distribution(monkeypatch):
     )
     monkeypatch.setattr(check, "_WarningStream", lambda: warning_stream)
 
-    assert check.check(["dist/*"], output_stream=output_stream)
-    assert output_stream.getvalue() == (
-        "Checking dist/dist.tar.gz: FAILED\n"
-        "  `long_description` has syntax errors in markup and would not be "
-        "rendered on PyPI.\n"
-        "    WARNING"
-    )
+    assert check.check(["dist/*"])
+
+    assert capsys.readouterr().out == "Checking dist/dist.tar.gz: FAILED\n"
+    assert caplog.record_tuples == [
+        (
+            "twine.commands.check",
+            logging.ERROR,
+            "`long_description` has syntax errors in markup and would not be rendered "
+            "on PyPI.\nSyntax error",
+        ),
+    ]
     assert renderer.render.calls == [pretend.call("blah", stream=warning_stream)]
 
 
@@ -190,3 +215,8 @@ def test_main(monkeypatch):
 
     assert check.main(["dist/*"]) == check_result
     assert check_stub.calls == [pretend.call(["dist/*"], strict=False)]
+
+
+# TODO: Test print() color output
+
+# TODO: Test log formatting

twine/cli.py

@@ -37,6 +37,7 @@ def configure_output() -> None:
         # doesn't support that (https://github.com/Textualize/rich/issues/343).
         force_terminal=True,
         no_color=getattr(args, "no_color", False),
+        highlight=False,
         theme=rich.theme.Theme(
             {
                 "logging.level.debug": "green",

twine/commands/check.py

@@ -15,16 +15,19 @@
 import argparse
 import cgi
 import io
+import logging
 import re
-import sys
-import textwrap
-from typing import IO, List, Optional, Tuple, cast
+from typing import List, Optional, Tuple, cast
 
 import readme_renderer.rst
+from rich import print
 
 from twine import commands
 from twine import package as package_file
 
+logger = logging.getLogger(__name__)
+
+
 _RENDERERS = {
     None: readme_renderer.rst,  # Default if description_content_type is None
     "text/plain": None,  # Rendering cannot fail
@@ -65,7 +68,7 @@ def write(self, text: str) -> None:
         )
 
     def __str__(self) -> str:
-        return self.output.getvalue()
+        return self.output.getvalue().strip()
 
 
 def _check_file(
@@ -104,7 +107,6 @@ def _check_file(
 
 def check(
     dists: List[str],
-    output_stream: IO[str] = sys.stdout,
     strict: bool = False,
 ) -> bool:
     """Check that a distribution will render correctly on PyPI and display the results.
@@ -124,39 +126,37 @@ def check(
     """
     uploads = [i for i in commands._find_dists(dists) if not i.endswith(".asc")]
     if not uploads:  # Return early, if there are no files to check.
-        output_stream.write("No files to check.\n")
+        logger.error("No files to check.")
         return False
 
     failure = False
 
     for filename in uploads:
-        output_stream.write("Checking %s: " % filename)
+        print(f"Checking {filename}: ", end="")
         render_warning_stream = _WarningStream()
         warnings, is_ok = _check_file(filename, render_warning_stream)
 
         # Print the status and/or error
         if not is_ok:
             failure = True
-            output_stream.write("FAILED\n")
-
-            error_text = (
-                "`long_description` has syntax errors in markup and "
-                "would not be rendered on PyPI.\n"
+            print("[red]FAILED[/red]")
+            logger.error(
+                "`long_description` has syntax errors in markup"
+                " and would not be rendered on PyPI."
+                f"\n{render_warning_stream}"
             )
-            output_stream.write(textwrap.indent(error_text, "  "))
-            output_stream.write(textwrap.indent(str(render_warning_stream), "    "))
         elif warnings:
             if strict:
                 failure = True
-                output_stream.write("FAILED, due to warnings\n")
+                print("[red]FAILED due to warnings[/red]")
             else:
-                output_stream.write("PASSED, with warnings\n")
+                print("[yellow]PASSED with warnings[/yellow]")
         else:
-            output_stream.write("PASSED\n")
+            print("[green]PASSED[/green]")
 
         # Print warnings after the status and/or error
         for message in warnings:
-            output_stream.write("  warning: " + message + "\n")
+            logger.warning(message)
 
     return failure