From 8dea3a07f0183be7aea2c7ad995f658d1eb71fc2 Mon Sep 17 00:00:00 2001 From: Michael Schmitt Date: Wed, 22 Apr 2026 19:44:42 +0900 Subject: [PATCH 1/2] feat: add colorized option for highstate output Colorize unified diffs in the highstate outputter: added lines are green, removed lines are red, hunk headers are cyan, and context lines are gray. `newfile:` and other change keys are rendered with the same indentation as the current functionality provides. --- changelog/68982.added.md | 6 + doc/ref/cli/_includes/output-options.rst | 15 +- doc/ref/configuration/master.rst | 10 + doc/ref/configuration/minion.rst | 10 + salt/output/highstate.py | 148 +++++++++++++- tests/pytests/unit/output/test_highstate.py | 213 ++++++++++++++++++++ 6 files changed, 397 insertions(+), 5 deletions(-) create mode 100644 changelog/68982.added.md diff --git a/changelog/68982.added.md b/changelog/68982.added.md new file mode 100644 index 000000000000..b6c4d421763b --- /dev/null +++ b/changelog/68982.added.md @@ -0,0 +1,6 @@ +Added ``_color`` modifier for ``state_output``: setting ``state_output`` to +``full_color``, ``terse_color``, ``mixed_color``, ``changes_color``, or +``filter_color`` enables colorized unified diff output in the highstate +outputter. Added lines are green, removed lines are red, hunk headers +(``@@``) are cyan, file headers (``---``) are red, and context lines are +gray. All other behavior is identical to the base mode without ``_color``. diff --git a/doc/ref/cli/_includes/output-options.rst b/doc/ref/cli/_includes/output-options.rst index a70c48ac153a..708623219f69 100644 --- a/doc/ref/cli/_includes/output-options.rst +++ b/doc/ref/cli/_includes/output-options.rst @@ -44,8 +44,19 @@ Output Options .. option:: --state-output=STATE_OUTPUT, --state_output=STATE_OUTPUT Override the configured state_output value for minion - output. One of 'full', 'terse', 'mixed', 'changes' or - 'filter'. Default: 'none'. + output. One of ``full``, ``terse``, ``mixed``, ``changes`` or + ``filter``. Default: ``none``. + + Each mode accepts two optional suffixes: + + * ``_id`` — use the state ID as the display name instead of the state's + ``name`` value (e.g. ``full_id``). + * ``_color`` — colorize unified diffs in the changes section: added lines + green, removed lines red, hunk headers (``@@``) cyan, file headers + (``---``) red, context lines gray (e.g. ``full_color``). + + The two suffixes can be combined in either order, e.g. ``full_id_color`` + or ``full_color_id``. .. option:: --state-verbose=STATE_VERBOSE, --state_verbose=STATE_VERBOSE diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 40e48c9b6ef0..26cf2e3a05e9 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -3072,10 +3072,20 @@ The state_output setting controls which results will be output full multi line: ``full_id``, ``mixed_id``, ``changes_id`` and ``terse_id`` are also allowed; when set, the state ID will be used as name in the output. +Any of the above modes can be suffixed with ``_color`` (e.g. ``full_color``, +``mixed_color``) to enable colorized unified diff output in the changes +section. Added lines are shown in green, removed lines in red, hunk headers +in cyan, and context lines in gray. All other output behavior is identical to +the mode without the ``_color`` suffix. + .. code-block:: yaml state_output: full +.. code-block:: yaml + + state_output: full_color + .. conf_master:: state_output_diff ``state_output_diff`` diff --git a/doc/ref/configuration/minion.rst b/doc/ref/configuration/minion.rst index a4a9354697c3..74da50a0c5d4 100644 --- a/doc/ref/configuration/minion.rst +++ b/doc/ref/configuration/minion.rst @@ -2408,10 +2408,20 @@ The state_output setting controls which results will be output full multi line: ``full_id``, ``mixed_id``, ``changes_id`` and ``terse_id`` are also allowed; when set, the state ID will be used as name in the output. +Any of the above modes can be suffixed with ``_color`` (e.g. ``full_color``, +``mixed_color``) to enable colorized unified diff output in the changes +section. Added lines are shown in green, removed lines in red, hunk headers +in cyan, and context lines in gray. All other output behavior is identical to +the mode without the ``_color`` suffix. + .. code-block:: yaml state_output: full +.. code-block:: yaml + + state_output: full_color + .. conf_minion:: state_output_diff ``state_output_diff`` diff --git a/salt/output/highstate.py b/salt/output/highstate.py index dc00885753fa..2ca5d23cfe07 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -44,13 +44,22 @@ These can be set as such from the command line, or in the Salt config as `state_output_exclude` or `state_output_terse`, respectively. - The output modes have one modifier: + The output modes have two modifiers that can be combined: ``full_id``, ``terse_id``, ``mixed_id``, ``changes_id`` and ``filter_id`` If ``_id`` is used, then the corresponding form will be used, but the value for ``name`` will be drawn from the state ID. This is useful for cases where the name value might be very long and hard to read. + ``full_color``, ``terse_color``, ``mixed_color``, ``changes_color`` and ``filter_color`` + If ``_color`` is used, unified diffs in the changes section will be + colorized: added lines in green, removed lines in red, hunk headers + (``@@``) in cyan, file headers (``---``) in red, and context lines in + gray. All other output behavior is identical to the base mode. + + The ``_id`` and ``_color`` modifiers can be combined, e.g. ``full_id_color`` + or ``full_color_id``. + state_tabular: If `state_output` uses the terse output, set this to `True` for an aligned output format. If you wish to use a custom format, this can be set to a @@ -479,6 +488,9 @@ def _format_host(host, data, indent_level=1): tcolor = colors["LIGHT_YELLOW"] state_output = __opts__.get("state_output", "full").lower() + # Strip the _color modifier before all mode comparisons so that + # e.g. "full_color" behaves identically to "full" for layout purposes. + state_output = state_output.replace("_color", "") comps = tname.split("_|-") if state_output.endswith("_id"): @@ -742,15 +754,138 @@ def _counts(label, count): return "\n".join(hstrs), nchanges > 0 +def _render_diff(diff_str, indent): + """ + Render a unified diff string with per-line ANSI colorization. + + Each line is colored according to its unified-diff role: + ``---`` / ``+++`` (file headers) → LIGHT_RED (bold) + ``@@`` (hunk header) → CYAN + ``+`` (added line) → GREEN + ``-`` (removed line) → RED + context lines (leading space) → GREEN (same as other change values) + + The ``indent`` argument (an integer) is prepended as spaces to every line, + matching the nesting depth used by the surrounding nested outputter output. + """ + prefix = " " * indent + + if __opts__.get("color") is False: + return "\n".join(prefix + line for line in diff_str.splitlines()) + + colors = salt.utils.color.get_colors(True, __opts__.get("color_theme")) + GREEN = str(colors["GREEN"]) + ENDC = str(colors["ENDC"]) + RED = str(colors["RED"]) + CYAN = str(colors["CYAN"]) + WHITE = str(colors["LIGHT_GRAY"]) + LIGHT_RED = str(colors["LIGHT_RED"]) + + result = [] + for line in diff_str.splitlines(): + if line.startswith("---"): + color = LIGHT_RED + elif line.startswith("+++"): + color = GREEN + elif line.startswith("@@"): + color = CYAN + elif line.startswith("+"): + color = GREEN + elif line.startswith("-"): + color = RED + else: + color = WHITE + result.append(f"{prefix}{color}{line}{ENDC}") + return "\n".join(result) + + +def _render_changes_dict(changes, indent): + """ + Render a changes dict as indented lines, mirroring nested outputter style. + + Does not go through the Salt loader, so nested_indent is guaranteed to + apply correctly regardless of Salt version. Returns a list of strings + (no trailing newline). + """ + colors = salt.utils.color.get_colors( + __opts__.get("color"), __opts__.get("color_theme") + ) + CYAN = str(colors["CYAN"]) + GREEN = str(colors["GREEN"]) + ENDC = str(colors["ENDC"]) + + val_indent = indent + 4 + pad = " " * indent + val_pad = " " * val_indent + lines = [] + # Top-level separator (mirrors what NestDisplay.display does for Mapping at indent>0) + lines.append(f"{pad}{CYAN}----------{ENDC}") + for key in sorted(changes): + lines.append(f"{pad}{CYAN}{key}:{ENDC}") + val = changes[key] + if isinstance(val, str): + lines.extend( + f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines() + ) + elif isinstance(val, dict): + lines.extend(_render_changes_dict(val, val_indent)) + else: + lines.append(f"{val_pad}{GREEN}{val}{ENDC}") + return lines + + def _nested_changes(changes): """ - Print the changes data using the nested outputter + Print the changes data using the nested outputter. """ ret = "\n" ret += salt.output.out_format(changes, "nested", __opts__, nested_indent=14) return ret +def _nested_changes_colorized(changes): + """ + Print the changes data with diff colorization (used when state_output + contains the ``_color`` modifier, e.g. ``full_color``). + + If the changes dict contains a ``diff`` key whose value is a string, that + diff is rendered with per-line color (added=green, removed=red, etc.). + All other values are rendered by ``_render_changes_dict`` which mirrors the + nested outputter layout without going through the Salt loader, ensuring + correct indentation on all Salt versions. + """ + diff_str = None + if isinstance(changes, dict) and isinstance(changes.get("diff"), str): + diff_str = changes.pop("diff") + + # key_indent=14: "----------" separator and key names sit at 14 spaces. + # val_indent=18: string values sit 4 spaces deeper. + key_indent = 14 + val_indent = key_indent + 4 + + colors = salt.utils.color.get_colors( + __opts__.get("color"), __opts__.get("color_theme") + ) + CYAN = str(colors["CYAN"]) + ENDC = str(colors["ENDC"]) + + ret = "\n" + if changes: + ret += "\n".join(_render_changes_dict(changes, key_indent)) + elif diff_str is not None: + # No other keys: emit the separator manually. + ret += f"{' ' * key_indent}{CYAN}----------{ENDC}" + + if diff_str is not None: + key_line = f"{' ' * key_indent}{CYAN}diff:{ENDC}" + rendered_diff = _render_diff(diff_str, val_indent) + ret += "\n" + key_line + "\n" + rendered_diff + # Restore the diff key so the caller's data structure is unchanged. + changes["diff"] = diff_str + + return ret + + def _format_changes(changes, orchestration=False): """ Format the changes dict based on what the data is @@ -758,7 +893,11 @@ def _format_changes(changes, orchestration=False): if not changes: return False, "" + colorize = "_color" in __opts__.get("state_output", "").lower() + if orchestration: + if colorize: + return True, _nested_changes_colorized(changes) return True, _nested_changes(changes) if not isinstance(changes, dict): @@ -774,7 +913,10 @@ def _format_changes(changes, orchestration=False): changed = changed or c else: changed = True - ctext = _nested_changes(changes) + if colorize: + ctext = _nested_changes_colorized(changes) + else: + ctext = _nested_changes(changes) return changed, ctext diff --git a/tests/pytests/unit/output/test_highstate.py b/tests/pytests/unit/output/test_highstate.py index 7661a17aa350..9189348f4615 100644 --- a/tests/pytests/unit/output/test_highstate.py +++ b/tests/pytests/unit/output/test_highstate.py @@ -917,3 +917,216 @@ def test_nested_output(): assert " Succeeded: 2 (changed=1)" in ret assert " Failed: 0" in ret assert " Total states run: 2" in ret + + +# --------------------------------------------------------------------------- +# Tests for diff colorization +# --------------------------------------------------------------------------- + +# ANSI color codes used by salt.utils.color when color=True +_GREEN = "\x1b[0;32m" +_RED = "\x1b[0;31m" +_CYAN = "\x1b[0;36m" +_WHITE = "\x1b[0;37m" +_LIGHT_RED = "\x1b[0;1;31m" +_ENDC = "\x1b[0;0m" + + +def _strip_ansi(text): + """Remove all ANSI escape sequences from *text*.""" + return re.sub(r"\x1b\[[0-9;]+m", "", text) + + +def _leading_color(line): + """Return the first ANSI escape code found on *line*, or '' if none.""" + m = re.search(r"(\x1b\[[0-9;]+m)", line) + return m.group(1) if m else "" + + +import re # noqa: E402 (re is already imported at top; harmless duplicate) + + +class TestRenderDiff: + """Tests for highstate._render_diff — the direct diff colorizer.""" + + @pytest.fixture(autouse=True) + def _setup(self, minion_opts): + minion_opts.update({"color": True, "color_theme": None}) + with patch.dict(highstate.__opts__, minion_opts): + yield + + def _render(self, diff_str, indent=18): + """Call _render_diff and return the output lines.""" + return highstate._render_diff(diff_str, indent).splitlines() + + # ------------------------------------------------------------------ + # Individual line-type tests + # ------------------------------------------------------------------ + def test_added_line_is_green(self): + out = self._render("+added content\n") + assert _leading_color(out[0]) == _GREEN + + def test_removed_line_is_red(self): + out = self._render("-removed content\n") + assert _leading_color(out[0]) == _RED + + def test_hunk_header_is_cyan(self): + out = self._render("@@ -1,3 +1,4 @@\n") + assert _leading_color(out[0]) == _CYAN + + def test_file_header_minus_is_light_red(self): + out = self._render("--- /etc/motd\n") + assert _leading_color(out[0]) == _LIGHT_RED + + def test_file_header_plus_is_green(self): + out = self._render("+++ /etc/motd\n") + assert _leading_color(out[0]) == _GREEN + + def test_context_line_is_white(self): + out = self._render(" a context line\n") + assert _leading_color(out[0]) == _WHITE + + # ------------------------------------------------------------------ + # Indentation + # ------------------------------------------------------------------ + def test_indent_is_applied(self): + out = self._render("+line\n", indent=18) + assert _strip_ansi(out[0]).startswith(" " * 18) + + def test_indent_zero(self): + out = self._render("+line\n", indent=0) + assert _strip_ansi(out[0]).startswith("+") + + # ------------------------------------------------------------------ + # Content preservation + # ------------------------------------------------------------------ + def test_content_is_preserved(self): + diff = ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,3 @@\n" + " context\n" + "-old line\n" + "+new line\n" + ) + out = self._render(diff, indent=18) + plain = [_strip_ansi(line).strip() for line in out] + assert plain == [ + "--- /etc/motd", + "+++ /etc/motd", + "@@ -1,2 +1,3 @@", + "context", + "-old line", + "+new line", + ] + + # ------------------------------------------------------------------ + # No-color mode: plain text, still indented + # ------------------------------------------------------------------ + def test_no_color_produces_plain_indented_text(self, minion_opts): + minion_opts.update({"color": False}) + with patch.dict(highstate.__opts__, minion_opts): + out = highstate._render_diff("+line\n-line\n", indent=18) + assert "\x1b" not in out + for line in out.splitlines(): + assert line.startswith(" " * 18) + + # ------------------------------------------------------------------ + # End-to-end: diff surfaced through the full highstate output pipeline + # ------------------------------------------------------------------ + def _run_diff_color_test(self, minion_opts): + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } + } + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + + # Removed line must be wrapped in RED + assert _RED in rendered + # Added line must be wrapped in GREEN + assert _GREEN in rendered + # Plain text must still be present + assert "-old line" in _strip_ansi(rendered) + assert "+new line" in _strip_ansi(rendered) + # Diff lines must be indented (18 spaces for value inside nested_indent=14) + for line in rendered.splitlines(): + plain = _strip_ansi(line) + if "-old line" in plain or "+new line" in plain: + assert plain.startswith( + " " * 18 + ), f"Expected 18-space indent: {repr(plain)}" + + def test_diff_in_full_color_output(self, minion_opts): + """file.managed diff has red removed and green added lines with full_color mode.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + self._run_diff_color_test(minion_opts) + + def test_diff_in_full_color_output_color_default(self, minion_opts): + """With color=None (default), full_color mode still colorizes diff lines.""" + minion_opts.update( + { + "color": None, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + self._run_diff_color_test(minion_opts) + + def test_diff_not_colorized_in_plain_full_output(self, minion_opts): + """With state_output=full (no _color), diff is not colorized.""" + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": {"diff": "-old line\n+new line\n"}, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + # The diff-specific RED should not appear without _color mode + assert _RED not in rendered From 9f479f58c9eccb1244d428ea68b6c6cb30f96981 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 12 Jun 2026 20:38:33 -0700 Subject: [PATCH 2/2] Fix pre-commit failures in highstate colorizer - Black reformat the lines.extend() generator call in _render_changes_dict - Replace U+2192 arrows in _render_diff docstring with ASCII -> so the cp1252 docstring-encoding hook passes (these characters break salt-run -d and salt -d on Windows where stdout uses the locale-default encoding) --- salt/output/highstate.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/salt/output/highstate.py b/salt/output/highstate.py index 2ca5d23cfe07..d7a513519b19 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -759,11 +759,11 @@ def _render_diff(diff_str, indent): Render a unified diff string with per-line ANSI colorization. Each line is colored according to its unified-diff role: - ``---`` / ``+++`` (file headers) → LIGHT_RED (bold) - ``@@`` (hunk header) → CYAN - ``+`` (added line) → GREEN - ``-`` (removed line) → RED - context lines (leading space) → GREEN (same as other change values) + ``---`` / ``+++`` (file headers) -> LIGHT_RED (bold) + ``@@`` (hunk header) -> CYAN + ``+`` (added line) -> GREEN + ``-`` (removed line) -> RED + context lines (leading space) -> GREEN (same as other change values) The ``indent`` argument (an integer) is prepended as spaces to every line, matching the nesting depth used by the surrounding nested outputter output. @@ -824,9 +824,7 @@ def _render_changes_dict(changes, indent): lines.append(f"{pad}{CYAN}{key}:{ENDC}") val = changes[key] if isinstance(val, str): - lines.extend( - f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines() - ) + lines.extend(f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines()) elif isinstance(val, dict): lines.extend(_render_changes_dict(val, val_indent)) else: