From 60051732089df9abb852a75fa79270fc3e5853ca Mon Sep 17 00:00:00 2001
From: rocky <rb@dustyfeet.com>
Date: Sat, 14 Dec 2024 11:51:12 -0500
Subject: [PATCH] Improve TraceEvaluation ...

* Show return values
* Show rewrite rules
---
 mathics/builtin/trace.py   | 10 +++++++--
 mathics/eval/tracing.py    | 44 ++++++++++++++++++++++++++++++++------
 test/builtin/test_trace.py | 10 +++++++--
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/mathics/builtin/trace.py b/mathics/builtin/trace.py
index 8a2233bbc..8bde31641 100644
--- a/mathics/builtin/trace.py
+++ b/mathics/builtin/trace.py
@@ -378,12 +378,17 @@ def eval(self, expr, evaluation: Evaluation, options: dict):
         curr_trace_evaluation = evaluation.definitions.trace_evaluation
         curr_time_by_steps = evaluation.definitions.timing_trace_evaluation
 
-        old_evaluation_hook = mathics.eval.tracing.trace_evaluate_on_call
+        old_evaluation_call_hook = mathics.eval.tracing.trace_evaluate_on_call
+        old_evaluation_return_hook = mathics.eval.tracing.trace_evaluate_on_return
 
         mathics.eval.tracing.trace_evaluate_on_call = (
             mathics.eval.tracing.print_evaluate
         )
 
+        mathics.eval.tracing.trace_evaluate_on_return = (
+            mathics.eval.tracing.print_evaluate
+        )
+
         evaluation.definitions.trace_evaluation = True
         evaluation.definitions.timing_trace_evaluation = (
             options["System`ShowTimeBySteps"] is SymbolTrue
@@ -397,7 +402,8 @@ def eval(self, expr, evaluation: Evaluation, options: dict):
             evaluation.definitions.trace_evaluation = curr_trace_evaluation
             evaluation.definitions.timing_trace_evaluation = curr_time_by_steps
 
-            mathics.eval.tracing.trace_evaluate_on_call = old_evaluation_hook
+            mathics.eval.tracing.trace_evaluate_on_call = old_evaluation_call_hook
+            mathics.eval.tracing.trace_evaluate_on_return = old_evaluation_return_hook
 
 
 class TraceEvaluationVariable(Builtin):
diff --git a/mathics/eval/tracing.py b/mathics/eval/tracing.py
index 596082957..4c0ee7e86 100644
--- a/mathics/eval/tracing.py
+++ b/mathics/eval/tracing.py
@@ -23,13 +23,18 @@ def print_evaluate(expr, evaluation, status: str, fn: Callable, orig_expr=None):
     method when TraceActivate["evaluate" -> True]
     """
 
-    # Test and dispose of various situations where showing information
-    # is pretty useless:
-    # Showing the return value of a ListExpression literal is
-    # useless.
     if evaluation.definitions.timing_trace_evaluation:
         evaluation.print_out(time.time() - evaluation.start_time)
 
+    # Test and dispose of various situations where showing information
+    # is pretty useless: evaluating a Symbol is the Symbol.
+    # Showing the return value of a ListExpression literal is
+    # also useless.
+    from mathics.core.symbols import Symbol, SymbolConstant
+
+    if isinstance(expr, Symbol) and not isinstance(expr, SymbolConstant):
+        return
+
     if (
         status == "Returning"
         and hasattr(expr, "is_literal")
@@ -39,8 +44,35 @@ def print_evaluate(expr, evaluation, status: str, fn: Callable, orig_expr=None):
     ):
         return
 
+    if orig_expr == expr:
+        # If the two expressions are the same, there is no point in
+        # repeating the output.
+        return
+
     indents = "  " * evaluation.recursion_depth
-    evaluation.print_out(f"{indents}{status} {fn.__qualname__}(): {expr}")
+
+    if orig_expr is not None:
+        if fn.__name__ == "rewrite_apply_eval_step":
+            assert isinstance(expr, tuple)
+            if orig_expr != expr[0]:
+                if status == "Returning":
+                    if expr[1]:
+                        status = "Rewriting"
+                        arrow = " -> "
+                    else:
+                        return
+                else:
+                    arrow = " = "
+                    return
+
+                evaluation.print_out(
+                    f"{indents}{status}: {expr[0]}" + arrow + str(expr)
+                )
+        else:
+            evaluation.print_out(f"{indents}{status}: {orig_expr} = " + str(expr))
+
+    elif fn.__name__ != "rewrite_apply_eval_step":
+        evaluation.print_out(f"{indents}{status} {fn.__qualname__}(): {expr}")
     return
 
 
@@ -122,7 +154,7 @@ def call_event_print(event: TraceEvent, fn: Callable, *args) -> bool:
     """
     A somewhat generic function to show an event-traced call.
     """
-    if type(fn) == type or inspect.ismethod(fn) or inspect.isfunction(fn):
+    if isinstance(type(fn), type) or inspect.ismethod(fn) or inspect.isfunction(fn):
         name = f"{fn.__module__}.{fn.__qualname__}"
     else:
         name = str(fn)
diff --git a/test/builtin/test_trace.py b/test/builtin/test_trace.py
index 0a1f96122..d177cfe07 100644
--- a/test/builtin/test_trace.py
+++ b/test/builtin/test_trace.py
@@ -2,12 +2,14 @@
 """
 Unit tests for mathics.builtin.trace
 """
-
+from inspect import isfunction
 from test.helper import evaluate
+from typing import Callable
 
 import pytest
 
 import mathics.eval.tracing
+from mathics.core.evaluation import Evaluation
 from mathics.core.interrupt import AbortInterrupt
 
 trace_evaluation_calls = 0
@@ -20,13 +22,17 @@ def test_TraceEvaluation():
     old_recursion_limit = evaluate("$RecursionLimit")
     old_evaluation_hook = mathics.eval.tracing.print_evaluate
 
-    def counting_print_evaluate(expr, evaluation, status: str, orig_expr=None) -> bool:
+    def counting_print_evaluate(
+        expr, evaluation: Evaluation, status: str, fn: Callable, orig_expr=None
+    ) -> bool:
         """
         A replacement for mathics.eval.tracing.print_evaluate() that counts the
         number of evaluation calls.
         """
         global trace_evaluation_calls
         trace_evaluation_calls += 1
+        assert status in ("Evaluating", "Returning")
+        assert isfunction(fn), "Expecting 4th argument to be a function"
         return False
 
     try: