doorgan/sourceror

Introduce a `%Zipper{}` struct

zachallaun opened this issue · 10 comments

I absolutely love zippers now for all AST manipulation, but I find them sometimes difficult to debug simply because the inspect output is so verbose.

I’d propose introducing a Zipper struct. This would allow the Inspect protocol to be defined such that only the current node is visible and the surrounding context is “collapsed” in some way. I’d also personally prefer matching on a struct in code to guarantee non-nil vs. a two-element tuple, since it’s a lot more explicit.

I thought about this many times, I think it'd be nice so we can also implement other protocols, and maybe even the Access protocol

A PR is welcome!

I think it's enough to use the same fields as the meta map of the zipper, and put the node in a node field. Only the node field would be public API, the rest should be considered private api.
I don't recall if we have a way of expressing this with a typespec, it it's possible then great, if not then it's no big deal

Sounds good!

I don't think there's a way to make individual fields private in a typespec. Here's what Macro.Env does, for instance:

The following fields are private to Elixir's macro expansion mechanism and must not be accessed directly:
...

One option would be to keep the nesting and make path an opaque type:

defstruct [:node, :path]

@type t :: %__MODULE__{
  node: Macro.t(),
  path: path | nil
}

@opaque path :: %{
  l: [Macro.t()],
  ptree: t | nil,
  r: [Macro.t()]
}

Alternatively, the entire type could be opaque and Zipper.node/1 could be the only "public" way of accessing the node.

I think making the path opaque is good enough, I'd like to let users to pattern match on the zipper node. If that doesn't work then I wouldn't worry about it
If we want to go the extra mile we might want to rename the path properties to:
l -> left
r -> right
ptree -> parent_tree
If memory isn't failing me that was the meaning

Agreed!

While we're at it, what about making the parent public as well?

@type t :: %__MODULE__{
        node: Macro.t(),
        parent: t | nil,
        left: siblings,
        right: siblings
      }

@opaque siblings :: [Macro.t()]

The parent is not actually t, but a path, an "unzipped" representation of the tree
I'd say those are zipper internals that the user should not use and it's also hard to extract anything useful from it via pattern matching. Zipper.up is what gives you access to the parent
So, we can have it in the Zipper struct instead of in a path property, but I would still consider it private api

Oh, duh! I misread the original typespec. 👍

@doorgan Hmm, just wanted to confirm (and I apologize if I'm being obtuse), but it looks to me like the ptree is a zipper? link

It's unpacked as a "full zipper" elsewhere too.

I don't feel strongly about the parent tree being public or private, but just want to make sure I'm not missing something!

@zachallaun you're correct! I misremembered that
I'd still like for it to be private though, in case we want to add breadcrumbs(or as the paper calls them, scars), used for backtracking(which we currently don't support)

Implemented in #98