From 5a7e448463eb896d090d6ede30f729e2cb8ab11c Mon Sep 17 00:00:00 2001
From: Alexander Plavin <alexander@plav.in>
Date: Tue, 2 Jul 2024 15:47:52 -0400
Subject: [PATCH 1/2] nicer show for optics

---
 src/sugar.jl      | 25 +++++++++++++++++++++----
 test/test_core.jl |  2 ++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/sugar.jl b/src/sugar.jl
index 2dd33fbf..640636b3 100644
--- a/src/sugar.jl
+++ b/src/sugar.jl
@@ -493,12 +493,23 @@ IndexLens(::Tuple{Properties}) = Properties()
 ### nice show() for optics
 _shortstring(prev, o::PropertyLens{field}) where {field} = "$prev.$field"
 _shortstring(prev, o::IndexLens) ="$prev[$(join(repr.(o.indices), ", "))]"
-_shortstring(prev, o::Function) = "$o($prev)"
-_shortstring(prev, o::Base.Fix1) = "$(o.f)($(o.x), $prev)"
-_shortstring(prev, o::Base.Fix2) = "$(o.f)($prev, $(o.x))"
+_shortstring(prev, o::Function) = _isoperator(o) ? "$o$prev" : "$o($prev)"
+_shortstring(prev, o::Base.Fix1) = _isoperator(o.f) ? "$(o.x) $(o.f) $prev" : "$(o.f)($(o.x), $prev)"
+_shortstring(prev, o::Base.Fix2) = _isoperator(o.f) ? "$prev $(o.f) $(o.x)" : "$(o.f)($prev, $(o.x))"
 _shortstring(prev, o::Elements) = "$prev[∗]"
 _shortstring(prev, o::Properties) = "$prev[∗ₚ]"
 
+# can f be stringfied using the operator (infix) syntax?
+# otherwise uses regular function call syntax
+_isoperator(f::Function) = Base.isoperator(nameof(f))
+_isoperator(f) = false
+
+# does o need parens when nested in another such o?
+# let's be conservative and always put parens around operators-in-operators
+_need_parens(o::Base.Fix1) = _isoperator(o.f)
+_need_parens(o::Base.Fix2) = _isoperator(o.f)
+_need_parens(o) = _isoperator(o)
+
 function show_optic(io, optic)
     opts = deopcompose(optic)
     inner = Iterators.takewhile(x -> applicable(_shortstring, "", x), opts)
@@ -507,7 +518,13 @@ function show_optic(io, optic)
         show(io, opcompose(outer...))
         print(io, " ∘ ")
     end
-    shortstr = reduce(_shortstring, inner; init="_")
+    shortstr = reduce(inner; init=("_", false)) do (prev, need_parens_prev), o
+        # if _need_parens is true for this o and the one before, wrap the previous one in parentheses
+        if need_parens_prev && _need_parens(o)
+            prev = "($prev)"
+        end
+        _shortstring(prev, o), _need_parens(o)
+    end |> first
     if get(io, :compact, false)
         print(io, shortstr)
     else
diff --git a/test/test_core.jl b/test/test_core.jl
index b292456d..a681d85f 100644
--- a/test/test_core.jl
+++ b/test/test_core.jl
@@ -461,6 +461,7 @@ end
 
 @testset "full and compact show" begin
     @test sprint(show, (@optic _.a)) == "(@o _.a)"
+    @test sprint(show, (@optic _.a + 1)) == "(@o _.a + 1)"
     @test sprint(show, (@optic log(_.a[2]))) == "(@o log(_.a[2]))"
     @test sprint(show, (@optic log(_).a[2])) == "(@o _.a[2]) ∘ log"  # could be shorter, but difficult to dispatch correctly without piracy
     @test sprint(show, (@optic log(_.a[2])); context=:compact => true) == "log(_.a[2])"
@@ -512,6 +513,7 @@ end
             (@optic _.a) ∘ UserDefinedLens()   ∘ (@optic _.b)
             (@optic _.a) ∘ LensIfTextPlain() ∘ (@optic _.b)
             @optic 2 * (abs(_.a.b[2].c) + 1)
+            @optic 2 * (-(abs(_.a.b[2].c) + 1))
             @optic !(_.a) # issue 105
         ]
         buf = IOBuffer()

From d5520ac4fcdbaf919a5c346e53752a89d040f913 Mon Sep 17 00:00:00 2001
From: Alexander Plavin <alexander@plav.in>
Date: Wed, 3 Jul 2024 06:42:22 -0400
Subject: [PATCH 2/2] more show() tests

---
 test/test_core.jl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/test_core.jl b/test/test_core.jl
index a681d85f..3232bf68 100644
--- a/test/test_core.jl
+++ b/test/test_core.jl
@@ -462,6 +462,8 @@ end
 @testset "full and compact show" begin
     @test sprint(show, (@optic _.a)) == "(@o _.a)"
     @test sprint(show, (@optic _.a + 1)) == "(@o _.a + 1)"
+    @test sprint(show, (@optic (_.a + 1) * 2)) == "(@o (_.a + 1) * 2)"
+    @test sprint(show, (@optic (_.a * 2) + 1)) == "(@o (_.a * 2) + 1)"
     @test sprint(show, (@optic log(_.a[2]))) == "(@o log(_.a[2]))"
     @test sprint(show, (@optic log(_).a[2])) == "(@o _.a[2]) ∘ log"  # could be shorter, but difficult to dispatch correctly without piracy
     @test sprint(show, (@optic log(_.a[2])); context=:compact => true) == "log(_.a[2])"