Refactor Zipper to use a struct

[zachallaun]

Implements #96

TODO:

• Convert internal representation of Zipper from a two-tuple to a struct
• Implement Inspect for zippers
• Update the zippers intro notebook
• Update the multi-alias expansion notebook
• Update the Zipper moduledoc to document the inspect options

[zachallaun]

Any thoughts on how Inspect should work for zippers?

I'm wondering if we should use the :custom_options available in Inspect.Opts to support a few different ways of rendering:

• zipper: :as_ast (default) - only render the :node in its AST form, don't render path
• zipper: :as_code - format the current node as code -- could be useful for "finding your place" in a Zipper
• zipper: :raw - include full path

[doorgan]

@zachallaun these options sound good to me!

[doorgan]

An early comment:
We probably want to use something a little more coplex here to ensure better fit, like Nx does to print the tensors
https://github.com/elixir-nx/nx/blob/0af36df37f6b7582e242a9aa19748fb6a5e12dcd/nx/lib/nx/tensor.ex#L241-L248

[zachallaun]

Yeah, for sure -- I had to stop work on this early and was just committing what I had :)

[zachallaun]

I’d like to revisit Inspect later today.

I’ve been back and forth a lot on how to clearly surface useful information about “where you are” in the Zipper when displaying :as_ast or :as_code. The simplest thing is to just only show the current node, like

# :as_ast
#Sourceror.Zipper<
  {:def, [], [...]}
>

# :as_code
#Sourceror.Zipper<
  def foo do
    :bar
  end
>

But there are other potentially useful things to indicate that would normally be visible by looking at the path a bit:

• do we have ancestors/are we at the root?
• do we have siblings?

Spitballing a bit:

# :as_ast
#Sourceror.Zipper<
  #root
  {:def, [], [...]}
>

#Sourceror.Zipper<
  #...
  {:def, [], [...]}
  #...
>

# :as_code
#Sourceror.Zipper<
  #root
  def foo do
    :bar
  end
>

# all potentially valid:

#Sourceror.Zipper<
  #...
  def foo do
    :bar
  end
  #...
>

#Sourceror.Zipper<
  def foo do
    :bar
  end
  #...
>

#Sourceror.Zipper<
  #...
  def foo do
    :bar
  end
>

#Sourceror.Zipper<
  def foo do
    :bar
  end
>

Is it obvious what the #root and #... indicate here? I was writing out an explanation, but realized that a better "test" would be whether it is self-evident. :)

[zachallaun]

I went ahead and implemented it this way for now -- curious for your feedback when you have time!

[zachallaun]

One issue I found with :as_code -- there are situations where what's being formatted is a section of the AST that you can get to via the Zipper but isn't really valid AST, so the formatting you get is a bit weird/unexpected:

iex(22)> IEx.configure(inspect: [custom_options: [zipper: :as_code]])
:ok

iex(23)> "Foo.Bar.baz()" |> Code.string_to_quoted!() |> Z.zip()
#Sourceror.Zipper<
  #root
  Foo.Bar.baz()
>

iex(24)> "Foo.Bar.baz()" |> Code.string_to_quoted!() |> Z.zip() |> Z.next()
#Sourceror.Zipper<
  Foo.Bar . :baz
>

[zachallaun]

:as_code can also be a bit weird when you're dealing with Sourceror AST, which wraps literals in :__block__ tuples:

iex(5)> "1 + 2" |> Sourceror.parse_string!() |> Z.zip()
#Sourceror.Zipper<
  #root
  1 + 2
>

iex(6)> "1 + 2" |> Sourceror.parse_string!() |> Z.zip() |> Z.next()
#Sourceror.Zipper<
  1
  #...
>

iex(7)> "1 + 2" |> Sourceror.parse_string!() |> Z.zip() |> Z.next() |> Z.next()
#Sourceror.Zipper<
  1
>

[doorgan]

I think #root and #... are ok, but we might want to use a different color with IO.ANSI to make it clear it's a zipper formatting thing, and not an actual comment in the original code. We can use a greyish color for this

I wouldn't worry too much about a perfect representation of invalid ast; I think there are many disparate ways in which you can reach them, and this is a debugging utility so it shouldn't be too probolematic

[zachallaun]

Good idea! I looked into this initially but there wasn't a default "faint/subdued" color in :syntax_colors, which are used with Inspect.Algebra.color/3. We can add an arbitrary one though:

opts = Map.update!(opts, :syntax_colors, &Keyword.put_new(&1, :zipper_internal, :light_black))

...

color("#root", :zipper_internal, opts)

[zachallaun]

[zachallaun]

Need to do something else here; doesn't work on Elixir 1.12.3.

[zachallaun]

@doorgan I still need to update the multi-alias expansion example guide, but I think this is otherwise ready to review!

[zachallaun]

A couple thoughts:

1. Should the inspect option be zippers: :as_code etc. instead of zipper: :as_code to be consistent with the existing options? E.g. binaries: :as_strings, charlists: :as_lists

2. Should we provide a shortcut for setting a global default? More context:

With the multiple inspect options, it's kinda cumbersome to switch between them. Like if you just want your existing logging/etc. to always show a zipper in :raw format, you'd have to add custom_options: [zippers: :raw] at every call.

What if we had a public Sourceror.Zipper.Inspect that the inspect impl delegates to which could both serve as a place to document the inspect options and an API to set the default:

iex> Sourceror.Zipper.Inspect.default_option()
:as_ast

iex> Sourceror.Zipper.Inspect.default_option(:as_code)
:ok

[zachallaun]

I went ahead and implemented the suggestions I made above & updated the guides!

One note: I updated the guides using Livebook locally, but until a release is cut, you'll need to modify the Mix.install calls to be something like:

Mix.install([
  # :sourceror
  {:sourceror, path: Path.expand("../", __DIR__)}
])

[doorgan]

Looks great! Three cheers for the updated documentation too! 💜
Thank you for taking the time to implement this!

[doorgan]

💚 💙 💜 💛 ❤️