display wrap/format arg cleanup (#85352)

Co-authored-by: Matt Clay <matt@mystile.com>

changelogs/fragments/display_args.yml

@@ -0,0 +1,5 @@
+minor_changes:
+  - display - The ``formatted`` arg to ``warning`` has no effect.
+    Warning wrapping is left to the consumer (e.g. terminal, browser).
+  - display - The ``wrap_text`` and ``stderr`` arguments to ``error`` have no effect.
+    Errors are always sent to stderr and wrapping is left to the consumer (e.g. terminal, browser).

lib/ansible/config/base.yml

@@ -2185,12 +2185,6 @@ WIN_ASYNC_STARTUP_TIMEOUT:
   vars:
     - {name: ansible_win_async_startup_timeout}
   version_added: '2.10'
-WRAP_STDERR:
-  description: Control line-wrapping behavior on console warnings and errors from default output callbacks (eases pattern-based output testing)
-  env: [{name: ANSIBLE_WRAP_STDERR}]
-  default: false
-  type: bool
-  version_added: "2.19"
 YAML_FILENAME_EXTENSIONS:
   name: Valid YAML extensions
   default: [".yml", ".yaml", ".json"]

lib/ansible/plugins/callback/__init__.py

@@ -353,8 +353,7 @@ class CallbackBase(AnsiblePlugin):
         if res.pop('warnings', None) and self._current_task_result and (warnings := self._current_task_result.warnings):
             # display warnings from the current task result if `warnings` was not removed from `result` (or made falsey)
             for warning in warnings:
-                # DTFIX3: what to do about propagating wrap_text from the original display.warning call?
-                self._display._warning(warning, wrap_text=False)
+                self._display._warning(warning)
 
         if res.pop('deprecations', None) and self._current_task_result and (deprecations := self._current_task_result.deprecations):
             # display deprecations from the current task result if `deprecations` was not removed from `result` (or made falsey)
@@ -364,7 +363,7 @@ class CallbackBase(AnsiblePlugin):
     def _handle_exception(self, result: _c.MutableMapping[str, t.Any], use_stderr: bool = False) -> None:
         if result.pop('exception', None) and self._current_task_result and (exception := self._current_task_result.exception):
             # display exception from the current task result if `exception` was not removed from `result` (or made falsey)
-            self._display._error(exception, wrap_text=False, stderr=use_stderr)
+            self._display._error(exception, stderr=use_stderr)
 
     def _handle_warnings_and_exception(self, result: CallbackTaskResult) -> None:
         """Standardized handling of warnings/deprecations and exceptions from a task/item result."""

lib/ansible/utils/display.py

@@ -40,7 +40,6 @@ import secrets
 import subprocess
 import sys
 import termios
-import textwrap
 import threading
 import time
 import tty
@@ -329,7 +328,6 @@ class Display(metaclass=Singleton):
         self.noncow = C.ANSIBLE_COW_SELECTION
 
         self.set_cowsay_info()
-        self._wrap_stderr = C.WRAP_STDERR
 
         if self.b_cowsay:
             try:
@@ -663,13 +661,6 @@ class Display(metaclass=Singleton):
 
         return _join_sentences(msg, deprecation_msg)
 
-    def _wrap_message(self, msg: str, wrap_text: bool) -> str:
-        if wrap_text and self._wrap_stderr:
-            wrapped = textwrap.wrap(msg, self.columns, drop_whitespace=False)
-            msg = "\n".join(wrapped) + "\n"
-
-        return msg
-
     @staticmethod
     def _deduplicate(msg: str, messages: set[str]) -> bool:
         """
@@ -786,9 +777,6 @@ class Display(metaclass=Singleton):
         msg = _format_message(warning, _traceback.is_traceback_enabled(_traceback.TracebackEvent.DEPRECATED))
         msg = f'[DEPRECATION WARNING]: {msg}'
 
-        # DTFIX3: what should we do with wrap_message?
-        msg = self._wrap_message(msg=msg, wrap_text=True)
-
         if self._deduplicate(msg, self._deprecations):
             return
 
@@ -805,6 +793,8 @@ class Display(metaclass=Singleton):
         """Display a warning message."""
         _skip_stackwalk = True
 
+        # deprecated: description='The formatted argument has no effect.' core_version='2.23'
+
         # This is the pre-proxy half of the `warning` implementation.
         # Any logic that must occur on workers needs to be implemented here.
 
@@ -824,13 +814,12 @@ class Display(metaclass=Singleton):
 
         if warning_ctx := _DeferredWarningContext.current(optional=True):
             warning_ctx.capture(warning)
-            # DTFIX3: what to do about propagating wrap_text?
             return
 
-        self._warning(warning, wrap_text=not formatted)
+        self._warning(warning)
 
     @_proxy
-    def _warning(self, warning: _messages.WarningSummary, wrap_text: bool) -> None:
+    def _warning(self, warning: _messages.WarningSummary) -> None:
         """Internal implementation detail, use `warning` instead."""
 
         # This is the post-proxy half of the `warning` implementation.
@@ -842,9 +831,6 @@ class Display(metaclass=Singleton):
         if self._deduplicate(msg, self._warns):
             return
 
-        # DTFIX3: what should we do with wrap_message?
-        msg = self._wrap_message(msg=msg, wrap_text=wrap_text)
-
         self.display(msg, color=C.config.get_config_value('COLOR_WARN'), stderr=True, caplevel=-2)
 
     @_proxy
@@ -933,19 +919,20 @@ class Display(metaclass=Singleton):
             warning_ctx.capture(warning)
             return
 
-        self._warning(warning, wrap_text=False)
+        self._warning(warning)
 
     def error(self, msg: str | BaseException, wrap_text: bool = True, stderr: bool = True) -> None:
         """Display an error message."""
         _skip_stackwalk = True
 
+        # deprecated: description='The wrap_text argument has no effect.' core_version='2.23'
+        # deprecated: description='The stderr argument has no effect.' core_version='2.23'
+
         # This is the pre-proxy half of the `error` implementation.
         # Any logic that must occur on workers needs to be implemented here.
 
         if isinstance(msg, BaseException):
             event = _error_factory.ControllerEventFactory.from_exception(msg, _traceback.is_traceback_enabled(_traceback.TracebackEvent.ERROR))
-
-            wrap_text = False
         else:
             event = _messages.Event(
                 msg=msg,
@@ -956,10 +943,10 @@ class Display(metaclass=Singleton):
             event=event,
         )
 
-        self._error(error, wrap_text=wrap_text, stderr=stderr)
+        self._error(error, stderr=True)
 
     @_proxy
-    def _error(self, error: _messages.ErrorSummary, wrap_text: bool, stderr: bool) -> None:
+    def _error(self, error: _messages.ErrorSummary, stderr: bool) -> None:
         """Internal implementation detail, use `error` instead."""
 
         # This is the post-proxy half of the `error` implementation.
@@ -971,9 +958,6 @@ class Display(metaclass=Singleton):
         if self._deduplicate(msg, self._errors):
             return
 
-        # DTFIX3: what should we do with wrap_message?
-        msg = self._wrap_message(msg=msg, wrap_text=wrap_text)
-
         self.display(msg, color=C.config.get_config_value('COLOR_ERROR'), stderr=stderr, caplevel=-1)
 
     @staticmethod

test/integration/targets/data_tagging_controller/runme.sh

@@ -6,7 +6,7 @@ unset ANSIBLE_DEPRECATION_WARNINGS
 
 ansible-playbook untrusted_propagation.yml "$@" -e output_dir="${OUTPUT_DIR}"
 
-ANSIBLE_CALLBACK_FORMAT_PRETTY=0 ANSIBLE_WRAP_STDERR=0 _ANSIBLE_TEMPLAR_UNTRUSTED_TEMPLATE_BEHAVIOR=warning ansible-playbook -i hosts output_tests.yml -vvv 2>&1 | tee output.txt
+ANSIBLE_CALLBACK_FORMAT_PRETTY=0 _ANSIBLE_TEMPLAR_UNTRUSTED_TEMPLATE_BEHAVIOR=warning ansible-playbook -i hosts output_tests.yml -vvv 2>&1 | tee output.txt
 
 ../playbook_output_validator/filter.py actual_stdout.txt actual_stderr.txt < output.txt
 

test/units/utils/test_display.py

@@ -121,7 +121,7 @@ def test_Display_display_warn_fork(display_resource):
         display = Display()
         display.set_queue(queue)
         display.warning('foo')
-        queue.send_display.assert_called_once_with('_warning', _messages.WarningSummary(event=_messages.Event(msg='foo')), wrap_text=True)
+        queue.send_display.assert_called_once_with('_warning', _messages.WarningSummary(event=_messages.Event(msg='foo')))
 
     p = multiprocessing_context.Process(target=test)
     p.start()