fix: show hidden line characters and accessible colors (#126)

* wip: rewrite assertion diff report

* wip: add colours, more context?

* wip: refactor staged line logic

* wip: linting

* wip: rewrite assertion diff report (#150)

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* refactor: replace generator type with iterator

* refactor: limit context using generators

* refactor: reduce diff

* refactor: add note about impossible cases

* test: hidden characters

* chore: update changelog

* chore: update changelog

* test: diff reporter

* lint: shorten line

* test: more snapshot test cases

* fix: allow context style to work in all backgrounds

Co-authored-by: Emmanuel Ogbizi-Ugbe <iamogbz+github@gmail.com>
Co-authored-by: Emmanuel Ogbizi <emmanuel.ogbizi-ugbe@tophatmonocle.com>

CHANGELOG.md

@@ -8,7 +8,8 @@ From v1.0.0 onwards, this project adheres to [Semantic Versioning](https://semve
 
 ## Master (Unreleased)
 
-- Up to date with releases
+- Fix bug where missing carriage return not shown in diff reports (#126)
+- Account for accessibility and readability in snapshot outputs (#126)
 
 ## [v0.3.3](https://github.com/tophat/syrupy/compare/v0.3.2...v0.3.3)
 

src/syrupy/__init__.py

@@ -15,9 +15,9 @@
 from .location import TestLocation
 from .session import SnapshotSession
 from .terminal import (
-    green,
-    red,
+    received_style,
     reset,
+    snapshot_style,
 )
 from .utils import import_module_member
 
@@ -64,10 +64,14 @@ def pytest_assertrepr_compare(op: str, left: Any, right: Any) -> Optional[List[s
     https://docs.pytest.org/en/latest/reference.html#_pytest.hookspec.pytest_assertrepr_compare
     """
     if isinstance(left, SnapshotAssertion):
-        assert_msg = reset(f"{green(left.name)} {op} {red('received')}")
+        assert_msg = reset(
+            f"{snapshot_style(left.name)} {op} {received_style('received')}"
+        )
         return [assert_msg] + left.get_assert_diff(right)
     elif isinstance(right, SnapshotAssertion):
-        assert_msg = reset(f"{red('received')} {op} {green(right.name)}")
+        assert_msg = reset(
+            f"{received_style('received')} {op} {snapshot_style(right.name)}"
+        )
         return [assert_msg] + right.get_assert_diff(left)
     return None
 

src/syrupy/constants.py

@@ -3,3 +3,7 @@
 SNAPSHOT_UNKNOWN_FOSSIL_KEY = "unknown snapshot fossil"
 
 EXIT_STATUS_FAIL_UNUSED = 1
+
+SYMBOL_ELLIPSIS = "..."  # U+2026
+SYMBOL_NEW_LINE = "␤"  # U+2424
+SYMBOL_CARRIAGE = "␍"  # U+240D

src/syrupy/data.py

@@ -3,6 +3,7 @@
     TYPE_CHECKING,
     Dict,
     Iterator,
+    List,
     Optional,
 )
 
@@ -117,3 +118,28 @@ def __iter__(self) -> Iterator["SnapshotFossil"]:
 
     def __contains__(self, key: str) -> bool:
         return key in self._snapshot_fossils
+
+
+@attr.s
+class DiffedLine:
+    a: str = attr.ib(default=None)
+    b: str = attr.ib(default=None)
+    c: List[str] = attr.ib(factory=list)
+    diff_a: str = attr.ib(default="")
+    diff_b: str = attr.ib(default="")
+
+    @property
+    def has_snapshot(self) -> bool:
+        return self.a is not None
+
+    @property
+    def has_received(self) -> bool:
+        return self.b is not None
+
+    @property
+    def is_complete(self) -> bool:
+        return self.has_snapshot and self.has_received
+
+    @property
+    def is_context(self) -> bool:
+        return bool(self.c)

src/syrupy/extensions/amber.py

@@ -8,6 +8,7 @@
     Set,
 )
 
+from syrupy.constants import SYMBOL_ELLIPSIS
 from syrupy.data import (
     Snapshot,
     SnapshotFossil,
@@ -28,7 +29,7 @@ class DataSerializer:
 
     class MarkerDepthMax:
         def __repr__(self) -> str:
-            return "..."
+            return SYMBOL_ELLIPSIS
 
     @classmethod
     def write_file(cls, snapshot_fossil: "SnapshotFossil") -> None:

src/syrupy/extensions/base.py

@@ -10,28 +10,36 @@
 from typing import (
     TYPE_CHECKING,
     Callable,
-    Generator,
+    Dict,
+    Iterator,
+    List,
     Optional,
     Set,
-    Union,
 )
 
 from typing_extensions import final
 
-from syrupy.constants import SNAPSHOT_DIRNAME
+from syrupy.constants import (
+    SNAPSHOT_DIRNAME,
+    SYMBOL_CARRIAGE,
+    SYMBOL_ELLIPSIS,
+    SYMBOL_NEW_LINE,
+)
 from syrupy.data import (
+    DiffedLine,
     Snapshot,
     SnapshotEmptyFossil,
     SnapshotFossil,
     SnapshotFossils,
 )
 from syrupy.exceptions import SnapshotDoesNotExist
 from syrupy.terminal import (
-    emphasize,
-    green,
-    mute,
-    red,
+    context_style,
+    received_diff_style,
+    received_style,
     reset,
+    snapshot_diff_style,
+    snapshot_style,
 )
 from syrupy.utils import walk_snapshot_dir
 
@@ -214,36 +222,141 @@ def __ensure_snapshot_dir(self, *, index: int) -> None:
 class SnapshotReporter(ABC):
     def diff_lines(
         self, serialized_data: "SerializedData", snapshot_data: "SerializedData"
-    ) -> Generator[str, None, None]:
+    ) -> Iterator[str]:
         for line in self.__diff_lines(str(snapshot_data), str(serialized_data)):
             yield reset(line)
 
-    def __diff_lines(self, a: str, b: str) -> Generator[str, None, None]:
-        line_styler = {"-": green, "+": red}
-        staged_line, skip = "", False
-        for line in ndiff(a.splitlines(), b.splitlines()):
-            if staged_line and line[:1] != "?":
-                yield line_styler[staged_line[:1]](staged_line)
-                staged_line, skip = "", False
-            if line[:1] in "-+":
-                staged_line = line
-            elif line[:1] == "?":
-                yield self.__diff_line(line, staged_line, line_styler[staged_line[:1]])
-                staged_line, skip = "", False
-            elif not skip:
-                yield mute("  ...")
-                skip = True
-        if staged_line:
-            yield line_styler[staged_line[:1]](staged_line)
-
-    def __diff_line(
-        self, marker_line: str, line: str, line_style: Callable[[Union[str, int]], str]
+    @property
+    def _ends(self) -> Dict[str, str]:
+        return {"\n": self._marker_new_line, "\r": self._marker_carriage}
+
+    @property
+    def _context_line_count(self) -> int:
+        return 1
+
+    @property
+    def _context_line_max(self) -> int:
+        return self._context_line_count * 2
+
+    @property
+    def _marker_context_max(self) -> str:
+        return SYMBOL_ELLIPSIS
+
+    @property
+    def _marker_new_line(self) -> str:
+        return SYMBOL_NEW_LINE
+
+    @property
+    def _marker_carriage(self) -> str:
+        return SYMBOL_CARRIAGE
+
+    def __diff_lines(self, a: str, b: str) -> Iterator[str]:
+        for line in self.__diff_data(a, b):
+            show_ends = (
+                self.__strip_ends(line.a[1:]) == self.__strip_ends(line.b[1:])
+                if line.is_complete
+                else False
+            )
+            if line.has_snapshot:
+                yield self.__format_line(
+                    line.a, line.diff_a, snapshot_style, snapshot_diff_style, show_ends
+                )
+            if line.has_received:
+                yield self.__format_line(
+                    line.b, line.diff_b, received_style, received_diff_style, show_ends
+                )
+            yield from map(context_style, self.__limit_context(line.c))
+
+    def __diff_data(self, a: str, b: str) -> Iterator["DiffedLine"]:
+        staged_diffed_line: Optional["DiffedLine"] = None
+        for line in ndiff(a.splitlines(keepends=True), b.splitlines(keepends=True)):
+            is_context_line = line[0] == " "
+            is_snapshot_line = line[0] == "-"
+            is_received_line = line[0] == "+"
+            is_diff_line = line[0] == "?"
+
+            if is_context_line or is_diff_line:
+                line = self.__strip_ends(line)
+
+            if staged_diffed_line:
+                if is_diff_line:
+                    if staged_diffed_line.has_received:
+                        staged_diffed_line.diff_b = line
+                    elif staged_diffed_line.has_snapshot:
+                        staged_diffed_line.diff_a = line
+                    # else: should never happen because then it would have
+                    # encounted a diff line without any previously staged line
+                else:
+                    should_unstage = (
+                        staged_diffed_line.is_complete
+                        or (staged_diffed_line.has_snapshot and is_snapshot_line)
+                        or (staged_diffed_line.has_received and is_received_line)
+                        or (staged_diffed_line.is_context and not is_context_line)
+                    )
+                    if should_unstage:
+                        yield staged_diffed_line
+                        staged_diffed_line = None
+                    elif is_snapshot_line:
+                        staged_diffed_line.a = line
+                    elif is_received_line:
+                        staged_diffed_line.b = line
+                    elif is_context_line:
+                        staged_diffed_line.c.append(line)
+
+            if not staged_diffed_line:
+                if is_snapshot_line:
+                    staged_diffed_line = DiffedLine(a=line)
+                elif is_received_line:
+                    staged_diffed_line = DiffedLine(b=line)
+                elif is_context_line:
+                    staged_diffed_line = DiffedLine(c=[line])
+                # else: should never happen because then it would have
+                # encounted a diff line without any previously staged line
+
+        if staged_diffed_line:
+            yield staged_diffed_line
+
+    def __format_line(
+        self,
+        line: str,
+        diff_markers: str,
+        line_style: Callable[[str], str],
+        diff_style: Callable[[str], str],
+        show_ends: bool,
     ) -> str:
+        if show_ends:
+            for old, new in self._ends.items():
+                line = line.replace(old, new)
+        else:
+            line = self.__strip_ends(line)
         return "".join(
-            emphasize(line_style(char)) if str(marker) in "-+^" else line_style(char)
-            for marker, char in zip_longest(marker_line.strip(), line)
+            diff_style(char) if str(marker) in "-+^" else line_style(char)
+            for marker, char in zip_longest(diff_markers.rstrip(), line)
+            if char is not None
         )
 
+    def __limit_context(self, lines: List[str]) -> Iterator[str]:
+        yield from lines[: self._context_line_count]
+        num_lines = len(lines)
+        if num_lines:
+            if num_lines > self._context_line_max:
+                count_leading_whitespace: Callable[[str], int] = (
+                    lambda s: len(s) - len(s.lstrip())  # noqa: E731
+                )
+                if self._context_line_count:
+                    num_space = (
+                        count_leading_whitespace(lines[self._context_line_count - 1])
+                        + count_leading_whitespace(lines[-self._context_line_count])
+                    ) // 2
+                else:
+                    num_space = count_leading_whitespace(lines[num_lines // 2])
+                yield " " * num_space + self._marker_context_max
+            if self._context_line_count and num_lines > 1:
+                yield from lines[-self._context_line_count :]  # noqa: E203
+
+    def __strip_ends(self, line: str) -> str:
+        return line.rstrip("".join(self._ends.keys()))
+
 
 class AbstractSyrupyExtension(SnapshotSerializer, SnapshotFossilizer, SnapshotReporter):
     def __init__(self, test_location: "TestLocation"):

src/syrupy/report.py

@@ -6,7 +6,7 @@
 from typing import (
     TYPE_CHECKING,
     Any,
-    Generator,
+    Iterator,
     List,
     Set,
 )
@@ -131,7 +131,7 @@ def unused(self) -> "SnapshotFossils":
         return unused_fossils
 
     @property
-    def lines(self) -> Generator[str, None, None]:
+    def lines(self) -> Iterator[str]:
         summary_lines: List[str] = []
         if self.num_failed:
             summary_lines.append(

src/syrupy/terminal.py

@@ -23,14 +23,6 @@ def bold(text: Union[str, int]) -> str:
     return colored.stylize(text, colored.attr("bold"))
 
 
-def mute(text: Union[str, int]) -> str:
-    return colored.stylize(text, colored.attr("dim"))
-
-
-def emphasize(text: Union[str, int]) -> str:
-    return colored.stylize(bold(text), colored.attr("underlined"))
-
-
 def error_style(text: Union[str, int]) -> str:
     return bold(red(text))
 
@@ -41,3 +33,23 @@ def warning_style(text: Union[str, int]) -> str:
 
 def success_style(text: Union[str, int]) -> str:
     return bold(green(text))
+
+
+def snapshot_style(text: Union[str, int]) -> str:
+    return colored.stylize(text, colored.bg(225) + colored.fg(90))
+
+
+def snapshot_diff_style(text: Union[str, int]) -> str:
+    return colored.stylize(text, colored.bg(90) + colored.fg(225))
+
+
+def received_style(text: Union[str, int]) -> str:
+    return colored.stylize(text, colored.bg(195) + colored.fg(23))
+
+
+def received_diff_style(text: Union[str, int]) -> str:
+    return colored.stylize(text, colored.bg(23) + colored.fg(195))
+
+
+def context_style(text: Union[str, int]) -> str:
+    return colored.stylize(text, colored.attr("dim"))

src/syrupy/utils.py

@@ -2,7 +2,7 @@
 from pathlib import Path
 from typing import (
     Any,
-    Generator,
+    Iterator,
 )
 
 from .constants import SNAPSHOT_DIRNAME
@@ -13,7 +13,7 @@ def in_snapshot_dir(path: Path) -> bool:
     return SNAPSHOT_DIRNAME in path.parts
 
 
-def walk_snapshot_dir(root: str) -> Generator[str, None, None]:
+def walk_snapshot_dir(root: str) -> Iterator[str]:
     for filepath in Path(root).rglob("*"):
         if not in_snapshot_dir(filepath):
             continue